| *** Prince781 has joined #buildstream | 04:55 | |
| *** Prince781 has quit IRC | 05:01 | |
| *** Prince781_ has joined #buildstream | 05:01 | |
| *** lantw44 has quit IRC | 05:34 | |
| *** lantw44 has joined #buildstream | 06:02 | |
| *** Prince781_ has quit IRC | 06:44 | |
| *** tristan has joined #buildstream | 06:45 | |
| gitlab-br-bot | buildstream: merge request (juerg/cache-key-handling->master: Contain cache key handling in Element class) #296 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/296 | 06:53 |
|---|---|---|
| gitlab-br-bot | buildstream: merge request (juerg/cache-key-handling->master: Contain cache key handling in Element class) #296 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/296 | 07:06 |
| *** hase has joined #buildstream | 07:11 | |
| *** noisecell has quit IRC | 07:46 | |
| tristan | juergbi, nice refactor btw :) | 08:01 |
| juergbi | :) | 08:02 |
| juergbi | still not happy with the cache key complexity | 08:02 |
| juergbi | however, better to at least contain it in one place | 08:02 |
| juergbi | btw: link_ref is only used in combination with push/pull which tarcache doesn't support. and for some reason push/pull are not in the abstract base class at all | 08:03 |
| juergbi | should add all of those to the base class, though | 08:03 |
| tristan | Eeek sorry :) | 08:04 |
| juergbi | looking at #273 right now | 08:04 |
| juergbi | i regret agreeing to this 'calculate cache key after build' approach more and more | 08:04 |
| juergbi | i almost have a fix ready but it seems more complex than it should be | 08:05 |
| juergbi | i still prefer the approach where we keep the pristine workspace source separate from the build files | 08:05 |
| tristan | Well, we knew that cache keys being calculated dynamically was already a thing | 08:05 |
| tristan | And I already suspected this kind of fallout was going to happen had we not centralized logic around this | 08:06 |
| juergbi | where is it needed besides workspaces? | 08:06 |
| tristan | tracking | 08:06 |
| juergbi | well, not quite. tracking is simpler to handle | 08:06 |
| tristan | build + track, or just track; is dynamic cache key calculation also | 08:06 |
| juergbi | there we know from the start that the source is inconsistent | 08:07 |
| tristan | it was the first step yes; but then workspace builds happened too | 08:07 |
| juergbi | for the workspace case it depends on whether the artifact is in the (local or remote) cache or not | 08:07 |
| juergbi | which is very weird | 08:07 |
| tristan | Now we also have dynamic cache key resolution without this too | 08:07 |
| juergbi | if it's in the cache, we won't build it, so we can consider the cache key stable | 08:08 |
| tristan | Which is because of non-strict mode with active PullQueue | 08:08 |
| tristan | so 3 cases where the cache key can change during a session | 08:08 |
| juergbi | yes but in the other two cases we can still calculate weak and strict cache keys | 08:08 |
| tristan | I know, they are not the same; but still | 08:08 |
| tristan | for the workspace case, remote artifacts should not really be an issue; those are tainted | 08:09 |
| juergbi | yes but whether it's just local or local+remote that we check doesn't matter | 08:10 |
| tristan | (and we shouldnt really need to check for possible collisions of shas) | 08:10 |
| juergbi | the ugly thing is that we determine whether a cache key is usable or not based on whether we find something in the cache with that key | 08:10 |
| tristan | right | 08:11 |
| tristan | that is the ugly thing | 08:11 |
| juergbi | you'll see the patch in a minute or so ;) | 08:11 |
| tristan | uh oh :) | 08:11 |
| juergbi | it's not that bad, it just adds yet more complexity to _update_state() | 08:11 |
| * tristan is in the midst of writing some other code | 08:12 | |
| juergbi | and i first have to fix a bug | 08:12 |
| tristan | in fact, it's thanks to Chandon's workspace patches that the bind mounting customization for shells is going to be much easier to tackle :) | 08:12 |
| * tristan should have patch by end of day | 08:13 | |
| juergbi | :) | 08:19 |
| *** jonathanmaw has joined #buildstream | 08:44 | |
| *** noisecell has joined #buildstream | 08:45 | |
| *** lantw44 has quit IRC | 08:45 | |
| *** lantw44 has joined #buildstream | 08:45 | |
| tristan | whoa... configurable bind-mounts almost works out of the box | 08:45 |
| * tristan just had to ignore an exception | 08:46 | |
| *** hase has quit IRC | 08:57 | |
| *** aday has joined #buildstream | 09:11 | |
| tristan | eeek dead code | 09:30 |
| tristan | tlater, I'm curious, tests/integration/project/project.conf has an alias `project_dir: file://{project_dir}` | 09:31 |
| tristan | tlater, and tests/testutils/integration.py has `format_files()` ... a likely candidate for the function which might transform that into a real directory | 09:32 |
| tristan | But format_files() is deadcode | 09:32 |
| tristan | imported in a lot of integration tests, and unused afaics | 09:32 |
| tlater | tristan: I'm fairly sure it's used in at least one test | 09:33 |
| tristan | cmake, compose and autotools all use `project_dir` somehow | 09:34 |
| tristan | but not with format_files() | 09:34 |
| tristan | currently trying to understand how cmake integration test is working | 09:35 |
| tristan | Nope, just finding *more* deadcode :'( | 09:35 |
| tlater | tristan: Yeah, iirc the project_dir is set when the suite spins up - format_files is only used when other things need to be set | 09:35 |
| * tristan 's eyes bleeding | 09:35 | |
| * tlater wonders how there can be so much deadcode :| | 09:36 | |
| tristan | tests/integration/cmake.py has `element_path` variable in it's tests | 09:36 |
| tristan | always unused | 09:36 |
| tlater | Ack, yeah, that is unused | 09:41 |
| tlater | tristan: The project_dir is set here: https://gitlab.com/BuildStream/buildstream/blob/master/tests/testutils/runcli.py#L386 | 09:42 |
| tlater | And there were some tests that had to use format_files to set variables in their elements | 09:43 |
| tlater | But I quickly stopped using that pattern because generating elements on the fly turned out to be nicer | 09:44 |
| tlater | It's possible that I purged all of it, and forgot to remove the format_files method - can't find a test requiring it right now. | 09:44 |
| gitlab-br-bot | buildstream: merge request (remove-some-deadcode->master: Integration tests: Removing some dead code) #297 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/297 | 09:46 |
| tristan | tlater, thanks for the pointer to how it works | 09:48 |
| tristan | I need either to add something to the subst'ed project.conf or use a generated project.conf | 09:48 |
| tristan | that should help | 09:48 |
| tlater | Hm, right, I never considered multiple tests using different project.conf files :/ | 09:49 |
| gitlab-br-bot | buildstream: merge request (juerg/cache-key-handling->master: Contain cache key handling in Element class) #296 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/296 | 10:00 |
| gitlab-br-bot | buildstream: merge request (remove-some-deadcode->master: Integration tests: Removing some dead code) #297 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/297 | 10:01 |
| tristan | "Could not connect to the CI server. Please check your settings and try again" | 10:01 |
| * tristan slaps gitlab and tries again | 10:02 | |
| tristan | weeeird | 10:03 |
| tristan | the pipeline is triggered and running, and the merge request doesnt know about it | 10:03 |
| gitlab-br-bot | buildstream: merge request (juerg/workspace-cache-keys->master: Do not use/show unstable cache keys) #298 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/298 | 10:05 |
| *** ssam2 has joined #buildstream | 10:05 | |
| gitlab-br-bot | buildstream: merge request (sam/compose-symlinks-issue->master: WIP: Avoid compose element dropping files that are staged inside directory symlinks) #295 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/295 | 10:07 |
| juergbi | CI pipeline worked fine for the updated !296 | 10:10 |
| gitlab-br-bot | buildstream: merge request (remove-some-deadcode->master: Integration tests: Removing some dead code) #297 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/297 | 10:16 |
| *** noisecell has quit IRC | 10:17 | |
| *** noisecell has joined #buildstream | 10:18 | |
| juergbi | ssam2: fyi: !296 should reduce the cache key confusion, at least outside element.py | 10:24 |
| ssam2 | great, i will have a look | 10:24 |
| jjardon[m] | Hi, now that the flatpak plugin is in bst-external we would lime to use it; what is the plan to consume it if no tags will be created in that repo? Is there a plan to merge it in bst repo at some point? | 10:35 |
| jjardon[m] | I'm thinking not only in CI but people that needs to build locally and probably will need to install 2 packages instead only bst | 10:37 |
| juergbi | i would still go for date-based tags but i don't have a strong opinion how to handle this | 10:48 |
| ssam2 | people will indeed need to install 2 packages; that's also the case for projects building images with the x86image plugin for example | 10:58 |
| ssam2 | would you consider adding bst-external to freedesktop-sdk as a submodule for now ? you could then make it transparent for your users | 10:59 |
| *** noisecell has quit IRC | 11:13 | |
| tristan | aiden, around ? | 11:19 |
| aiden | yes | 11:21 |
| tristan | ah | 11:22 |
| tristan | aiden, so to get started, I suggest first try building something in GNOME, following the GNOME specific getting started wiki | 11:23 |
| tristan | here: https://wiki.gnome.org/Newcomers/BuildSystemComponent | 11:23 |
| tristan | And, request developer access here: https://gitlab.com/BuildStream/buildstream | 11:24 |
| tristan | We just grant developer access to anyone who asks, that'll save annoyances with having to keep a separate fork in sync | 11:24 |
| aiden | right, ok, great, i will start at lunch | 11:25 |
| aiden | thanks | 11:25 |
| tristan | Some minor things you might look at, are for instance this minor annoyance: https://gitlab.com/BuildStream/buildstream/issues/257 | 11:25 |
| tristan | A bit more involved, but still relatively easy (probably hardest part is writing tests for it), is https://gitlab.com/BuildStream/buildstream/issues/253 | 11:26 |
| jjardon[m] | ssam2 if the recommendation is to use sumodules, can that be documented somewhere? This is for gnome-build-meta as well | 11:27 |
| tristan | aiden, since there are a lot of people around now, to avoid confusion... if you start working on one of these minor bugs, just self-assign the issue | 11:27 |
| jjardon[m] | and I'd expect some distros to want to package bst-external soon | 11:27 |
| aiden | ok, will do | 11:27 |
| ssam2 | jjardon[m], not really up to me to decide | 11:32 |
| ssam2 | just suggesting how i would do it | 11:32 |
| *** aday has quit IRC | 11:37 | |
| tristan | jjardon[m], things are not gonna be clear from the get go; this is a discovery process | 11:38 |
| tristan | I would right now recommend bst-external as a submodule to be honest; especially because bst-external contains things which explicitly dont yet have strong API guarantees | 11:39 |
| tristan | it's sort of an incubation repo | 11:39 |
| tristan | which is why I would also think it would be a bad idea for distros to package that | 11:39 |
| gitlab-br-bot | buildstream: merge request (juerg/cache-key-handling->master: Contain cache key handling in Element class) #296 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/296 | 11:43 |
| tristan | juergbi, closing 228 sounds good to me | 11:47 |
| *** noisecell has joined #buildstream | 11:47 | |
| juergbi | ok, ta | 11:47 |
| gitlab-br-bot | buildstream: issue #228 ("Don't print tty process group warning when exiting bst shell") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/228 | 11:47 |
| tristan | It's annoying that when bubblewrap exits because of an error; we dont really distinguish that from an error reported from a child process :-S | 11:49 |
| juergbi | ah, i guess that would be tricky to handle | 11:50 |
| juergbi | bwrap would have to report the original child exit code separately | 11:51 |
| juergbi | (or we'd have to ptrace it) | 11:51 |
| tristan | yeah | 11:52 |
| tristan | valgrind has some funky semantics for the caller to tell it what exit code to report for which errors; because it can assume you know what you expect your actual program to report as errors | 11:53 |
| tristan | but we can't even assume that | 11:53 |
| gitlab-br-bot | buildstream: merge request (juerg/cache-key-handling->master: Contain cache key handling in Element class) #296 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/296 | 11:59 |
| * tristan files https://github.com/projectatomic/bubblewrap/issues/257 | 12:07 | |
| tristan | juergbi, you should rebase !298 | 12:09 |
| juergbi | will do | 12:09 |
| gitlab-br-bot | buildstream: merge request (juerg/workspace-cache-keys->master: Do not use/show unstable cache keys) #298 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/298 | 12:11 |
| *** aday has joined #buildstream | 12:11 | |
| jmac | Why does Plugin.node_get_member have a default value argument if it throws an error when the key isn't in the dictionary? | 12:19 |
| tristan | jmac, it will not throw an error if the default value is provided | 12:21 |
| jmac | Aha, subtle error | 12:21 |
| tristan | this is how you distinguish between things which must be specified, and things which need not be specified | 12:22 |
| tristan | or, that's what it's intended for | 12:22 |
| jmac | I did provide a default but it was None. | 12:22 |
| jmac | That doesn't work. | 12:22 |
| tristan | heh, yeah; that wouldn't work | 12:22 |
| jmac | Not without some horrible kwarg processing | 12:22 |
| tristan | jmac, note we have a lot of `self.node_get_member(node, str, "foo", default='') or None` | 12:23 |
| tristan | because of that | 12:23 |
| juergbi | we need many-valued logic ;) | 12:23 |
| tristan | the more values we have... the more valuable is the code ! | 12:23 |
| tristan | :) | 12:24 |
| jmac | That's pretty clean, thanks tristan | 12:24 |
| juergbi | :) | 12:24 |
| gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: WIP: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 13:12 |
| jjardon[m] | tristan: ssam2 thanks for the inputs: https://gitlab.com/BuildStream/bst-external/merge_requests/20 | 13:15 |
| gitlab-br-bot | buildstream: merge request (tristan/project-bind-mounts->master: Add shell configuration for bind mounts) #299 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/299 | 13:16 |
| tristan | now lets see if that bind mount stuff works in the fallback platform... I think there is a good chance... | 13:19 |
| tristan | nice | 13:32 |
| tristan | juergbi, well, lets hit the button on !298 ... it's getting a bit wild indeed, but softened up a bit by the previous !296 | 13:33 |
| gitlab-br-bot | buildstream: merge request (juerg/workspace-cache-keys->master: Do not use/show unstable cache keys) #298 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/298 | 13:33 |
| gitlab-br-bot | buildstream: issue #273 ("Cache keys not printed correctly with workspaced elements") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/273 | 13:33 |
| juergbi | ok, great | 13:34 |
| juergbi | i at least tried to add sufficient comments | 13:34 |
| juergbi | but it's a complex topic | 13:34 |
| tristan | yeah | 13:35 |
| tristan | jjardon[m], so what do you think, should I just press merge ? or should I tell you that "Therefore" is spelled with an e at the end ? | 13:35 |
| * tristan rather doesnt mind so much the spelling mistake hehe | 13:36 | |
| tristan | or is therefor a valid spelling ? | 13:36 |
| tristan | eh, my spellcheck doesnt catch it for some reason; I guess *someone* on the planet thinks that is correct spelling | 13:36 |
| * tristan merges | 13:37 | |
| juergbi | wiktionary: therefor: (obsolete) Therefore, for that or this reason or cause. | 13:39 |
| tristan | So; lets say that in `shell` configuration for `host-mounts`... we disallow / error out if the provided host path is a directory | 13:40 |
| tristan | and, only allow host directory mounting with an explicit `bst shell` CLI option, as suggested here: https://gitlab.com/BuildStream/buildstream/issues/274 | 13:41 |
| tristan | A.) 'host-mounts' is a decent name here ? | 13:41 |
| tristan | B.) Thoughts on the above policy ? | 13:41 |
| tristan | C.) Dont error out if the host file doesnt exist, but print a warning about any host files which dont exist when entering the shell ? | 13:42 |
| tristan | juergbi, thoughts on the above three ? | 13:42 |
| juergbi | A) i think so. bind-mounts would be a possible alternative but having a reference to the host does make sense | 13:43 |
| juergbi | B) i can't currently think of a need to add a directory to the project config but it's possible that someone will come up with a sensible use case at some point | 13:44 |
| juergbi | so for now i think that's fine | 13:44 |
| juergbi | C) it's definitely good to print something. no strong feelings whether that should be a warning or an error | 13:45 |
| tristan | ssam2, pointed out that say, mounting the home dir in the sandbox via project configuration can have dangerous consequences | 13:45 |
| juergbi | that was actually me on | 13:45 |
| tristan | i.e. because the user is not entirely aware that their own files are somewhere in there | 13:45 |
| juergbi | #274 | 13:45 |
| tristan | ah ! | 13:46 |
| tristan | juergbi, right sorry, you commented, ssam2's report :) | 13:46 |
| tristan | ok, on the same page then | 13:46 |
| juergbi | indeed | 13:46 |
| juergbi | btw: i assume you're aware that source and dest are swapped compared to the mount command? | 13:47 |
| juergbi | it does make sense to have dest as key but the swapping could create confusion | 13:47 |
| tristan | it's a bit confusing, I think gitlab sends me emails from Sam when they are commends from anyone on an issue he opened | 13:47 |
| juergbi | oh, really? i think i see the right From here | 13:48 |
| tristan | It is possible that it's gmail to blame | 13:48 |
| juergbi | do we have a use case where it makes sense for source and dest to be different in the project config? | 13:48 |
| * tristan gets gitlab via gmail | 13:48 | |
| tristan | I dont want to create anything too rigid | 13:48 |
| juergbi | yes, in general i agree. i was just wondering because of the order of dest source | 13:49 |
| tristan | And while it is possible to mount the same host file at two locations in the sandbox, it is impossible to mount two host files to the same location in the sandbox | 13:49 |
| juergbi | exactly, that's why i agree that key makes sense | 13:49 |
| juergbi | it doesn't mean that there won't be any confusion | 13:49 |
| tristan | I know :) | 13:49 |
| tristan | confusion can not always be avoided :-S | 13:50 |
| tristan | I'll try to document it well | 13:50 |
| juergbi | however, for now i can't think of a use case where source and dest differ (unless your sandbox has weird prefix/path) | 13:50 |
| juergbi | in which case there won't be confusion anyway | 13:50 |
| tristan | I was also thinking that we might want to support project relative paths as sources | 13:50 |
| juergbi | hm | 13:50 |
| juergbi | if host-independent files work, shouldn't they already part of the staged files? | 13:51 |
| juergbi | *be | 13:51 |
| tristan | Or, support env var expansion (mount this file under $XDG_DATA_HOME/...) | 13:51 |
| *** hase has joined #buildstream | 13:52 | |
| juergbi | in the source? maybe | 13:52 |
| tristan | Well, I could change it | 13:54 |
| tristan | It could for instance be a list-or-dict thing | 13:54 |
| tristan | We could start with a list and later support list-or-dict if needed | 13:54 |
| tristan | like we have in other places | 13:54 |
| juergbi | i was wondering the same | 13:55 |
| juergbi | maybe actually also call it host-files | 13:55 |
| tristan | :) | 13:55 |
| juergbi | in case we want to support directories later after all | 13:55 |
| tristan | Ok, I'll make these changes | 13:55 |
| juergbi | tristan: btw: i see some confusing variable names in the code (already in master but extended in the MR) | 13:56 |
| juergbi | like mount_source being used for the destination. or am i misreading that? | 13:56 |
| tristan | it's inherently complex :-S | 13:56 |
| juergbi | is this source because it's the source of FUSE? | 13:57 |
| tristan | In fact, there are 2 things called a mount source | 13:57 |
| tristan | The original one, is the shadow directory in the sandbox where things are mounted from, into the real mount area | 13:57 |
| tristan | The new one was added with workspaces | 13:58 |
| juergbi | ah ok | 13:58 |
| tristan | That whole thing can use some more untangling, making that clear is a bigger job though | 13:58 |
| juergbi | a bit confusing to read | 13:58 |
| tristan | yes, certainly | 13:59 |
| juergbi | understandable | 13:59 |
| tristan | wrapping your head around the mounts and a sandbox is... mind numbing a bit | 13:59 |
| * tristan leaving closing coffee shop... | 14:00 | |
| gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: WIP: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 14:02 |
| gitlab-br-bot | buildstream: merge request (214-we-need-a-way-to-make-elements-depend-on-a-subset-of-an-element-s-output->master: WIP: Resolve "We need a way to make elements depend on a subset of an element's output") #284 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/284 | 14:02 |
| *** tristan has quit IRC | 14:03 | |
| *** tristan has joined #buildstream | 14:13 | |
| gitlab-br-bot | buildstream: merge request (changing_the_dictionary_in_get_unique_key->master: Changing the dictionary in get unique key) #300 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/300 | 14:14 |
| tristan | juergbi, ugh... ok I can't realistically do config as a list only, because test cases; I can do list-or-dict off the bat, though | 14:16 |
| juergbi | why can't you use the absolute test path in a generated project.conf? | 14:17 |
| tristan | Well, I could test one avenue but not both | 14:19 |
| tristan | I.e. the fist test ensures that mounting a file into an existing directory in the runtime works | 14:19 |
| tristan | The second test checks /usr/share/pony/pony.txt, knowing that /usr/share/pony didnt exist | 14:19 |
| tristan | Which, requires a special case | 14:20 |
| gitlab-br-bot | buildstream: merge request (changing_the_dictionary_in_get_unique_key->master: Changing the dictionary in `get_unique_key()`) #300 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/300 | 14:20 |
| * juergbi writes a pony package that installs that file and sneaks it into all distros ;) | 14:20 | |
| tristan | Yeah, I'll just ensure the controlled runtime image we test against doesnt have that package | 14:20 |
| gitlab-br-bot | buildstream: merge request (changing_the_dictionary_in_get_unique_key->master: Changing the dictionary in `get_unique_key()`) #300 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/300 | 14:28 |
| jennis_ | Is a rule of thumb that if a pipeline passes, the MR is "mergeable"? | 14:48 |
| gitlab-br-bot | buildstream: merge request (changing_the_dictionary_in_get_unique_key->master: Changing the dictionary in `get_unique_key()`) #300 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/300 | 14:51 |
| ssam2 | jennis_, it's an absolute rule that if it doesn't pass, it's *not* mergable | 14:53 |
| ssam2 | whether it *is* mergable depends on lots of things | 14:53 |
| jjardon[m] | tristan: are you seriously asking me how to spell correct english? :) | 14:57 |
| jjardon[m] | tristan: don't hesitate to comment in the MR next time: happy to correct it and resubmit | 14:58 |
| jennis_ | thanks ssam2 | 14:58 |
| tristan | jjardon[m], yeah I was... well it seems I was right but your spelling is an also correct, albeit obsolete spelling :) | 15:04 |
| jjardon[m] | tristan: I'd call it posh english | 15:05 |
| gitlab-br-bot | buildstream: merge request (tristan/project-bind-mounts->master: Add shell configuration for bind mounts) #299 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/299 | 15:06 |
| gitlab-br-bot | buildstream: merge request (tristan/project-bind-mounts->master: Add shell configuration for bind mounts) #299 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/299 | 15:07 |
| tristan | juergbi, I think "host-files" is quite ready to land now, although I did support the list-or-dict thing | 15:09 |
| tristan | because why not, and it's better for the tests also | 15:09 |
| juergbi | :) | 15:10 |
| tristan | Docs, tests quite thorough | 15:10 |
| * tristan even reworded his commit messages to say "host files" (hence the gitlab spam) | 15:10 | |
| tristan | and next merge request is in the 300s ! | 15:11 |
| tristan | will be nice to see MR 1000 | 15:11 |
| juergbi | lgtm. didn't review all details but it looks fine to me | 15:16 |
| *** hase has quit IRC | 15:18 | |
| gitlab-br-bot | buildstream: merge request (tristan/project-bind-mounts->master: Add shell configuration for bind mounts) #299 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/299 | 15:25 |
| tristan | juergbi, erm, I think we need a plugin level format bump for your recent ETag addition, although it's a bit unclear | 15:26 |
| tristan | i.e. git.py plugin | 15:26 |
| tristan | if you use a recent version of BuildStream, and push changes with new etags in tracking results | 15:27 |
| tristan | No sorry, that was _downloadablesource (tar & zip, not git) | 15:27 |
| tristan | ... then you need to require a format version of the tar plugin which supports this new semantic to consume the YAML | 15:28 |
| juergbi | ah because otherwise the new generated etag key gets rejected | 15:28 |
| juergbi | it would work fine without it but it bails out | 15:28 |
| juergbi | i'm still obviously not used to the format version handling | 15:28 |
| tristan | yes, it would just change it to allow people to require that, this... feels like a weird unintentional case, though | 15:29 |
| tristan | I mean, this is because tracking will do it | 15:29 |
| tristan | juergbi, yes, I'm getting the hang of it myself, I mean we designed it: Now we have to get used to using it | 15:29 |
| tristan | separate processes | 15:29 |
| tristan | lets see how it works out in practice :) | 15:30 |
| tristan | For the changes I'm affecting to buildstream & gnome-build-meta, this works well | 15:30 |
| tristan | because I can now make a change to gnome-build-meta which requires a new feature; and users have a very good clue what to do if it fails | 15:31 |
| tristan | Also, I would not be averse to less strict revisioning (i.e. only one rev per release); however most people use BuildStream from git; so it's better that we do this more in sync | 15:32 |
| *** ernestask has joined #buildstream | 15:36 | |
| jmac | When I run `setup.py test` locally all the tests in integration/ are skipped, like this: https://paste.gnome.org/p72lgmuq1 | 15:38 |
| jmac | Is there some condition flag somewhere for integration tests? | 15:38 |
| *** mcatanzaro has joined #buildstream | 15:40 | |
| ssam2 | yes, you pass --integration to py.test | 15:40 |
| ssam2 | or `--addopts "--integration" to ./setup.py test | 15:40 |
| ssam2 | i think the custom code for that is in conftest.py | 15:40 |
| ssam2 | might be nice if it told you how to enable the tests in cases where it skipped them | 15:40 |
| jmac | So it is... why are those special cases? | 15:42 |
| ssam2 | integration tests can be quite slow | 15:42 |
| ssam2 | so we wanted to avoid running them by default | 15:42 |
| ssam2 | they are not so slow since we switched to the Alpine sysroot instead of the Freedesktop SDK, but still slower than tests which don't need to download a binary sysroot in order to run | 15:43 |
| jmac | OK | 15:43 |
| jmac | Cheers ssam2 | 15:43 |
| ssam2 | that's effectively what "Integration test" means in buildstream: something that runs real binaries inside the sandbox | 15:43 |
| jmac | It's a bit difficult to know where to put new tests. I've gone for integration since that seems to be the only place build-commands were used, but I'm not actually running a built binary | 15:46 |
| jmac | (It's a test for setting the UID used during builds) | 15:46 |
| ssam2 | if you need binaries for the test to work, it's an integration test | 15:48 |
| ssam2 | testing pretty much anything about the sandbox beyond whether or not we can get files in there probably needs to be an integration-test | 15:48 |
| jmac | Specifically binaries produced by buildstream, or any binaries (/bin/sh)? | 15:49 |
| ssam2 | any binaries | 15:49 |
| jmac | Righto | 15:49 |
| ssam2 | I guess if you bind-mount binaries in from the host, it's a gray area :-) | 15:49 |
| ssam2 | not sure any tests do that right now | 15:49 |
| ssam2 | and probably shouldn't, because it would prevent the test suite working on e.g. Windows | 15:50 |
| tristan | ssam2, is that even possible ? | 16:02 |
| ssam2 | running the test suite on Windows? of course not | 16:02 |
| * tristan just added bind mounts from host, but it wont work in a build environment and is specific for interactive testing `bst shell` environments | 16:03 | |
| ssam2 | oh, bind-mounting binaries from the host ? A test could certainly do so if it wanted... but hopefully none do | 16:03 |
| tristan | You mean, trick an import element into importing a local source that is bind-mounted into the project dir ? | 16:03 |
| ssam2 | or that, yeah | 16:03 |
| tristan | wow, you need to really go out of your way | 16:03 |
| ssam2 | there are always ways. my point is just that we shouldn't do it :-) | 16:04 |
| tristan | yeah, that's nothing valid to test anyway :) | 16:04 |
| gitlab-br-bot | buildstream: merge request (239-use-pylint-for-linting->master: WIP: Resolve "Use pylint for linting") #283 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/283 | 16:04 |
| jonathanmaw | ssam2: do you have a repo that you tested the x86image plugin changes against? | 16:05 |
| jonathanmaw | iirc I used the imported baserock definitions last time I tried | 16:05 |
| ssam2 | yeah, baserock definitions is a good one | 16:06 |
| ssam2 | however, that currently doesn't build with bst 1.x due to https://gitlab.com/BuildStream/buildstream/issues/270 | 16:06 |
| ssam2 | freedesktop SDK also has a VM image target now, which happens to work with 1.x | 16:06 |
| ssam2 | tlater also has a branch somewhere to build an image, not sure where though | 16:07 |
| ssam2 | i don't think we agreed on where to land it yet | 16:07 |
| ssam2 | oh, it's here: https://gitlab.com/BuildStream/buildstream-examples/commits/x86image | 16:07 |
| *** hase has joined #buildstream | 16:14 | |
| jonathanmaw | ta ssam2 | 16:14 |
| * jonathanmaw tries to get it working | 16:14 | |
| *** Prince781 has joined #buildstream | 16:20 | |
| *** noisecell has quit IRC | 16:22 | |
| *** noisecell has joined #buildstream | 16:22 | |
| *** noisecell has quit IRC | 16:23 | |
| jonathanmaw | ssam2: aha, it probably hasn't been merged because it looks to import a tar from file:///src/files/image-tools.tar | 16:25 |
| *** hase has quit IRC | 16:26 | |
| *** hase has joined #buildstream | 16:26 | |
| jonathanmaw | hrm, `bst track` on docker sources seems to be failing now. did they change their API recently? | 16:36 |
| ssam2 | pretty bad move on their part if they did | 16:36 |
| ssam2 | what's the error ? | 16:36 |
| jonathanmaw | using the element provided by integration-tests, https://pastebin.com/SxKMwL8a | 16:38 |
| ssam2 | https://gitlab.com/BuildStream/bst-external/merge_requests/17 | 16:38 |
| ssam2 | fixed a while ago but nobody noticed, it seems | 16:38 |
| jonathanmaw | okie doke, I'll give that a go | 16:39 |
| *** aday has quit IRC | 16:42 | |
| jonathanmaw | ssam2: looks like that MR has a missing "manifest" in some cases | 16:43 |
| jonathanmaw | error text at https://pastebin.com/p1eyJCTN | 16:43 |
| jonathanmaw | I'll see if I can figure out what's going on | 16:43 |
| ssam2 | oops, good catch | 16:44 |
| ssam2 | looks totally broken actually | 16:46 |
| ssam2 | line 205 should be "manifest = json.loads(response.text)" surely ? | 16:46 |
| jonathanmaw | that's how it seemed to me | 16:46 |
| ssam2 | which raises the question ... why is the CI passing ?? | 16:46 |
| jonathanmaw | making that change seemed to fix it, but I'm no docker expert | 16:46 |
| jonathanmaw | ssam2: probably because we're not actually testing tracking in CI | 16:46 |
| jonathanmaw | bst-external's CI is nowhere near as comprehensive | 16:47 |
| ssam2 | right, I guess this codepath only runs for `bst track` | 16:47 |
| ssam2 | once jennis_ gets the new integration tests landed we can improve those testcases a bit more easily | 16:48 |
| jonathanmaw | indeedily! | 16:51 |
| *** Prince781 has quit IRC | 16:51 | |
| gitlab-br-bot | buildstream: issue #241 ("Allow name resolution inside bst shell") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/241 | 16:55 |
| tristan | juergbi, just food for thought: We may want to revisit the idea of having a common format version in the BuildStream repo | 17:00 |
| tristan | while external plugins need to be revisioned separately, it would seem that we can gain more convenience by revisioning format features for all core plugins under the same core format version | 17:01 |
| *** hase has quit IRC | 17:09 | |
| jonathanmaw | ssam2: hrm, after incorporating that fix, I'm still seeing problems when I try to fetch in the x86image test | 17:10 |
| jonathanmaw | is the URL right here? https://pastebin.com/DWue2ak5 | 17:10 |
| jonathanmaw | it seems to be, when compared to the one in bst-external's integration-tests | 17:10 |
| jonathanmaw | but `bst track` fails because of a 404 error | 17:11 |
| jonathanmaw | "404 Client Error: NOT FOUND" | 17:11 |
| *** aday has joined #buildstream | 17:11 | |
| ssam2 | not sure that's correct | 17:12 |
| ssam2 | https://registry.hub.docker.com/library/buildstream/image-tools/ doesn't even open in a browser | 17:13 |
| *** xjuan has joined #buildstream | 17:13 | |
| ssam2 | https://hub.docker.com/r/buildstream/image-tools/ exists (that's the web page) | 17:13 |
| ssam2 | not sure what is the correct URL though | 17:14 |
| ssam2 | opening correct image URLs in a browser just redirects to hub.docker.com/ in all cases, unhelpfully | 17:15 |
| ssam2 | ok, so maybe that is the correct URL | 17:15 |
| ssam2 | i'm not sure what the issue is I'm afraid | 17:16 |
| *** noisecell has joined #buildstream | 18:03 | |
| *** hase has joined #buildstream | 18:04 | |
| *** noisecell has quit IRC | 18:09 | |
| *** jonathanmaw has quit IRC | 18:11 | |
| *** ssam2 has quit IRC | 18:16 | |
| *** noisecell has joined #buildstream | 18:18 | |
| gitlab-br-bot | buildstream: merge request (jmac/build-uid->master: WIP: Specify custom UID for build sandbox in elements) #301 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/301 | 18:26 |
| *** hase has quit IRC | 18:48 | |
| *** xjuan has quit IRC | 19:06 | |
| *** xjuan_ has joined #buildstream | 19:06 | |
| *** xjuan_ is now known as xjuan | 19:08 | |
| *** valentind has joined #buildstream | 19:13 | |
| gitlab-br-bot | buildstream: issue #275 ("Indicate where artifacts are going to and comming from in the log") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/275 | 19:18 |
| *** valentind has quit IRC | 19:31 | |
| *** noisecell has quit IRC | 19:36 | |
| *** valentind has joined #buildstream | 20:17 | |
| *** ernestask has quit IRC | 20:42 | |
| *** xjuan has quit IRC | 21:54 | |
| *** tristan has quit IRC | 22:06 | |
| *** mcatanzaro_ has joined #buildstream | 22:27 | |
| *** mcatanzaro has quit IRC | 22:30 | |
| *** mcatanzaro_ is now known as mcatanzaro | 22:30 | |
| *** valentind has quit IRC | 22:37 | |
| *** aday has quit IRC | 22:44 | |
| *** mcatanzaro has quit IRC | 23:56 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!