Add round-trip tests for precompiled gems #76
Merged
Conversation
This file contains 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
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
16 times, most recently
from
7671e6b to
46112b8
Compare
Merged
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
2 times, most recently
from
d023bf2 to
4852100
Compare
This was referenced Jul 17, 2023
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
3 times, most recently
from
e983957 to
a9fdc8e
Compare
stanhu
commented
Jul 17, 2023
stanhu
commented
Jul 17, 2023
This was referenced Jul 17, 2023
Closed
|
@mudge This is now passing all tests! Getting Windows working was a lot trickier than I had expected. |
mudge
reviewed
Jul 18, 2023
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
from
a9fdc8e to
8d2b685
Compare
This pulls in flavorjones/mini_portile#129. When cross-compiling Windows targets with cmake in a Linux environment, the MSYS generator may not be available. Supplying -G MSYS will cause the build to fail.
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
from
8d2b685 to
840b822
Compare
This pulls in flavorjones/mini_portile#130. We no longer need to write custom logic for finding the right compilers and setting CMake variables for cross-compilation.
This ensures the generated gem has a unique version number, so define `RE2::VERSION` in a separate file to make this possible.
Previously sence `rake spec` required the `compile` step, `scripts/test-gem-install` failed because `extconf.rb` was removed. Fix this by making the default task depend on the `compile` task, but the `spec` task independent of the `compile` task.
This ensures that all precompiled gems are built properly and can be run on their target operating systems. Closes mudge#69
This reduces the filename lengths since Windows can hit a limit of 250 characters (CMAKE_OBJECT_PATH_MAX).
These are required dependencies for abseil until a custom musl gem is built: mudge#67 (comment)
On systems where pkgconf v1.9.3 is used for pkg-config, the output of `pkg-config --static --libs` is incorrect, resulting in build failures (pkgconf/pkgconf#268). This commit fixes the issue by hard-coding the abseil libraries needed to link the extension properly.
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
from
840b822 to
6985843
Compare
Rather than remove all files and conditionally guard against version.rb load errors, just include the required files for testing in the gem and test against the installed directory.
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
from
3fa1bb4 to
0bec72c
Compare
|
@flavorjones if you have any availability, it’d be great if you could cast a critical eye on this too. |
flavorjones
reviewed
Jul 21, 2023
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
from
54a35a6 to
43e2d36
Compare
flavorjones
reviewed
Jul 22, 2023
| with: | ||
| name: cruby-x64-mingw-ucrt-gem | ||
| path: gems | ||
| - run: ./scripts/test-gem-install gems |
There was a problem hiding this comment.
I'm curious how this is running without specifying shell: bash? I expected this to fail. I wonder if there's been some shebang magic introduced on the windows images (or if github changed the default behavior?)
There was a problem hiding this comment.
Hmm, I'm not sure https://github.com/mudge/re2/actions/runs/5596591577/job/15159473313 actually ran. Maybe since ErrorActionPreference=Stop wasn't set, the job didn't actually fail.
There was a problem hiding this comment.
https://github.com/mudge/re2/actions/runs/5631492710/job/15258525260 did run, so I suspect this was a exit code issue.
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
from
43e2d36 to
06bbaa1
Compare
This avoids the visibility setting warnings in the linker (https://github.com/mudge/re2/actions/runs/5631492710/job/15258506852) and reduces the size of the binaries.
stanhu
force-pushed
the
sh-add-precompiled-ci-tests
branch
from
eef907e to
f42a93d
Compare
flavorjones
approved these changes
Jul 24, 2023
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 ensures that all precompiled gems are built properly and can be run on their target operating systems.
In addition, this ensures the Ruby platform gem can be installed and tested on all platforms.
This also fixes a number of build and install issues on Windows, such as pkgconf/pkgconf#268.
Closes #69 and #70