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.

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

[plugin] Fix #505 : resources are not included in the archive on Linux #506

Merged
merged 4 commits into from
Mar 17, 2025

Conversation

sebsto
Copy link
Contributor

@sebsto sebsto commented Mar 13, 2025

Resources are not included in the archive on Linux
#505

In addition, the script that checks if the resources are correctly included in an archive when building on Linux is not working.
It failed to detected missing resources.

This patch fix both the plugin and the CI script

@sebsto sebsto added the 🔨 semver/patch No public API change. label Mar 13, 2025
@sebsto sebsto requested a review from fabianfett March 13, 2025 19:52
@sebsto sebsto self-assigned this Mar 13, 2025
@sebsto
Copy link
Contributor Author

sebsto commented Mar 13, 2025

The unit test "PR / Integration Tests / Test archive plugin (ResourcesPackaging) (pull_request)" fails. This is expected

@sebsto sebsto changed the title Fix integration test Fix #505 : resources are not included in the archive on Linux Mar 13, 2025
@sebsto
Copy link
Contributor Author

sebsto commented Mar 13, 2025

The PR now fixes both the CI script and the plugin.

@sebsto sebsto changed the title Fix #505 : resources are not included in the archive on Linux [plugin] Fix #505 : resources are not included in the archive on Linux Mar 14, 2025
unzip -l "${ZIP_FILE}" | grep --silent hello.txt
SUCCESS=$?
if [ "$SUCCESS" -eq 1 ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know these two lines can be merged as one but I favored readability over concision

Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sebsto sebsto merged commit 7322a36 into swift-server:main Mar 17, 2025
30 of 31 checks passed
@sebsto sebsto deleted the sebsto/fix_resource_check branch March 17, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants