Skip to content

engine: set include and exclude globs deterministically#8471

Merged
marcosnils merged 1 commit into
dagger:mainfrom
marcosnils:fix/include_exclude_deterministic_order
Sep 17, 2024
Merged

engine: set include and exclude globs deterministically#8471
marcosnils merged 1 commit into
dagger:mainfrom
marcosnils:fix/include_exclude_deterministic_order

Conversation

@marcosnils
Copy link
Copy Markdown
Contributor

@marcosnils marcosnils commented Sep 16, 2024

change includeSet and excludeSet to use a slice without duplicates given
that the order of the globs is important when calling buildkit.

Signed-off-by: Marcos Lilljedahl marcosnils@gmail.com

@marcosnils marcosnils force-pushed the fix/include_exclude_deterministic_order branch from 9ad88fe to 86f2188 Compare September 17, 2024 00:06
@marcosnils marcosnils marked this pull request as ready for review September 17, 2024 00:07
@marcosnils marcosnils requested review from jedevc, sipsma and vito and removed request for sipsma September 17, 2024 00:07
change includeSet and excludeSet to use a slice without duplicates given
that the order of the globs is important when calling buildkit.

Signed-off-by: Marcos Lilljedahl <marcosnils@gmail.com>
@marcosnils marcosnils force-pushed the fix/include_exclude_deterministic_order branch from 86f2188 to da1b8de Compare September 17, 2024 00:11
Copy link
Copy Markdown
Contributor

@sipsma sipsma left a comment

Choose a reason for hiding this comment

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

LGTM, this looks like it addresses part of what I mentioned in this comment: #8254 (comment)

Not sure if that entire issue is solved yet, but probably at minimum improved 🎉


It would be nice to have a test here, but it's really quite difficult to test in a deterministic way, so I'm fine merging as is for now.

@marcosnils marcosnils merged commit 7291d32 into dagger:main Sep 17, 2024
@marcosnils marcosnils deleted the fix/include_exclude_deterministic_order branch September 17, 2024 02:40
@gerhard gerhard added this to the v0.13.1 milestone Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants