forked from git/git
-
Notifications
You must be signed in to change notification settings - Fork 141
[DO-NOT-SUBMIT-YET] ci: cache vcpkg artifacts #879
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
Draft
dscho
wants to merge
9
commits into
gitgitgadget:master
Choose a base branch
from
dscho:ci-cache-vcpkg-artifacts
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In our continuous builds, Windows is the odd cookie that requires a complete development environment to be downloaded because it is not installed by default. Side note: technically, there _is_ a development environment: MSYS2. But it differs from Git for Windows' SDK in subtle points, enough so to prevent Git's test suite from running without failures. Traditionally, we support downloading this environment (which we nicknamed `git-sdk-64-minimal`) via a PowerShell scriptlet that accesses the build artifacts of a dedicated Azure Pipeline (which packages a tiny subset of the full Git for Windows SDK, containing just enough to build Git and run its test suite). This PowerShell script is unfortunately not very robust and sometimes due to network issues. Instead of doing all of this in Git's own `.github/workflows/`, let's offload this logic to the brand-new GitHub Action at https://github.com/marketplace/actions/setup-git-for-windows-sdk This Action not only downloads and extracts git-sdk-64-minimal _outside_ the worktree (making it no longer necessary to meddle with `.gitignore`), it also adds the `bash.exe` to the `PATH` and sets the environment variable `MSYSTEM` (an implementation detail that Git's workflow should never have needed to know about). This allows us to convert all those funny PowerShell tasks that wanted to call git-sdk-64-minimal's `bash.exe`: they all are now regular `bash` scriptlets. This finally lets us get rid of the funny quoting and escaping where we had to pay attention not only to quote and escape in the Bash scriptlets properly, but also to add a second level of escaping (with backslashes for double quotes and backticks for dollar signs) so that PowerShell would not do unintended things. Further, this Action uses a fast caching strategy native to GitHub Actions that is not only very fast, but should accelerate the download across CI runs: git-sdk-64-minimal is usually updated once per 24h, and needs to be cached only once within that period. With this we can drop the homerolled caching where we try to accelerate the test phase by uploading git-sdk-64-minimal as a workflow artifact after using it to build Git, and then download it as workflow artifact in the test phase. Even better: the `vs-test` job no longer needs to depend on the `windows-build` job. The only reason it depended on it was to ensure that the workflow artifact was available. Signed-off-by: Johannes Schindelin <[email protected]>
We use a `.bat` script to copy the DLLs in the `vs-build` job, and those type of scripts are native to CMD, not to PowerShell. Signed-off-by: Johannes Schindelin <[email protected]>
The GitHub Actions to upload/download workflow artifacts saw a major upgrade since Git's GitHub workflow was established. Let's use it. Signed-off-by: Johannes Schindelin <[email protected]>
Git's test suite is excruciatingly slow on Windows, mainly due to the fact that it executes a lot of shell script code, and that's simply not native to Windows. To help with that, we established the pattern where the artifacts are first built in one job, and then multiple test jobs run in parallel using the artifacts built in the first job. We take pains in transferring only the build outputs, and letting `actions/checkout` fill in the rest of the files. One major downside of that strategy is that the test jobs might fail to check out the intended revision (e.g. because the branch has been updated while the build was running, as is frequently the case with the `seen` branch). Let's transfer also the files tracked by Git, and skip the checkout step in the test jobs. Signed-off-by: Johannes Schindelin <[email protected]>
We already build Git for Windows with `NO_GETTEXT` when compiling with GCC. Let's do the same with Visual C, too. Signed-off-by: Dennis Ameling <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
By upgrading from v1 to v2 of `actions/checkout`, we avoid fetching all the tags and the entire history: v2 only fetches one revision by default. This should make things a lot faster. Note that `actions/checkout@v2` seems to be incompatible with running in containers: actions/checkout#151. Therefore, we stick with v1 there. Signed-off-by: Johannes Schindelin <[email protected]>
It is much easier to read the invocation using `curl` and `jq` for non-PowerShell experts. Signed-off-by: Johannes Schindelin <[email protected]>
The vcpkg artifacts are rebuilt only once per week, therefore it makes sense to cache them closer to where the GitHub workflow runs. Signed-off-by: Johannes Schindelin <[email protected]>
…roblem There is currently a problem with actions/cache@v2: when specifying a path that contains slashes (such as `compat/vcbuild/vcpkg`, which we now do in the CI/PR builds), on Windows they are converted to backslashes _and_ it uses the `tar.exe` found in the PATH. And since we install git-sdk-64-minimal, there is a GNU tar in the PATH that thinks that backslashes are a perfectly fine file name characters, not directory separators (the MSYS2 runtime will translate file name characters that are legal on Linux/Unix but illegal on Windows, mapping them into the private Unicode page). As a consequence, it fails to tar up the vcpkg files. Version 2.1.4 of actions/cache fixes these problems, by converting the backslashes back to forward slashes _and_ by insisting on using the `tar.exe` found in `C:\Windows\system32`. So: problem solved, right? Not so fast. An _unrelated_ bug was introduced in that version (actions/[email protected] now uses GNU tar on macOS, which seems to be unable to restore certain data shapes that were previously cached using BSD tar, see actions/cache#527 for details) and therefore v2 was rolled back to point to v2.1.3 for the moment. So let's hard-code v2.1.4 for now, hoping that the issue will be resolved soon. Signed-off-by: Johannes Schindelin <[email protected]>
b8f3085
to
232c531
Compare
Better use https://github.com/git-for-windows/get-azure-pipelines-artifact (and possibly merge the patches to Git for Windows early, so that we can stabilize that Action to v1 before sending this patch off to the Git mailing list). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This not only shaves off some 40 seconds off of every
vs-build
run, it also makes it a lot more robust: these downloads fail from time to time, and if we keep restoring the cached artifacts for a week (they are built once a week), there is a lot less chance for those network problems to let builds fail.This PR is blocked by a bug in actions/cache; We have a workaround with b8f3085 but it would be nicer if we didn't need it. Let's see how quickly that issue is getting resolved.