You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by "abderrahim (via GitHub)" <gi...@apache.org> on 2024/08/02 11:00:31 UTC
[PR] _context: use artifact caches from the parent project for junctions [buildstream]
abderrahim opened a new pull request, #1941:
URL: https://github.com/apache/buildstream/pull/1941
This makes a junction use the artifact cache of the parent project before the ones defined for the junction
This was originally done in 24c0de16faec2b8b9bd6a03504ce951dc49afbe2, but regressed at some point
Fixes https://github.com/apache/buildstream/issues/1839
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-4938572578
The above niche case I mentioned doesn't actually work without serious user config trickery as BuildStream will (afaik) push parent project artifacts to child project cache anyway bloating it with unused artifacts. But might be worth a check from implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-5145406426
@jjardon let's start lobbying distros to pull this once released in a version. I reckon many might use distro versions of BuildStream.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-2538514307
I just thought of one case that might be affected. Currently child project can have a tiny cache that only fits its own artifacts and I parent project artifacts will be pulled from parent project cache instead. This is disk storage vs network problem. Usually disk storage problem is of course easier to resolve than network. So maybe the ideal solution would be allow-parent-project-artifacts with default value True that can optionally be set to False if you really know what you're doing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "jjardon (via GitHub)" <gi...@apache.org>.
jjardon commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-4938429038
@juergbi @abderrahim can we please reconsider this? Default is increasing the pressure in fdsdk as more projects are using GNOMEOS and other fdsdk downstreams as a base
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-5146521647
@juergbi looks like crashes in CI?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-2271151997
commit 24c0de16faec2b8b9bd6a03504ce951dc49afbe2 was intentionally reverted as part of https://gitlab.com/BuildStream/buildstream/-/merge_requests/1403.
The remote cache configuration was later redesigned in #1434.
I have to dive deeper to figure out whether this is a correct bug fix, but want to make sure we don't unintentionally change the behavior. And we would also need to consider whether this behavior change may break any users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-5144411359
I pushed a clean rebase to trigger up-to-date CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim merged PR #1941:
URL: https://github.com/apache/buildstream/pull/1941
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "abderrahim (via GitHub)" <gi...@apache.org>.
abderrahim commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-2271254702
> I have to dive deeper to figure out whether this is a correct bug fix, but want to make sure we don't unintentionally change the behavior (away from the agreed design).
Yeah, the problem is that there is no agreed design. The mailing list post around #1434 didn't elaborate on this point precisely and my reading of it didn't seem to contradict my understanding of it. However, it was ultimately merged without much discussion.
My research didn't lead me to https://gitlab.com/BuildStream/buildstream/-/merge_requests/1403 (I wasn't on the mailing list back then). I still need to read the whole discussion around that change.
This change brings back the behaviour that we have been using for a long time on buildstream 1.x. I'll try to post on the mailing list with a summary of my research and make a case for this change.
> And we would also need to consider whether this behavior change may break any users.
Do you have an idea of who these users might be? In all buildstream projects I worked on, this is the right thing to do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "nanonyme (via GitHub)" <gi...@apache.org>.
nanonyme commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-2271362890
@juergbi this has a real monetary impact on any project used as base project that has to pay for bandwidth and discourages such projects from having public artifact caches. It doesn't seem like decision that is matter of gut feeling. Current behaviour is bad for BuildStream community.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] _context: use artifact caches from the parent project for junctions [buildstream]
Posted by "juergbi (via GitHub)" <gi...@apache.org>.
juergbi commented on PR #1941:
URL: https://github.com/apache/buildstream/pull/1941#issuecomment-5144384485
> Yeah, the problem is that there is no agreed design. The mailing list post around #1434 didn't elaborate on this point precisely and my reading of it didn't seem to contradict my understanding of it. However, it was ultimately merged without much discussion.
Re-reading the redesign mails, I think @gtristan was actually also in favor of defaulting to pulling from the top-level project as he stated in https://mail.gnome.org/archives/buildstream-list/2020-May/msg00018.html:
> On this point, I agree with Javier[[10]](https://gitlab.com/BuildStream/buildstream/-/work_items/401#note_74690536)'s expectations and think that this should be the default behavior
I can't find any later discussion of this point, so it's possible that it was simply missed in the implementation. So, it may be fair to merge this as a fix for a missed part of the last redesign without reopening a larger discussion.
I myself also think it makes sense for the cache servers of top-level projects to cache elements of all subprojects. With remote execution this is essential for storage locality, but it also seems reasonable for just remote caching. Artifacts of top-level projects typically include lots of files from artifacts of subproject elements, so they often need to cache a large part of those elements anyway.
### Possible follow-up
I would even argue that there should be a project-level configuration to completely ignore cache servers defined in subprojects somewhat similar to the `disallow-subproject-uris` option to ensure no third party cache servers are used even without the user setting `override-project-caches`. This would be a project-level replacement for the per-junction `ignore-junction-remotes` we once had but was considered too fine-grained. This could be something like:
```
junctions:
ignore-subproject-caches: True
```
However, this wouldn't be in the scope of this PR and wouldn't affect the default behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org