-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix improper capturing of mask into package
We have observed packages getting built containing a file removal mask, despite `test_package_with_circular_dep_does_not_collect_file_removals` attempting to test correct behavior in this area. It turns out that a specific setup is required to exhibit the incorrect behavior. When the result of a build is being diffed to figure out what changes are to be captured into the new package, the "environment_filesystem" Manifest was being incorrectly calculated. If multiple layers in the Solution contain entries for the same subdirectory, only the latest layer that does so would be used to represent what the "before" contents were for that directory. Consider one package that puts a file into "subdir/" like "subdir/foo.txt", and another package that simply creates an empty directory called "subdir/". The order that layers end up appears in a Solution is arbitrary, so with this setup it is possible for the Manifest to end up with an empty "subdir/" directory, whereas the true build environment would have contained the "subdir/foo.txt" file entry. In our environment we have a package called "stdfs" which contains and empty "lib/" subdirectory. When this package's entries are walked, its empty "lib/" directory would overwrite any contents that had already been encounted under "lib/". In our problem case, that included a _deletion_ of a subdirectory under "lib/". Since the "before" contents ended up not containing the entry that was deleted by the build, when diffing the before and after, the after would include a diff event that "Added" a new mask entry, and its presence in the diff would allow it to be collected into the newly built package. One "fix" for this was to modify `diff_path` so it detects this (unexpected) situation and ignore mask entries when the mask is deleting something that didn't exist in the "before" Manifest. But this problem is a symptom of the "before" Manifest being incorrect. The proper fix was to change the "environment_filesystem" calculation to merge two Trees when Trees with the same path appear in multiple layers. In order to properly test the expected behavior, the "environment_filesystem" calculation was refactored out into its own method. It was moved to `ResolvedLayers` since practically none if its logic was related to `BinaryPackageBuilder`. The recursive package test was updated in such a way that it would actually fail without these changes, however the problem ended up not being specific to recursive packages. A new test was added to specifically test the Tree merge behavior. Fixes #1013. Signed-off-by: J Robert Ray <jrray@jrray.org>
- Loading branch information
Showing
9 changed files
with
248 additions
and
99 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
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
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
Oops, something went wrong.