Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using Docker to build #82

Merged
merged 1 commit into from
Apr 12, 2016
Merged

Allow using Docker to build #82

merged 1 commit into from
Apr 12, 2016

Conversation

domenic
Copy link
Member

@domenic domenic commented Feb 10, 2016

Closes #55. Supercedes whatwg/html#612.

@defunctzombie, this is what I came up with after the discussions on whatwg/html#612. I think we may eventually consider harder merging the html-build and html repos, but until we do, this seems to fit better with the current infrastructure. I think we do all appreciate though your perspective that we are making life harder on contributors by not letting them just clone one repo and go.

Notably, this setup allows us to reuse ./build.sh as the main entry point, which does things like check for updates and guide the users through cloning whatwg/html. It thus helps hide the potentially-scary docker commands behind a nicer facade.

I'd appreciate any review, both from @defunctzombie for the mutilations I made to his beautiful setup, and from other editors who are willing to give this a shot, or review the new README.md.

Also, if anyone has ideas on how to best solve the

# TODO: pass in $DO_UPDATE, $QUIET, and $VERBOSE?

that'd be swell. (Basically, if those are set in the outer invocation of build.sh, then the Dockerfile should pass the corresponding --no-update, --quiet, or --verbose flags onward. I can imagine several ways to do this, all fairly inelegant.)

@defunctzombie
Copy link
Contributor

LGTM if it produces the result you want!

@domenic domenic force-pushed the dockerize branch 2 times, most recently from 448aebc to c998d19 Compare February 19, 2016 21:45
@domenic
Copy link
Member Author

domenic commented Feb 19, 2016

@defunctzombie I tried to update this to pass the --no-update flag along, which (when used locally) says to avoid doing internet downloads in favor of using the stuff in html-build's .cache subdirectory. However, this doesn't seem to work. The flag gets passed through, but every time the build script runs inside the container, the .cache directory is empty.

This kind of makes sense; it's trying to be stateless between runs, I guess. It's not a persistent VM I get to boot up and play in every run. But do you have any advice on this pattern?

@defunctzombie
Copy link
Contributor

The way docker layer caching works is by looking at what changed in a given layer. This could be the layer run command or the files copied during a layer.

For example the following layer ADD . /whatwg/build will invalidate everything after it if any file in the current build dir (that is not ignored) changes.

Likewise ADD $html_source_dir /whatwg/html invalidates when any html source changes.

Since your build script runs after both of these lines it will be run every time any source or build file changes.

I suspect what you really want is to run some version of the build script that downloads only the dependencies after the ADD . /whatwg/build line but before the html source is added. In this way, the .cache directory is populated after any build files change and also available for when the source is built with the final build.sh run.

@domenic
Copy link
Member Author

domenic commented Feb 24, 2016

That makes a lot of sense, thanks. I'll open that as a new possibility.

In the meantime, I am using this day-to-day on my Windows machine and loving it. I'd like to get it merged. Do any other maintainers want to try it out and report back? Or at least just review the changes to the README?

@sideshowbarker
Copy link
Contributor

I’ll make time to try this out over the next couple days but would not be unhappy if somebody else beat me to it :) (and very glad to see this getting ready to land)

@domenic
Copy link
Member Author

domenic commented Apr 12, 2016

I'm going to merge this. I've been using it successfully for months in order to build on Windows :).

@domenic domenic merged commit 9f25dca into master Apr 12, 2016
@domenic
Copy link
Member Author

domenic commented Apr 12, 2016

Uhhhh GitHub merge UI didn't give @defunctzombie credit, force-pushing better merge...

@domenic domenic deleted the dockerize branch June 7, 2017 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants