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.

Sign up for GitHub

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

Jump to bottom

.github: build vs-build with NO_GETTEXT #3040

Merged
merged 7 commits into from
Mar 11, 2021
Merged

.github: build vs-build with NO_GETTEXT #3040

merged 7 commits into from
Mar 11, 2021

Conversation

dennisameling
Copy link

As discussed in #3017 (comment), removes the dependency on git-sdk-64-minimal in vs-build by changing to NO_GETTEXT.

@dennisameling dennisameling mentioned this pull request Feb 11, 2021
Merged
@dscho
Copy link
Member

dscho commented Feb 11, 2021

Hmm. It looks as if we require make.exe, still, if only to bundle the build artifacts...

  & git-sdk-64-minimal\usr\bin\bash.exe -lc @"
   mkdir -p artifacts &&
   eval \"`$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts 2>&1 | grep ^tar)\"
 "@

How about using the same cache trick you provided for the git-artifacts.yml workflow, just for git-sdk-64-minimal?

@dennisameling
Copy link
Author

Will have a look at this tomorrow towards the end of the day!

@dscho
Copy link
Member

dscho commented Feb 16, 2021

Will have a look at this tomorrow towards the end of the day!

Actually, I thought about it, and it might be a perfect time to get a GitHub Action started for that. Let me try that.

@dscho
Copy link
Member

dscho commented Feb 18, 2021

[...] it might be a perfect time to get a GitHub Action started for that. Let me try that.

Took long enough: https://github.com/git-for-windows/setup-git-for-windows-sdk. It does not yet implement caching, but I think that'll be next.

@dennisameling
Copy link
Author

Very fancy, love it! 🚀 Let me try 👀

@dscho
Copy link
Member

dscho commented Feb 18, 2021

Very fancy, love it! 🚀 Let me try 👀

Are you familiar with Typescript, Jest & Mocking? I would love to have somebody look over this (I have a feeling that this is hodgepodge code, and could be done much more elegantly): https://github.com/git-for-windows/setup-git-for-windows-sdk/blob/f6f78470e56fadd03f2175101f627aacd6362602/__tests__/downloader.test.ts#L51-L77

@dscho
Copy link
Member

dscho commented Feb 18, 2021

It does not yet implement caching, but I think that'll be next.

Here it is: git-for-windows/setup-git-for-windows-sdk#8

@dennisameling dennisameling mentioned this pull request Feb 18, 2021
Merged
@dennisameling
Copy link
Author

I have a feeling that this is hodgepodge code, and could be done much more elegantly

Done in git-for-windows/setup-git-for-windows-sdk#11 😊 hope that helps!

@dscho
Copy link
Member

dscho commented Feb 18, 2021

The good news: I cherry-picked this commit into gitgitgadget#878. The bad news: the Action does not yet do what it is supposed to do...

@dennisameling dennisameling force-pushed the vs-build-no-gettext branch 2 times, most recently from 28c9dbc to ef908c3 Compare February 18, 2021 16:39
@dennisameling
Copy link
Author

Just added your shiny new GH Action in ef908c3 and it seems to work: https://github.com/git-for-windows/git/pull/3040/checks?check_run_id=1928729677

dscho and others added 6 commits February 19, 2021 13:43
@dscho
ci: use the brand-new GitHub Action to download git-sdk-64-minimal
0622b62 
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 <johannes.schindelin@gmx.de>
@dscho
ci (vs-build): use cmd to copy the DLLs, not powershell
49c51e5 
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 <johannes.schindelin@gmx.de>
@dscho
ci: upgrade to using actions/{up,down}load-artifacts v2
38e41e6 
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 <johannes.schindelin@gmx.de>
@dscho
ci(windows): transfer the tracked files to the test jobs
6b6bf80 
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 <johannes.schindelin@gmx.de>
@dennisameling @dscho
ci(vs-build): build with NO_GETTEXT
23eecab 
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 <dennis@dennisameling.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
ci: accelerate the checkout
c7774e7 
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 <johannes.schindelin@gmx.de>
@dscho
Copy link
Member

dscho commented Feb 19, 2021

What would you think of force-pushing gitgitgadget#878's branch to this PR, since it essentially includes your work (but splits concerns into multiple commits, and also adds additional speed-ups)?

@dscho
Copy link
Member

dscho commented Feb 25, 2021

What would you think of force-pushing gitgitgadget#878's branch to this PR, since it essentially includes your work (but splits concerns into multiple commits, and also adds additional speed-ups)?

@dennisameling if you're okay with going forward with this, I can do it for you... I just don't want to step on your toes.

@dennisameling
Copy link
Author

Hi @dscho, I've been swamped with work in the last days, sorry. Please, if you have time, feel free to go ahead with this one 😊 I hope to have some time during the weekend as well if that helps!

@dennisameling
Copy link
Author

@dscho I think it's easier to just close this PR and create a new one from your branch https://github.com/dscho/git/tree/use-setup-git-for-windows-sdk-action. What do you think? Please feel free to close this one 👍🏼

@dscho
Copy link
Member

dscho commented Mar 9, 2021

Force-pushing the branch to this PR would have the advantage for me that I would save me some time... 😄

@dennisameling dennisameling force-pushed the vs-build-no-gettext branch 3 times, most recently from 62be51c to c7774e7 Compare March 11, 2021 10:32
@dennisameling
ci(vs-build): re-apply Git for Windows-specific changes
fc590b3 
Git for Windows's vs-build job supports both Windows x64 and arm64.
This commit re-enables that support after changes were done to the
gitgitgadget repo.
git-for-windows-ci pushed a commit that referenced this pull request Jun 26, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
842ecc6 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Jul 2, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
9d4440a 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 2, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
17bf488 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 5, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
84d5c64 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 5, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
f0c33d3 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 5, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
2a5636a 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 5, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
6b6f162 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 5, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
89b6599 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 5, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
6b27cc1 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 6, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
e300f89 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 6, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
37415fa 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 6, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
4401746 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Jul 10, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
da5835f 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Jul 10, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
af54dcb 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Jul 14, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
f75a75d 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 15, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
6c03960 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 15, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
24c5dcc 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 19, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
89aa124 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 19, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
f250c98 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Jul 19, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
2bf5f6a 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 24, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
7804634 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Jul 29, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
b8c03dd 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Aug 2, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
f870a05 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Aug 3, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
3d058e4 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Aug 11, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
1d0fd99 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Aug 13, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
2002e8c 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Aug 18, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
9066dc2 
.github: build vs-build with NO_GETTEXT
dscho added a commit that referenced this pull request Aug 23, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
e2619a8 
.github: build vs-build with NO_GETTEXT
git-for-windows-ci pushed a commit that referenced this pull request Aug 24, 2021
@dscho
Merge pull request #3040 from dennisameling/vs-build-no-gettext
21d4492 
.github: build vs-build with NO_GETTEXT
Sasha2936

This comment was marked as off-topic.

Sign in to view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sasha2936 Sasha2936 Sasha2936 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.31.0
Development

Successfully merging this pull request may close these issues.
3 participants
@dennisameling @dscho @Sasha2936