The 32-bit Windows releases are 64-bit #1472
Comments
|
Thanks a lot for reporting this and for the initial investigation! The fact that this issue is created years after the first 64 bit binary disguised as 32 bit probably says a lot about the usage (or usefulness) of the Win32 builds. Thus it's probably a good idea to try and fix the issue, rather than remove 32 bit Windows builds entirely.
Maybe you could zip up a Please note that I usually copied my setups from |
|
I am thinking that actually making test releases is simpler that using a modified workflow that refrains from doing so, but I don't want those to come up as recommendations in GitHub for people, which would be confusing and could inadvertently lead to actual use of them. I could use a tagged version in my fork with something like I'm wondering, though, if maybe the amount of time it uses the runners is low enough to fix within the free minutes for a private repo, so that manual private reupload of the repository could make private releases, allowing me to test this in a near-equivalent setup, except of course that I would not be uploading anything to crates.io. |
|
That's very mindful of you! |
|
I had said:
Actually I spoke too soon, sort of. One of the problems affecting gitoxide's release workflow is that environment variables are not being set on Windows because notation like I've found that fixing just that problem--by using Git Bash--causes
Showing the command and error that happens for the So more than superficial changes may be required to fix this. As in the current ripgrep workflow, the I may decide to hew to the ripgrep approach. Although GitHub Actions workflows tend to be patched together from prior workflows in other projects and, at least socially speaking, the expectation to worry about licensing for them seems fairly minimal, the situation with ripgrep is even significantly more permissive, since one of the licenses the project is offered under is the Unlicense. So I may "sync" the workflow with that. If I do, I'll note this at least in a commit message. If doing so doesn't break anything, then I think such an overhaul could have further benefits. One of those benefits, related to the comment discussion in #1239, could be to facilitate more builds, including those requested in #1242.
I don't know how long it will take to do this, and I may want to keep the testing repository for future related use, as well as to avoid prematurely deleting GitHub Actions logs in case some of the information there proves relevant. The repo would not technically be a fork in the GitHub sense, since I can only have one (without making an organization or using a second account) and I don't want to delete the fork I've been using to contribute to I am furthermore not confident that people would not use releases from a fork, even if it were a real GitHub fork. Based on the way GitHub places release announcements in feeds, people who want a prerelease hoping it may fix some issue may not notice, and visiting the repository would show things like your avatar for sponsoring (unless removed) that could create the wrong impression that it is official. Also, a number of forks do release new versions that are meant to be used, either because the fork officially supersedes its parent repo (e.g. vscode-python), or because the fork unofficially publishes a modified version that is useful for a while before the upstream project gains a feature or build target (which is something I have done). So far I've been able to test in the private repo. If I get close to running out of free private minutes, I can pivot to testing the techniques and workflow in one of my own tiny Rust projects and then applying it to gitoxide. It may not be a bad idea anyway to test out release logic in my own projects that have few or no users before |
|
Thanks for looking into this more, making releases work properly (again) would be great, and adding more targets would be even better as a side-effect. |
I've opened #1475 for this.
I did not add any more targets, but that's something that should be easier to do now. There are major conditions where it should mostly "just work," as noted in the PR. But I think some possible future targets of interest might still present some difficulties.
I believe #1475 fixes that as well, though the only change clearly related to it is to the glob pattern that tags have to match to produce a release, and it looks like it's even something you had tried before. But in the private fork I used for testing, pushing was sufficient to trigger the workflow staring in the commit that made that change. |
This adds the `if:` key from the ripgrep workflow to the "Use Cross" step, so that we only use `cross` instead of `cargo` when both of the following hold: (a) We are on a Linux runner. (b) It is possible that we are cross-compiling. Of those conditions, (a) is more important than (b), since `cross` can be used even when not cross-compiling. However, on Windows, it fails to create the environment, because there is no Docker image available for it, and also it is a Windows environment so Windows containers would have to be used, which are not available. This would also not currently be able to work on Windows hosted GHA runners, because they don't support nested virtualization. Although using `cross` on Windows in this workflow had never worked properly, there were two different incorrect behaviors: - It was previously not actually being used at all, because PowerShell rather than bash was being used as the shell, and the attempt to use syntax like `>> $GITHUB_ENV` in PowerShell did not succeed at setting the `CARGO` environment variable (nor others). Although this is *conceptually* related to the bug where the 32-bit (i686) Windows release archives actually contained 64-bit binaries (GitoxideLabs#1472), it does not seem to have been the direct cause, since cross-compiling with `cargo` should have worked. But the related failure to set the `$TARGET_FLAGS` and `$TARGET_DIR` environment variables was an important factor. This was one factor responsible for the bug where the 32-bit - With bash used for all script steps on all operating systems, it failed early with an error from `docker` about not finding an image on ghcr.io corresponding to the target. For some further details, see: GitoxideLabs#1472 (comment)
Current behavior 😯
On the GitHub release pages where binaries can be downloaded, the four archives named to indicate that they are
i686-pc-windows-msvcbuilds are actually allx86_64-pc-windows-msvcbuilds.I don't believe this is specific to recent versions. But in the current latest version the affected archives are:
Downloading these releases from a releases page or using
gh release download, or attempting to automatically download and install them usingcargo binstall, obtains 64-bit executables instead.This problem appears specific to the 32-bit Windows releases. Other releases all appear to be of the claimed architecture.
The cause?
The release workflow contains
win32-msvcjobs that are meant to build and produce archives of 32-bit Windows executables. But this is producing archives of 64-bit executables instead.I'm not sure exactly what is going on. It seems to me from the build output that those jobs' behavior may be accidentally equivalent in effect to that of the
win-msvcjobs--that is, they may be building the wrong target. But I may just be reading the build output wrong.However, separately from the compilation itself, it looks like there is a bug in the "Build archive" step, where it accounts for the target that was built on Unix-like systems but does not account for it on Windows:
https://github.com/Byron/gitoxide/blob/55cffe4ecf78bfea76059b742fafaebe964a725f/.github/workflows/release.yml#L203-L211
That shows that, on Windows, the files being copied for inclusion in the archive are specified as:
target/release/${{ env.EXE_NAME }}.exe target/release/gix.exeWhile, in contrast, on other operating systems, they are specified as:
If that is not the full cause, or the needed fix is not immediately apparent when you look at it, then I would be pleased to investigate further and try to fix it.
I am not sure of the best way is to experiment with this, but it looks feasible. In my fork, I could remove the "Upload release archive" step check the output in some other way. Or if necessary I could leave it in and publish "releases" from my fork while testing.
Expected behavior 🤔
The releases for Windows marked
i686should target that architecture rather thanx86_64.(If some builds with particular feature configuration cannot be produced, then they can simply be omitted. But I suspect they all can be built. In particular, rust-lang/libz-sys#197 does not seem to affect local cross-compilation from a 64-bit Windows machine.)
Git behavior
32-bit builds of Git for Windows offered for download are 32-bit, targeting i686.
Steps to reproduce 🕹
This can be verified manually, by attempting to install a 32-bit build with
cargo binstalland examining the result, or semi-automatically by downloading all archives of a release and checking which of them match the archive names.Demonstration with
cargo binstallThis is from a 32-bit Windows 10 system:
Demonstration checking binaries in all archives of a release
Downloading all archives of a release can be done with the
ghcommand. I did the following in PowerShell on a Windows system with 7-Zip installed (providing the7zcommand) and with the MSYS2filecommand made available in thePATH:This confirmed that all
i686-pc-windows-msvcarchives were affected, with their executables beingx86_64rather thani686, and that no other archives were affected. The full transcript is in this gist. The most useful part is the output of the last command, which is:The text was updated successfully, but these errors were encountered: