| gitlab-br-bot | buildstream: Tristan Van Berkom deleted branch 44-symlinks-in-the-sandbox-are-broken-by-the-path-to-the-buildstream-cache-containing-symlinks | 07:55 |
|---|---|---|
| *** tristan has joined #buildstream | 08:03 | |
| *** ChanServ sets mode: +o tristan | 08:20 | |
| *** tiagogomes has joined #buildstream | 08:25 | |
| *** jude has joined #buildstream | 08:28 | |
| juergbi | tristan: besides moving the load_public_data call from build() to Pipeline.__init__(), i also need to add such a call to TrackQueue.done(), right? (guarded by if element._cached()) | 08:29 |
| *** violeta has joined #buildstream | 08:31 | |
| *** tlater has joined #buildstream | 08:35 | |
| tristan | juergbi, not really no... unless that is a way to strengthen this fragile setup ? | 08:47 |
| tristan | juergbi, currently... TrackQueue.done() only force recalculates the consistency (sources) | 08:47 |
| tristan | The reason why building with --track works, is that the artifact has never been interrogated for whether it is cached or not, until *after* it passes through the TrackQueue | 08:48 |
| tristan | that's why it will have an unresolved '????????' key until it (and it's dependencies) get passed the TrackQueue | 08:48 |
| *** jonathanmaw has joined #buildstream | 08:49 | |
| tristan | juergbi, so basically, creating a pipeline with Pipeline( ... inconsistent=True ... ), just means; load it up but dont interrogate the source consistency or cached state | 08:49 |
| tristan | this is also a bit of an optimization for the case of tracking enabled scenarios (because interrogating the caches can be I/O intensive and take a long time) | 08:50 |
| tristan | we dont need to know if an element was cached, or if it's sources have the correct ref, when we know we're going to ask for a new ref along the way anyway | 08:51 |
| juergbi | tristan: as my suggestion is for TrackQueue.done() i don't see why it would break building, i.e., it's already been processed by the track queue | 08:57 |
| juergbi | what other place do you suggest to trigger plugin data loading for the case of build --track where some artifacts are already in the local cache? | 08:57 |
| juergbi | (there is currently no implicit loading in get_public_data() or anything like this) | 08:57 |
| *** ssam2 has joined #buildstream | 08:58 | |
| juergbi | eh, public data | 08:58 |
| tristan | juergbi, Ah right... just because we tracked, doesnt mean we're going to build it, so it needs doing somewhere | 08:58 |
| tristan | juergbi, Ok so maybe this belongs directly inside Element._cached() ? | 08:58 |
| juergbi | yes, we could do it conditionally for performance reasons | 08:59 |
| tristan | In Element._cached() there is also a force recalculate, so when force recalculating it would seem the right place to reload that too | 08:59 |
| juergbi | i was considering this originally but that relies on someone calling _cached() before calling get_public_data() | 08:59 |
| juergbi | will probably always be the case but more by coincidence | 09:00 |
| tristan | Well, I guess an invariant is that an element has been interrogated for being cached, before ever trying to build it | 09:00 |
| tristan | And calling get_public_data() is invalid on an element that was never either built or cached | 09:01 |
| juergbi | i guess that's relatively sane | 09:01 |
| juergbi | it's still somewhat side effect-heavy for _cached() | 09:01 |
| tristan | It could be more user friendly to have better exceptions for this than 'assert()' | 09:01 |
| tristan | But in any case they should remain regular exceptions (not derived from _BstError), so that they always show up as "BUG" and have a stack trace | 09:02 |
| tristan | I guess that's not very important anyway | 09:02 |
| tristan | juergbi, True, but then maybe it's _cached() which needs to be renamed ? | 09:02 |
| tristan | juergbi, i.e. it's a bit confusing already the flow of this | 09:03 |
| juergbi | i would actually like it better to load it on demand in get_public_data(). the reason i didn't go down that route was that it might be mainly triggered in child processes and thus, it would be done multiple times, increasing overhead | 09:03 |
| tristan | the side effect is already that something can have a cache key before it should | 09:03 |
| tristan | when accidentally calling _cached() ahead of a TrackQueue | 09:03 |
| tristan | which is pretty bad | 09:03 |
| *** jude has quit IRC | 09:10 | |
| tristan | So I wonder if there is a design flaw; I think I need "project variants" | 09:23 |
| tristan | Otherwise, when building a huge amount of GNOME modules for either flatpak runtime *or* regular system, then I will have a lot of repetative YAML defining variant conditional prefix etc | 09:24 |
| *** jude has joined #buildstream | 09:31 | |
| tlater | It's possible that there is a globbing issue with the default project conf split rules - since moving to the newest master my test on that has been failing. | 09:40 |
| tlater | Specifically, only specifying "runtime" in a compose element stages everything. | 09:40 |
| tlater | Ah, yeah | 09:43 |
| tristan | ... | 09:44 |
| tristan | tlater, yeah there is a bug ? | 09:44 |
| tristan | tlater, ok so what about the everything else... should we get the CI up first and address this separately ? | 09:44 |
| tlater | Yeah, that should work. | 09:45 |
| * tristan was hoping to start with one test and a functional CI pipeline before adding more tests | 09:45 | |
| tlater | I think that the rules ignore things like /usr/share/doc/* | 09:45 |
| tristan | tlater, /usr/share/doc/* only means every file in /usr/share/doc/, it does not capture /usr/share/doc/subdir/file | 09:46 |
| tlater | Yup. I think the default config does that - which probably isn't expected? | 09:46 |
| tristan | although the default project config has %{docdir}/** | 09:46 |
| tristan | So it should be okay | 09:46 |
| tlater | Hmm... | 09:46 |
| tlater | Well, I'll get the CI up first, I suppose. | 09:46 |
| tristan | tlater, the default is data/projectconfig.yaml | 09:47 |
| tristan | yes, lets merge that ASAP | 09:47 |
| tristan | with colors | 09:47 |
| tristan | :) | 09:47 |
| tlater | Colors are working :) | 09:47 |
| tristan | tlater, I just fixed something that was missing (import problem)... but with master you can do, say: `bst show --deps none --format "%{public}" <element>` | 09:54 |
| tristan | but yeah, dont let it get in the way of landing the initial CI | 09:55 |
| tlater | Probably pushing for the last time before landing it now... Just waiting for the tests to finish. | 09:55 |
| *** tlater has left #buildstream | 09:59 | |
| *** tlater has joined #buildstream | 10:00 | |
| gitlab-br-bot | push on buildstream@master (by Sam Thursfield): 1 commit (last: build-all.sh.in: Fix typo `ldcondig` -> `ldconfig`) https://gitlab.com/BuildStream/buildstream/commit/8dc8a3ecba70501fbe0ad1e1abf614cab34d4948 | 10:02 |
| gitlab-br-bot | buildstream: merge request (artifact-storage->master: WIP: Artifact storage enhancements) #45 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/45 | 10:03 |
| tlater | Could you check this merge request, tristan? https://gitlab.com/BuildStream/buildstream-tests/merge_requests/6 | 10:03 |
| juergbi | jonathanmaw: please retest with my latest branch | 10:08 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: WIP: Add integration tests) #40 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 10:08 |
| jonathanmaw | juergbi: okie dokie | 10:08 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: WIP: Add integration tests) #40 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 10:09 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: WIP: Add integration tests) #40 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 10:11 |
| ssam2 | tristan, did you say you've seen this zlib issue before ? https://pastebin.com/bZv13Q1Y | 10:18 |
| * ssam2 starts by forcing `make -j 1` | 10:19 | |
| ssam2 | which gives a much saner output, but still fails to find the ld.so ... https://pastebin.com/mKfy93Ua | 10:20 |
| ssam2 | urgh! this is broken symlinks in my chroot again :-( | 10:20 |
| ssam2 | hopefully this is just an old chroot, otherwise something has regressed | 10:21 |
| tristan | tlater, ummm, so you are sure that that branch only contains clean commits ahead of upstream master... right ? | 10:21 |
| tristan | tlater, I dont think so | 10:21 |
| tlater | Which one? | 10:21 |
| tristan | tlater, buildstream-tests | 10:22 |
| tlater | Hm... lemme check, it should | 10:22 |
| tristan | ok well, I seem to have a totally screwed buildstream-tests checkout | 10:23 |
| tristan | so I'm recloning that to take a look | 10:24 |
| tristan | tlater, looks like it was my branch which was totally screwed this time, not yours | 10:24 |
| tlater | It should be clean, nothing strange going on. | 10:24 |
| tlater | Ah, alright | 10:24 |
| tristan | not sure how that happened, but doing git pull --rebase in master, caused mayhem | 10:24 |
| tristan | ok, lets merge that | 10:25 |
| tlater | I'll run the pipeline on buildstream then, just to confirm nothing breaks there, and then that can also be merged | 10:26 |
| tristan | tlater, the other one is borked it seems | 10:26 |
| tristan | https://gitlab.com/tlater/buildstream/-/jobs/21151991 | 10:26 |
| tlater | Yeah, shouldn't be anyomre | 10:27 |
| tristan | yeah maybe update that branch first, now that the other is merged, you want buildstream-tests master | 10:27 |
| gitlab-br-bot | push on buildstream@master (by Sam Thursfield): 1 commit (last: buildelement.py: Log commands run by source-bundle scripts) https://gitlab.com/BuildStream/buildstream/commit/f997084c22c2e1b744b03907ee850299ed3d90e5 | 10:27 |
| tlater | That's something it pulls from the other branch | 10:27 |
| tristan | nod | 10:27 |
| tristan | ssam2, I dont recall from looking at that... I need to see the conversation you were having about zlib... | 10:27 |
| tristan | lemme check history here... | 10:27 |
| ssam2 | tristan, it was something you mentioned in the planning meeting on monday | 10:28 |
| ssam2 | on reflection, i doubt it was anything related though | 10:28 |
| tristan | ssam2, ah okay... | 10:28 |
| tristan | now I remember what I was remembering in our meeting | 10:28 |
| tristan | here is a hint about what might be going wrong... | 10:28 |
| ssam2 | the problem i'm having is that /usr/lib/ld-linux.so is a broken symlink again; but maybe i'm using an old build | 10:28 |
| ssam2 | hopefully :/ | 10:29 |
| ssam2 | actually, since the symlink fix didn't affect cache keys, it's entirely possible that i managed to build with the wrong branch | 10:29 |
| tristan | basically this crazy stuff here: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_sandboxbwrap.py#L387 | 10:30 |
| tristan | ssam2, I think that's what I was thinking of, you ended up having some crazy device files lying around right ? | 10:30 |
| ssam2 | ah! that's a separate issue but yeah I still need to work out why that happened | 10:31 |
| tristan | there is something weird about bwrap, when you tell it to bind mount a specific device, it can leave it behind | 10:31 |
| ssam2 | urgh | 10:31 |
| tristan | and in a weird state, like; bwrap creates it with it's suid root | 10:31 |
| tristan | so it's in group root | 10:31 |
| tristan | freaking weird | 10:31 |
| tristan | leaks files it creates | 10:31 |
| tristan | should probably be fixed upstream | 10:31 |
| ssam2 | yeah that could be involved | 10:31 |
| tristan | I thought it was fixed though, because it was causing real issues for me before I worked around it | 10:32 |
| tristan | but maybe changing to proper ostree user mode checkouts fixed that | 10:32 |
| tristan | (or "hid" the bug) | 10:32 |
| tristan | I think I was failing to create the bogus files | 10:32 |
| tristan | under group root | 10:32 |
| ssam2 | yeah, the issue with /dev/null was that it was actually a regular file which looked very much like something had done `mkdir --version > /dev/null` and the output had been literally written to /dev/null | 10:33 |
| ssam2 | which seems like the inverse of that issue, i.e. /dev/null wasn't present when it should have been | 10:33 |
| tristan | that would be very odd | 10:33 |
| tristan | but it could be that it ends up being a regular file | 10:33 |
| tristan | maybe that sandbox stuff should be more brutal and lets say: prejudice about files left in /dev | 10:34 |
| tristan | right not it tries to be very selective | 10:34 |
| ssam2 | it could do; i can't see any use case for allowing artifacts to contain /dev | 10:34 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: WIP: Add integration tests) #40 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 10:34 |
| tristan | so if the user wants to store their own crap in /dev (not as 'null' or the devices we care about), we dont make any assumptions about what they are building and just let them do that | 10:34 |
| tristan | Well, who knows, maybe /dev is not a mount point in the target, but contains some file; who am I to say "you cant do that" :D | 10:35 |
| tristan | anyway that's my *first* instinct | 10:35 |
| tristan | but anyway, brutally clearing /dev can be a good idea | 10:36 |
| tristan | eventually I think though, we should have some clear documentation of limitations | 10:36 |
| tristan | including "You are not allowed to create artifacts with stuff you want to store in /dev" | 10:36 |
| ssam2 | yeah, as long as we automatically mount stuff there in BuildStream, it's going to be a special case | 10:38 |
| tlater | Damnit, I'll have to rebase again... | 10:48 |
| tlater | The tests are working on buildstream too, though | 10:48 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: WIP: Add integration tests) #40 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 10:49 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: main.py, widget.py: Moving some imports from main -> widget) https://gitlab.com/BuildStream/buildstream/commit/ed62ffd5229e90893ed5854dbc60368bcb8ddf9f | 10:58 |
| jonathanmaw | juergbi: artifact-storage branch seems to be working fine for me now | 10:58 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: Add integration tests) #40 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 11:03 |
| tlater | Alright, that (^) can probably be merged too. | 11:03 |
| jonathanmaw | juergbi: I can even checkout the changes afterwards. | 11:07 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: Add integration tests) #40 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 11:08 |
| tlater | I'll go grab lunch while I'm waiting :) | 11:11 |
| juergbi | jonathanmaw: great, thanks for testing | 11:12 |
| juergbi | is this already with dynamic public data or just making sure there are no regressions? | 11:13 |
| jonathanmaw | juergbi: with dynamic public data | 11:13 |
| juergbi | nice | 11:13 |
| gitlab-br-bot | buildstream: merge request (artifact-storage->master: WIP: Artifact storage enhancements) #45 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/45 | 11:19 |
| gitlab-br-bot | buildstream: merge request (artifact-storage->master: Artifact storage enhancements) #45 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/45 | 11:20 |
| *** tristan has quit IRC | 11:22 | |
| *** tristan has joined #buildstream | 11:24 | |
| *** ChanServ sets mode: +o tristan | 11:25 | |
| tristan | https://gitlab.com/tlater/buildstream/-/jobs/21159175 | 11:26 |
| tristan | sweet | 11:26 |
| * tristan merges | 11:27 | |
| tristan | we haz integration tests \o/ | 11:27 |
| tristan | yay tlater | 11:27 |
| gitlab-br-bot | buildstream: merge request (tristan/test_tests->master: Add integration tests) #40 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/40 | 11:27 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 3 commits (last: .gitlab-ci.yml: Add integration tests) https://gitlab.com/BuildStream/buildstream/commit/c7158e4db79b87e70432c946224b9a7ce4944a18 | 11:28 |
| tristan | ummm | 11:29 |
| tristan | juergbi, I'm a bit worried about get_public_data() loading on demand, as we lose the assertions | 11:30 |
| tristan | juergbi, dont we ? | 11:30 |
| tristan | Also, if it's possible that a cache key can be calculated through that code path, then it's possible that an element calling get_public_data() at the wrong time, might load the incorrect public data in advance of processing a TrackQueue | 11:30 |
| tlater | Cool, colorful integration tests \o/ | 11:39 |
| tlater | tristan: I assume I should start working on developer workspaces then? | 11:40 |
| tristan | tlater, if you can get to the bottom of compose element today, better tie that up first | 11:42 |
| tristan | tlater, what other tests are we skipping ? | 11:42 |
| tristan | and why, what's failing ? | 11:42 |
| tristan | juergbi, I know what to do | 11:42 |
| tlater | git (because I couldn't figure out how to get .git directories to run) | 11:42 |
| tlater | ostree (a checkout error, haven't gotten to the bottom of that) | 11:42 |
| tlater | And... | 11:43 |
| juergbi | tristan: we still have the _assert_cached() call. yes, it must not be called before potential tracking. however, that's the same for _cached() | 11:43 |
| tristan | juergbi, I'm going to keep what you did (load on demand), but I'm going to make 'recalculate' of _cached() tristate | 11:43 |
| tlater | Oh, the x86image test | 11:43 |
| tlater | Because it has way too many dependencies | 11:43 |
| tlater | And I'm just not sure how to write it | 11:43 |
| tristan | juergbi, this way, default recalculate is None, means find out if cached if not yet checked | 11:43 |
| tristan | juergbi, but explicit False means, dont try at all unless its already been calculated | 11:44 |
| juergbi | ah, to prevent accidental recalculate | 11:44 |
| tristan | exact | 11:44 |
| juergbi | sounds sensible | 11:44 |
| tristan | and retain the assertion | 11:44 |
| tristan | if ever a plugin calls outside of build phase | 11:44 |
| tristan | juergbi, I'll just merge the branch and do it locally, wont take me a minute... | 11:44 |
| juergbi | great, thanks | 11:44 |
| tristan | tlater, ok screw x86image, that's too complex | 11:45 |
| tristan | tlater, and I'm not happy with the API surface yet either | 11:45 |
| tristan | tlater, and git and ostree well... we could just remove those tests honestly, Sources all have converage in ./setup.py test anyway | 11:45 |
| tlater | So the only test to fix is compose? | 11:46 |
| tristan | tlater, ok so... if you can get a test for compose, that would be awesome, and you can just remove the others | 11:46 |
| tristan | tlater, yep, funny git is not working though, that means any of the build element tests are relying exclusively on tarballs ? | 11:47 |
| tlater | Yep | 11:47 |
| tristan | that's not ideal :-/ | 11:47 |
| tlater | Git is working, technically, I just can't include a git repo in the repo | 11:47 |
| tristan | Oh | 11:47 |
| tristan | tlater, yeah, in ./setup.py test, we generate them on demand | 11:47 |
| tlater | Ah | 11:48 |
| tlater | Hm, that could actually work here | 11:48 |
| tristan | a bit weird to have a git repo revisioned inside a git repo | 11:48 |
| tlater | It's impossible without strange command flags, anyway. | 11:48 |
| tristan | yeah well, we set environment variables so that we can test predictable track output | 11:49 |
| tristan | (git shas) | 11:49 |
| tlater | Should I change things to that? I've just cloned repositories and made tarballs locally, it should be predictable | 11:49 |
| tristan | tlater, but in any case, it's not ultimately important to test that part... just annoying we cant use real git repos as test cases | 11:49 |
| tristan | tlater, an alternative, if we're worried about relying on external / untrustable resources... is we could commit to some 'test-repo/' prefixed branches of buildstream-tests itself | 11:50 |
| tristan | if we want to use git checkouts, that are not from, say, git.gnome.org or smth, in the test suite | 11:50 |
| tristan | meh | 11:51 |
| tlater | That could work, although it won't make things too much more reliable | 11:51 |
| tristan | tlater, nah leave that as is, I'd rather get you on workspaces asap | 11:51 |
| tlater | Alright, I'll try and see what the problem with compose is and fix that then. | 11:51 |
| tristan | but would be really great to have a test case for compose, and maybe we solve a bug in the process | 11:51 |
| tristan | nod | 11:51 |
| tlater | I'll have to reboot first... Turns out the reason I kept pushing 755 files was that my root directory was mounted with those permissions in fstab. So much for using debian because it doesn't break. | 11:54 |
| *** tlater has quit IRC | 11:55 | |
| gitlab-br-bot | buildstream: merge request (sam/host-and-target-arch->master: Add --host-arch and --target-arch, and 'host-arches' conditional) #34 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/34 | 11:55 |
| *** tlater has joined #buildstream | 11:57 | |
| ssam2 | turns out i didn't test my `bst checkout` changes til now :-( | 11:57 |
| ssam2 | but they are now fixed | 11:57 |
| ssam2 | and the good news is that the symlinks issue and the /dev/null issue have gone away after a clean rebuild | 11:57 |
| ssam2 | the later issue may have been caused by me not mounting /dev in the bootstrap chroot initially, rather than any buildstream problem | 11:58 |
| tristan | Hmmm | 12:02 |
| tristan | how come I can no longer checkout 'build-gnome' in buildstream-tests with 'git checkout build-gnome' ? | 12:02 |
| tristan | git branch -r says there is an origin/build-gnome there | 12:03 |
| tlater | I didn't mess with origin... I'll try | 12:04 |
| tristan | dont worry about it | 12:05 |
| tristan | tlater, | 12:05 |
| tlater | Either way, it checks out fine for me | 12:05 |
| tristan | juergbi, your branch does something really strange I didnt anticipate :-S | 12:06 |
| tristan | juergbi, usually when doing `bst show <target>`, logging is disabled during the phase that we load the pipeline and interrogate cached state | 12:06 |
| tristan | Now... I have a ton of log lines showing me that it's interrogating whether a source has it's ref or not (git cat-file) | 12:07 |
| tristan | which usually only shows up in `bst --debug...` mode | 12:07 |
| * tristan tries to work out why | 12:07 | |
| tristan | oh its my change ! | 12:08 |
| tristan | ok maybe I did it wrong | 12:09 |
| tristan | yeah my mistake, fixed | 12:11 |
| juergbi | ok | 12:11 |
| tristan | So, as it stands, we dont have Element.set_public_data() | 12:13 |
| tristan | Are we going to need it ? | 12:13 |
| tristan | I can see that modifying the returned dict of Element.get_public_data() can work in most cases | 12:13 |
| tristan | but does not allow adding a new domain | 12:13 |
| juergbi | i was wondering that as well, forgot to mention | 12:13 |
| * tristan doesnt mind leaving this out for now, seems that jonathanmaw is getting along fine without it | 12:13 | |
| juergbi | it probably makes sense to support it | 12:13 |
| tristan | yeah | 12:13 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 10 commits (last: _yaml.py: Dump sanitized mappings as unordered mappings) https://gitlab.com/BuildStream/buildstream/commit/3ba6b89bf3fdfcadf95565215a3d3380c112fcf2 | 12:13 |
| tristan | it's a simple API addition | 12:13 |
| gitlab-br-bot | buildstream: merge request (artifact-storage->master: Artifact storage enhancements) #45 changed state ("closed"): https://gitlab.com/BuildStream/buildstream/merge_requests/45 | 12:14 |
| gitlab-br-bot | buildstream: issue #14 ("Artifact Cache Storage Enhancements") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/14 | 12:14 |
| gitlab-br-bot | buildstream: issue #15 ("Dynamically set Element public data") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/15 | 12:15 |
| tristan | tlater, ssam2, is it correct that `source bundle` need not care about integration commands ? | 12:17 |
| tristan | I guess | 12:17 |
| tristan | ? | 12:17 |
| tristan | Because anyway, with recent changes, it means that it is just not allowed | 12:18 |
| tristan | Or, you must have previously built a pipeline before being able to source-bundle it, which would be very strange | 12:18 |
| tristan | I think it matters little, though | 12:18 |
| tlater | I used the same implementation morph did, and just run ldconfig after every element | 12:19 |
| tlater | Probably not the best thing to do | 12:19 |
| tristan | I think it's fine for the purpose it was made for | 12:20 |
| tlater | Is it expected that the /usr/share/doc directory is included in an artifact that was created with just the runtime domain? | 12:22 |
| tlater | Without any contents, that is. | 12:23 |
| tristan | tlater, yes | 12:23 |
| tristan | tlater, well, it is with the default rules | 12:23 |
| tristan | tlater, we can change the default rules, though :) | 12:23 |
| tristan | tlater, /usr/share/doc/** does not capture '/usr/share/doc' itself | 12:24 |
| tlater | Hm, yeah. This changed from before globbing, I think | 12:24 |
| tristan | tlater, actually you raise an interesting point, it *could* | 12:24 |
| tlater | I don't think changing globbing rules to do that makes much sense, I wouldn't expect it to do that. But I would expect the defaults to exclude the doc dir if I specify I don't want docs. | 12:25 |
| tristan | juergbi, jonathanmaw: I'm going to make set_public_data() a hard requirement, I think it's all around safer this way | 12:26 |
| jonathanmaw | tristan: ok, sounds sensible. | 12:26 |
| tristan | So Element.get_public_data() will return a deepcopy() of the data, not risking any unintentional mutations of the data model | 12:27 |
| jonathanmaw | s/sounds/seems/ | 12:27 |
| tristan | Same with set_public_data() | 12:27 |
| juergbi | ok | 12:27 |
| tristan | tlater, fwiw the specific case revolves around whether 'utils.list_relative_paths()' is going to report directories with a trailing '/' or not | 12:28 |
| tlater | AFAIK, the current implementation matches how .gitignore works | 12:29 |
| tristan | tlater, I'm confused that the regex appraoch differed in this regard, but thats approximately the 'thing' I think | 12:29 |
| tristan | tlater, /usr/share/doc/** would match the directory and it's contents, if directories are listed with a trailing '/', but it would not match /usr/share/document.txt | 12:30 |
| tristan | I'm a bit torn, I think current approach is better ? | 12:30 |
| tristan | than listing directories with explicit '/ | 12:30 |
| tristan | ' | 12:30 |
| tristan | ? | 12:30 |
| tlater | Yeah, probably, but I think that's up to you ;) I'm just a bit concerned that the behavior changed, but I guess there are no real users yet. | 12:31 |
| tristan | I guess current approach is superior | 12:33 |
| tlater | Should I change the defaults to exclude the docs/other directories though? | 12:33 |
| tristan | tlater, even if you need to write 2 entries in split rules to do /usr/share/doc AND it's recursive content... the other way means that there is no way to exclude a directory but not it's content | 12:34 |
| tristan | i.e. current approach leaves every case possible to capture, afaics | 12:34 |
| tristan | tlater, sure, I guess that is sensible, the default splits will become much larger, but that's fine | 12:34 |
| tlater | Cool :) | 12:35 |
| tlater | One thing to note though, including a directory without its contents is probably a very very rare use case | 12:35 |
| tristan | oops I broke the testsuite :) | 12:35 |
| tristan | tlater, better to not impose restrictions though, I think | 12:36 |
| jonathanmaw | tristan: I'd like your input in the correct way to do something for dpkg-deploy. The problem is that commands are written in configure, and the list of packages to extract is not available until later. My current approach is to substitute those packages in by modifying the command list in assemble, just before chaining up to ScriptElement's assemble. To do this, I'm accessing self._ScriptElement__commands. (I'm also doing similar in dpkg-build, | 12:38 |
| jonathanmaw | but that's based of BuildElement, where commands isn't prefixed by __) | 12:38 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 2 commits (last: element.py: Added Element.set_public_data()) https://gitlab.com/BuildStream/buildstream/commit/8806a8841e1829791c1b532e71cf6b4b4f5e4a26 | 12:38 |
| tristan | jonathanmaw, ok heads up, your modifications of dicts returned by get_public_data() will no longer work, set_public_data() is required | 12:39 |
| jonathanmaw | Is _ScriptElement__commands the right way to do it? Should I be changing ScriptElement to remove the __? Should I be rewriting the assemble method so that I modify the commands before running the command in the sandbox? | 12:39 |
| tristan | (can be a get -> modify -> set) | 12:39 |
| jonathanmaw | tristan: yep, that bit's understood. | 12:39 |
| tristan | jonathanmaw, lets think of what API we need for this | 12:39 |
| tristan | of course, poking into private data is certainly wrong | 12:40 |
| tristan | but seems you have a need | 12:40 |
| tristan | "commands are written in configure" | 12:40 |
| tristan | You mean something like a dpkg kind of configure | 12:40 |
| tristan | not an Element.configure() | 12:40 |
| tristan | I'm a bit lost | 12:41 |
| jonathanmaw | a DpkgDeployElement's configure method | 12:41 |
| jonathanmaw | afaict, at that time I can't access the elements I'm depending on | 12:41 |
| tristan | jonathanmaw, ok so thats right..., this derives from ScriptElement right ? | 12:41 |
| jonathanmaw | so I can't extract the list of which packages exist | 12:41 |
| jonathanmaw | tristan: DpkgDeployElement derives from ScriptElement, yep | 12:42 |
| jonathanmaw | DpkgBuildElement derives from a BuildElement. | 12:42 |
| tristan | so why is ScriptElement.add_commands() not correct ? | 12:42 |
| tristan | Ok we're talking about two things | 12:42 |
| tristan | one at a time please | 12:42 |
| tristan | jonathanmaw, wanna talk about build element or deploy element first ? | 12:43 |
| jonathanmaw | tristan: the deploy element | 12:43 |
| tristan | Ok | 12:43 |
| tristan | so the deploy element is an element to deploy an artifact to a dpkg package | 12:43 |
| jonathanmaw | yep | 12:43 |
| tristan | And does not require anything about what kind of element produced the artifact | 12:44 |
| tristan | It has a sane default for the output of regular build elements | 12:44 |
| tristan | But that can be augmented by public data | 12:44 |
| tristan | Which is either manually specified | 12:44 |
| tristan | Or possibly dynamically generated by a previous dpkg-build element | 12:44 |
| tristan | jonathanmaw, so far we're on the same page ? | 12:45 |
| jonathanmaw | tristan: a dpkg-build element automatically generates public data that a dpkg-deploy element can use to generate the package. | 12:45 |
| jonathanmaw | I haven't given much thought to defaults for when that doesn't exist | 12:45 |
| tristan | Ok | 12:45 |
| tristan | But a human being can also specify those details | 12:46 |
| tristan | Right ? | 12:46 |
| jonathanmaw | yep | 12:46 |
| tristan | So it could have been something I converted, imported from a source rpm, maybe generated public data for | 12:46 |
| tristan | and then decided to deploy as debian package | 12:46 |
| tristan | So, you are reading some things in Element.configure(), for the element configuration ? | 12:47 |
| jonathanmaw | tristan: yep. that reminds me, I need to document in some central place what the appropriate fields are for public data. | 12:47 |
| tristan | Which define some things to add to the commands of a script element ? | 12:47 |
| jonathanmaw | tristan: well, DpkgDeployElement.configure. It uses ScriptElement.add_commands() to add the commands | 12:48 |
| tristan | jonathanmaw, Ok so... one thing is... please in the very near future, write to the mailing list about what all the public data you are using actually is | 12:48 |
| jonathanmaw | AIUI that's the appropriate place, since that information is needed to generate cache keys. | 12:48 |
| tristan | it would be nice to briefly discuss this on the ML and look at what parts have to be dpkg specific, which can be generalized, and which could potentially undergo a smooth transition in the future (i.e. maybe they remain dpkg specific until such a date that we can change that in an API compatible way) | 12:49 |
| tristan | jonathanmaw, ok so what exactly are you reading from the configuration that needs to go into the commands ? | 12:49 |
| tristan | And... again, why is ScriptElement.add_commands() not good enough ? | 12:50 |
| tristan | why do you absolutely need to poke at the insides ? | 12:50 |
| jonathanmaw | tristan: I need to know which packages were generated. Currently I'm reading the public data of the element that was specified as "input" from the yaml, and using the keys from public.bst.split-rules as the list of packages | 12:51 |
| jonathanmaw | hmm, add_commands can overwrite based on group name, so that makes things a bit simpler. | 12:52 |
| jonathanmaw | Currently the way I'm inserting the list of packages into the commands is substituting the text "<packages>" with a space-separated list of packages | 12:53 |
| jonathanmaw | (since I'm using a shell for-loop to run the commands) | 12:53 |
| jonathanmaw | I suppose if I store a copy of the commands, I can make the substitutions and then make the changes using add_commands() without having to touch private data | 12:55 |
| tristan | jonathanmaw, if you need to dynamically set the commands based on element public data, then that has to be done at assemble() time, not in configure() | 12:56 |
| tristan | and will be omitted from the cache key, but it's okay, as long as the algorithm is stable, it will create the same thing based on that public data | 12:56 |
| tristan | jonathanmaw, of course, you can load something from the yaml and put that in your cache key, and substitute later at assemble() time, that's also fine | 12:57 |
| jonathanmaw | tristan: yep, seems sensible | 12:57 |
| tristan | user configurable things on your build element itself, need to be in the get_unique_key() | 12:57 |
| jonathanmaw | I was about to suggest the same but in different words | 12:58 |
| tristan | Oh | 13:03 |
| tristan | I screwed up again with public data ?? | 13:03 |
| tristan | frak | 13:03 |
| gitlab-br-bot | push on buildstream@test-public-data (by Tristan Van Berkom): 1 commit (last: Testing) https://gitlab.com/BuildStream/buildstream/commit/c8c5516c7f426b17cfc5c7ba4f29c09a0ba13d73 | 13:10 |
| gitlab-br-bot | push on buildstream@test-public-data (by Tristan Van Berkom): 1 commit (last: Testing) https://gitlab.com/BuildStream/buildstream/commit/4f3a71f898ad550e98c1e74f96ffa1fd145c9980 | 13:17 |
| tristan | Ok, so integration tests caught my bug... mere *hours* after landing them | 13:23 |
| tristan | \o/ | 13:23 |
| tlater | \o/ | 13:23 |
| tlater | Yeah, the caching is really slow, unfortunately | 13:24 |
| tristan | nah it's pretty quick ! | 13:24 |
| tlater | I have a really strange one | 13:25 |
| tlater | Compose tests are working, but I decided to add more to double check default behaviour | 13:25 |
| tlater | All of them work... | 13:25 |
| tlater | Except for the docs dir | 13:25 |
| tlater | That one is still not excluded | 13:25 |
| tlater | I guess I'll push my branches and ask then, probably overlooking a typo or something | 13:26 |
| tristan | tlater, it should be specified as '/usr/share/doc' to capture the directory itself, not '/usr/share/doc/' | 13:26 |
| tlater | Oh | 13:26 |
| tlater | Wouldn't doc/ work? | 13:26 |
| tristan | Nope | 13:26 |
| tristan | We discussed this recently :) | 13:26 |
| tristan | if /usr/share/doc/ were to work, then /usr/share/doc/** would also capture it | 13:27 |
| tlater | Oh, right, I'm stupid | 13:27 |
| tristan | I suppose that is noteworthy in the public data documentation | 13:27 |
| tlater | Interestingly enough that change also doesn't fix this | 13:27 |
| tristan | I need more information | 13:28 |
| tristan | link ? | 13:28 |
| tlater | Yeah, I'll push the branch in a second | 13:28 |
| tlater | Oh, nevermind, seems to be working now? I'll take it... | 13:29 |
| tlater | Cool | 13:29 |
| tristan | heh | 13:30 |
| tristan | Okay ! | 13:30 |
| tristan | I was worried we had bugs in the compose element, but it turns out it is working as expected ! | 13:30 |
| tristan | good news | 13:30 |
| tristan | tlater, I suppose they might break for one shot depending on what order we apply the defaults change and the tests | 13:31 |
| tlater | Hm, yeah | 13:31 |
| tristan | tlater, also, I wonder if it makes more sense, at least for this test, to override the split rules entirely in the tests project.conf | 13:31 |
| tlater | I can add a [exclude-ci] commit thing | 13:31 |
| tristan | and, if that is even possible | 13:31 |
| tristan | tlater, nah dont worry, it doesnt matter if a pipeline fails once just for that | 13:32 |
| tristan | sometimes gitlab is very slow to *start* a test, though | 13:33 |
| tlater | You can manually start the pipeline. I've had it refuse to start before. | 13:33 |
| ssam2 | the issue may be that we only have 2 runners, and i am running lots of builds for the GCC upgrade | 13:37 |
| ssam2 | i'm not sure entirely how it works, jjardon[m] would know more ... but I think it's limited to 2 because he is paying per isntance | 13:38 |
| ssam2 | *instance | 13:38 |
| tlater | Let's hope my computer didn't do weird permission things again... | 13:38 |
| ssam2 | ah no, that's nonsense, the buildstream ones seem to be separate from the Baserock ones | 13:39 |
| jjardon[m] | ssam2: we have unlimited runners in definitions: they are spawn on demand by a manager runner that is always on | 13:39 |
| ssam2 | ah OK | 13:39 |
| ssam2 | so i'm talking double nonsense | 13:40 |
| tlater | Would you merge the MR, tristan? Then I can try the buildstream pipeline. https://gitlab.com/BuildStream/buildstream-tests/merge_requests/7 | 13:40 |
| ssam2 | but are the buildstream ones using the same infra or different ? | 13:40 |
| ssam2 | i don't know how to check that | 13:40 |
| jjardon[m] | dont think buildstream is using baserock runners atm | 13:40 |
| jjardon[m] | OH, seems you are actually using it: see baserock-manager-runner2 in https://gitlab.com/BuildStream/buildstream/settings/ci_cd | 13:42 |
| jjardon[m] | but you are using the shared runners as well (the ones gitlab.com offer for free), so maybe sometimes you do builds on those as well | 13:42 |
| gitlab-br-bot | push on buildstream@tristan/split-domains (by Tristan Maat): 1 commit (last: projectconfig.yaml: Restore old default split domain behaviour) https://gitlab.com/BuildStream/buildstream/commit/c6446ec07edab5b069a5bf1a74ba109f39cf5c17 | 13:44 |
| tlater | Heh, I just noticed that the files bst checks out are created on jan 1st 1970 | 13:51 |
| tlater | I suppose that's the mtime normalization? | 13:51 |
| tristan | tlater, actually that is ostree | 13:53 |
| tristan | funny enough, our normalization is nov 11, 2011 | 13:54 |
| tlater | Oh, I guess my interface shows creation and not modification then. | 13:54 |
| tlater | tristan: Did you flag the message earlier? It would be nice if we could get this merged so I can move on: https://gitlab.com/BuildStream/buildstream-tests/merge_requests/7 | 14:04 |
| tristan | tlater, I pressed "merge when pipeline completes" :) | 14:06 |
| tlater | It won't though because the changes haven't been applied to master yet :/ | 14:06 |
| tristan | Ah | 14:06 |
| tlater | Actually, merging master first makes more sense, these cases aren't being tested for yet | 14:06 |
| tlater | *buildstream | 14:07 |
| tlater | s/master/buildstream/ | 14:07 |
| tristan | merged | 14:07 |
| tlater | Ok, thanks :) | 14:07 |
| tristan | tlater, so we need an MR for buildstream to update the default splits ? | 14:07 |
| tristan | and *that* pipeline will pass | 14:08 |
| tristan | tlater, this looks misleading: Error: File permissions differ for files results//compose-no-devel/usr/lib/debug/hello (-rw-r--r--) and expected//compose-no-devel/usr/lib/debug/hello (-rwxr-xr-x\n)Error: File permissions differ for files 'results//compose-no-devel/usr/lib/debug/hello' and 'expected//compose-no-devel/usr/lib/debug/hello' | 14:09 |
| tristan | compose-no-devel | 14:09 |
| tlater | Yeah, I believe that MR is there already | 14:09 |
| tristan | tlater, is it actually true ? | 14:09 |
| tlater | No, it's my system's weird permission issue again -.- | 14:09 |
| tristan | I dont see any MR for that at: https://gitlab.com/BuildStream/buildstream/merge_requests | 14:09 |
| tlater | Oh, wait, let me put that one up then | 14:09 |
| tlater | Will be easier to test that way around, since that one isn't affected by my strange defaults | 14:09 |
| tristan | tlater, I think that one is failing because a file that was expected to exist, didnt; or the other way around | 14:10 |
| tristan | i.e. it's the failing compose test | 14:10 |
| gitlab-br-bot | buildstream: merge request (tristan/split-domains->master: projectconfig.yaml: Restore old default split domain behaviour) #49 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/49 | 14:10 |
| tristan | And it's telling us that it's a permission mismatch | 14:10 |
| tristan | I think the reporting is misleading | 14:10 |
| tlater | No, I think the reporting is right | 14:10 |
| tlater | There probably actually is a permission mismatch, because my machine generates things with the executable bit set for some reason. | 14:11 |
| tristan | ohhhh | 14:11 |
| tristan | weird | 14:11 |
| gitlab-br-bot | buildstream: merge request (tristan/split-domains->master: projectconfig.yaml: Restore old default split domain behaviour) #49 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/49 | 14:11 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: projectconfig.yaml: Restore old default split domain behaviour) https://gitlab.com/BuildStream/buildstream/commit/c6446ec07edab5b069a5bf1a74ba109f39cf5c17 | 14:11 |
| tristan | thats why you were making MRs with weird permission changes ? | 14:11 |
| gitlab-br-bot | buildstream: Tristan Maat deleted branch tristan/split-domains | 14:11 |
| tlater | Yeah. I thought I fixed it, but apparently not | 14:12 |
| tristan | Looks like _pipeline.py accidentally became executable | 14:13 |
| tristan | annoying | 14:13 |
| * tristan fixes | 14:13 | |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: _pipeline.py: Restoring good old 0644 on this file) https://gitlab.com/BuildStream/buildstream/commit/4f5a72edaa64a107595e52f677a43d05e70a12e4 | 14:13 |
| tristan | any reason why build-all.sh.in and build-module.sh.in templates should be executable ? | 14:14 |
| * tristan thinks not | 14:14 | |
| tristan | the ostree push/pull scripts need to be | 14:14 |
| tlater | Those scripts are set executable by bst when they are installed, so no, they don't need to be | 14:14 |
| * tristan still doesnt have the ability to test real ostree-push with gnome7... time to pull my hair out in agonizing stress :-/ | 14:15 | |
| tlater | Gah, sorry, I'll need to sort out this weirdness soon. | 14:15 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: build-all.sh.in, build-module.sh.in: Removing unnecessary executable bits) https://gitlab.com/BuildStream/buildstream/commit/b304bdd015ef226d524afc1d622549b037ca4080 | 14:16 |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 1 commit (last: main.py: Removing final unnecessary executable bit.) https://gitlab.com/BuildStream/buildstream/commit/ae59134a32cf7c28d8d0b4ccb5518a2a0c18b97b | 14:18 |
| tristan | tlater, I'm soon leaving the building... | 14:33 |
| tristan | is there another buildstream-tests commit you need up there so we can close the lid on this ? | 14:33 |
| tlater | Yeah, but it's not done yet :/ I need to pull a new base sdk too, so this will be another hour | 14:33 |
| tlater | I'll make sure the MR is there tomorrow morning, this time without failing tests | 14:34 |
| tlater | -> Not sure why, but my cache decided to empty itself | 14:34 |
| tlater | I need to move to a distro I understand a bit better... | 14:34 |
| tristan | tlater, I think cache key calculation change with commits of today | 14:42 |
| tristan | rather than being emptied | 14:42 |
| tristan | tlater, specifically juergbi's branch this morning changed everything | 14:42 |
| tlater | That's possible I suppose | 14:42 |
| tlater | Hm... | 14:42 |
| tlater | I think I found it though | 14:43 |
| juergbi | yes, that was unavoidable | 14:43 |
| tlater | What's the fastest way to keep empty directories in git? | 14:43 |
| gitlab-br-bot | buildstream: merge request (jonathan/dpkg-build->master: WIP: Jonathan/dpkg build) #37 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/37 | 14:43 |
| tristan | tlater, I dont know that you can revision an empty directory | 14:44 |
| tristan | I think you cannot, with git | 14:45 |
| tlater | Hmm... That's annoying | 14:45 |
| tlater | I think the standard is keeping a .gitkeep file in it | 14:45 |
| tlater | But that needs to be removed before the script runs | 14:45 |
| tlater | Oh, I can just remove those in the test | 14:45 |
| tlater | With a bit of luck that should have fixed it | 14:48 |
| tlater | Unfortunately the CI doesn't seem to want to run... | 14:49 |
| *** tristan has quit IRC | 14:56 | |
| *** ssam2 has quit IRC | 15:08 | |
| *** ssam2 has joined #buildstream | 15:10 | |
| *** jonathanmaw has quit IRC | 15:42 | |
| tlater | ]#] | 16:05 |
| *** tlater has quit IRC | 16:20 | |
| gitlab-br-bot | push on buildstream@master (by Tristan Van Berkom): 2 commits (last: _yaml.py: Added node_copy() and list_copy()) https://gitlab.com/BuildStream/buildstream/commit/fcd2c348bb595fc3d3549cc1207afef632f83d80 | 16:22 |
| *** jude has quit IRC | 16:27 | |
| *** tristan has joined #buildstream | 17:44 | |
| *** ChanServ sets mode: +o tristan | 17:45 | |
| ssam2 | the source-bundle bootstrap on armv8l64 is finally getting underway | 18:11 |
| ssam2 | had to rework fhs-dirs so that /lib64 is a proper symlink to /usr/lib64 | 18:11 |
| gitlab-br-bot | push on buildstream@sam/host-and-target-arch (by Sam Thursfield): 2 commits (last: Add --host-arch and --target-arch, and 'host-arches' conditional) https://gitlab.com/BuildStream/buildstream/commit/13dfcb284bc6a83cd94739b1ca9f38a7e43f9aa5 | 18:53 |
| gitlab-br-bot | buildstream: Sam Thursfield deleted branch sam/host-and-target-arch | 18:53 |
| *** ssam2 has quit IRC | 19:03 | |
| *** tristan has quit IRC | 20:32 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!