Skip to content

Fix incremental builds for embedInCode resources #7616

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

kateinoigakukun
Copy link
Member

SwiftPM's incremental build did not detect changes in embedded resources.

Motivation:

SwiftPM's embedInCode feature did not detect changes in the resource files and it led to using old resource contents even though they were modified.

You can reproduce it with the fixture in this repository like below:

$ rm -rf Fixtures/Resources/EmbedInCodeSimple/.build
$ swift run --package-path Fixtures/Resources/EmbedInCodeSimple
hello world

$ echo "update 1" > Fixtures/Resources/EmbedInCodeSimple/Sources/EmbedInCodeSimple/best.txt
$ swift run --package-path Fixtures/Resources/EmbedInCodeSimple
update 1

$ echo "update 2" > Fixtures/Resources/EmbedInCodeSimple/Sources/EmbedInCodeSimple/best.txt
$ swift run --package-path Fixtures/Resources/EmbedInCodeSimple
update 1

The first update ("update 1") is reflected in the final executable output because:

  • The embedded_resources.swift is generated while computing build descriptions
  • Build descriptions are always re-computed in the 2nd build iteration to build PackageStructure build cache

However, the second update ("update 2") is not updated because we don't re-compute build descriptions in this stage, and the llbuild build system does not track embedding resource files.

Modifications:

Generate embedded_resources.swift during llbuild build step instead of during computing build descriptions.

Result:

In this way, llbuild build system can correctly detect resource file modifications in any build iterations.

@kateinoigakukun
Copy link
Member Author

@swift-ci test

@kateinoigakukun kateinoigakukun added build system Changes to interactions with build systems bug labels May 31, 2024
@kateinoigakukun
Copy link
Member Author

@swift-ci test macOS

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Thanks!

@MaxDesiatov
Copy link
Contributor

@swift-ci test macos

@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@MaxDesiatov
Copy link
Contributor

Please do cherry-pick this for 6.0 with the usual release branch PR description template 🙏

@kateinoigakukun
Copy link
Member Author

CI is broken until the next branch of apple/llvm-project merges llvm/llvm-project@fecf5c7

@kateinoigakukun
Copy link
Member Author

@swift-ci test macOS

@kateinoigakukun
Copy link
Member Author

@swift-ci test Windows

@kateinoigakukun kateinoigakukun merged commit d33814b into swiftlang:main Jun 1, 2024
5 checks passed
@kateinoigakukun kateinoigakukun deleted the yt/fix-incremental-embedincode branch June 1, 2024 11:51
kateinoigakukun added a commit that referenced this pull request Jun 4, 2024
**Explanation**: Fix a build system hole that incremental build did not
detect changes in embedded resources.
**Scope**: Incremental build with `embedInCode` feature
**Risk**: Low. 
**Testing**: Added a new test to cover the incremental build case.
**Original PR**:
#7616
**Reviewers**: @MaxDesiatov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug build system Changes to interactions with build systems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants