Page MenuHomePhabricator

Pull Doxyfile artifact from build.bitcoinabc.org
AbandonedPublic

Authored by jasonbcox on Mon, Jul 1, 21:01.

Details

Reviewers
deadalnix
Fabien
Group Reviewers
Restricted Project
Summary

PR10155 changes the Doxyfile from static to being generated by the build system. When the Doxyfile is static, it can be pulled from the repo without any changes and compiled by doxygen (which the host does today). D3508 changes Doxyfile -> Doxyfile.in which must be compiled by the build system prior to passing to doxygen. We pull the compiled Doxyfile from latest, known good TeamCity artifacts in order to deploy.

Test Plan
# build the docker image
# These env variables will be provided by the TeamCity infra and are only here to demonstrate how to test locally
export TEAMCITY_USERID="<your username>"
export TEAMCITY_PASSWORD="<your password>"
export TEAMCITY_URL="https://build.bitcoinabc.org"
docker build -t abc-docs -f Dockerfile-doxygen --build-arg TEAMCITY_USERID --build-arg TEAMCITY_PASSWORD --build-arg TEAMCITY_URL .

# run the docker image (nginx static HTML)
docker run --rm -p 4000:80 abc-docs

Diff Detail

Repository
rABC Bitcoin ABC
Branch
doxydocker
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6743
Build 11533: Bitcoin ABC Teamcity Staging
Build 11532: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Mon, Jul 1, 21:01
deadalnix requested changes to this revision.Mon, Jul 1, 22:47

The host serving the doc should probably not be the one building it. The description of the diff doesn't explain why this is necessary.

This revision now requires changes to proceed.Mon, Jul 1, 22:47
jasonbcox edited the summary of this revision. (Show Details)Tue, Jul 2, 15:07
jasonbcox requested review of this revision.Tue, Jul 2, 15:09
jasonbcox edited the summary of this revision. (Show Details)

Updated the summary to better reflect rationale.

Fabien requested changes to this revision.Wed, Jul 3, 15:11
Fabien added a subscriber: Fabien.
Fabien added inline comments.
Dockerfile-doxygen
4 ↗(On Diff #9881)

Why do you need 2 calls to apt-get install ?

12 ↗(On Diff #9881)

You should run with the -j option if possible.

This revision now requires changes to proceed.Wed, Jul 3, 15:11
jasonbcox updated this revision to Diff 10027.Sat, Jul 6, 03:08
  • one-line apt-get install
  • call make -j

I read the rationale and did not understand any of it. It seems like Doxyfile is generated by the build system now, but that doesn't explain why it has to be done on the machine hosting the doc. If deploying artifacts on host is something that is challenging, then it seems like this problem should be talked, because this is pretty much the ONLY thing that deployment does.

deadalnix requested changes to this revision.Sat, Jul 6, 15:24
This revision now requires changes to proceed.Sat, Jul 6, 15:24
jasonbcox updated this revision to Diff 10102.Mon, Jul 8, 18:27

Pull existing artifact from TeamCity instead of building it

jasonbcox retitled this revision from Run make in the doxygen docker build to Pull Doxyfile artifact from build.bitcoinabc.org.Mon, Jul 8, 18:33
jasonbcox edited the summary of this revision. (Show Details)
jasonbcox edited the test plan for this revision. (Show Details)
jasonbcox edited the test plan for this revision. (Show Details)
jasonbcox planned changes to this revision.Mon, Jul 8, 20:21
jasonbcox updated this revision to Diff 10111.Mon, Jul 8, 20:24

Tested against new TeamCity artifacts and fixed some bugs associated with the previous change.

jasonbcox updated this revision to Diff 10112.Mon, Jul 8, 20:25

Removed now unused .dockerignore

If whatever is calling this dockerfile has these parameters, then wouldn't that be better to simply download the file and copy it within the docker image ?

I don't have much experience with teamcity, but this way of doing things by download an artifact over the internet seems HIGHLY suspicious and do not seem very necessary. British airways just learned that lesson the hard way. Everybody's laughing their ass off while doing the same.

In addition, one has to wonder what this does at the root of the source code tree. This is clearly completely useless to anyone who has a different infra setup.

deadalnix requested changes to this revision.Tue, Jul 9, 01:13
This revision now requires changes to proceed.Tue, Jul 9, 01:13

If whatever is calling this dockerfile has these parameters, then wouldn't that be better to simply download the file and copy it within the docker image ?
I don't have much experience with teamcity, but this way of doing things by download an artifact over the internet seems HIGHLY suspicious and do not seem very necessary. British airways just learned that lesson the hard way. Everybody's laughing their ass off while doing the same.
In addition, one has to wonder what this does at the root of the source code tree. This is clearly completely useless to anyone who has a different infra setup.

The intent here was to not hide these details in the TeamCity configuration. The TeamCity config already hides some of this logic away from the source repo. It isn't right, but no one has the time to migrate them just quite yet.

There is a way to instruct the TeamCity agent to download the artifact before building the docker image, but again it was against the rationale in the short/mid term.

There are a lot of improvements that we're missing on this front (basically everything touched by TeamCity). I can revisit these once the bulk of our release process is automated (freeing up even more time to do so). Coincidentally, I'm learning more and more about how TeamCity works while I progress on these tasks. When the time comes to migrate all of the configs to in-repo, I believe the migration will be rather smooth. Until then, I think it's best we move forward on this low-impact side-task.

jasonbcox abandoned this revision.Tue, Jul 9, 05:45

Thought about it some more and realized there are other benefits to doing it the other way.