Tools: fix Cygwin CI build #29013
Merged
Tools: fix Cygwin CI build #29013
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
There are currently issues where the non-.exe-suffixed files can't be copied into the `artifacts` folder; `cp` claims "File exists". Previously this worked but the suffix was added by Cygwin so all files in `artifacts` had a `.exe` suffix anyway. This is evidently intended, though non-intuitive, behavior: https://sourceware.org/legacy-ml/cygwin/2009-08/msg00293.html > On Cygwin, you should avoid having a file "foo" and a file "foo.exe" > in the same directory at all cost to avoid puzzeling POSIX borderline > behaviour like this. What you do is essentially in the "not > supported" class of problems. > [...] Cygwin does not check for a file "foo", if the name of the file > is explicitely given as "foo.exe". Apparently something similar was addressed in PR ArduPilot#20926; the current code installs files with both suffixes, but that fix contradicts the info above and now has broken. This PR changes the code to only install .exe-suffixed files, as opposed to only non-.exe-suffixed files, which was the behavior before that PR.
robertlong13
approved these changes
Jan 5, 2025
Using the master branch exposed us to transient breakage as the action was developed.
|
Looks like the root cause for the change in observed behavior was us using the master branch of the Cygwin install action, which had a bug for a little while that caused it to install the testing versions of packages by default. It's now fixed in the master branch so CI will work again as is, but I've added another commit to use the latest released version (which includes the fix for that bug) instead. It's also probably best to fix our misuse of the .exe extension now. |
|
cygwin CI populates this: https://firmware.ardupilot.org/Tools/MissionPlanner/sitl/ |
tridge
approved these changes
Jan 7, 2025
|
this is included in ArduPilot-4.6.0-beta3 |
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.
There are currently issues where the non-.exe-suffixed files can't be copied into the
artifactsfolder;cpclaims "File exists". Previously this worked but the suffix was added by Cygwin so all files inartifactshad a.exesuffix anyway.This is evidently intended, though non-intuitive, behavior: https://sourceware.org/legacy-ml/cygwin/2009-08/msg00293.html
Apparently something similar was addressed in PR #20926; the current code installs files with both suffixes, but that fix contradicts the info above and now has broken.
This PR changes the code to only install .exe-suffixed files, as opposed to only non-.exe-suffixed files, which was the behavior before that PR. The set of installed files should not actually change.