-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Rewrite PowerShell CI build script in Bash #16252
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script is unrelated, but I noticed it's not used or referenced anywhere. This is a refactor PR anyway, so I thought I'd include it.
f1e9abb to
1965c8a
Compare
scripts/ci/build_win.sh
Outdated
| # Use last commit date rather than build date to avoid ending up with builds for | ||
| # different platforms having different version strings (and therefore producing different bytecode) | ||
| # if the CI is triggered just before midnight. | ||
| TZ=UTC git show --quiet --date="format-local:%Y.%-m.%-d" --format="${prerelease_source}.%cd" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Argh... For some reason %-m and %-d do not seem to be supported on Windows. Only %m and %d work. They result in git silently exiting with code 127.
We need those to get a version string like v0.2.5-ci.2021.1.2 rather than v0.2.5-ci.2021.01.02 (i.e. no leading zeros, so the date can pass for a version number). While I don't necessarily like this format, we are using it already, so we need to be able to match it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like date does support this format. So what I ended up doing is to parse it into a date and then format back, without leading zeros. More annoying but at least portable.
542615f to
bcb67b0
Compare
bcb67b0 to
9327b28
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approve with minor optional comment
|
Bash is now supported on Windows? How does this work in the first place? Does it use the Ubuntu subsystem? |
9327b28 to
e4421c0
Compare
No, I think it's simply what comes with Git for Windows. MSYS2 I guess? Or some other implementation - there have been several implementations of GNU tools and shells floating around since forever. In any case, CircleCI has always had |
Cygwin I as well I guess - it seems very nicely integrated, weird that we haven't done it before. |
|
Well, I think it's not Cygwin. AFAIK Cygwin has always been a bigger, slower and more integrated thing. MSYS has fewer pieces. But I guess it still has some relation to Cygwin in the end. Maybe it's sharing pieces with it. |
Prerequisite for #16246.
Having to research PowerShell syntax each time the the Windows version has to be touched and test it separately via SSH has always been a major pain. This finally unifies our CI builds scripts on all platforms.