| *** leopi has joined #buildstream | 04:25 | |
| *** tristan has joined #buildstream | 04:29 | |
| *** bochecha has joined #buildstream | 05:37 | |
| *** tristan has quit IRC | 06:24 | |
| *** juergbi has quit IRC | 06:45 | |
| *** juergbi has joined #buildstream | 06:47 | |
| *** tristan has joined #buildstream | 07:17 | |
| *** toscalix has joined #buildstream | 07:51 | |
| qinusty | Can anyone tell me how/where plugins are loaded within buildstream? | 08:09 |
|---|---|---|
| Kinnison | I'd guess from https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_plugincontext.py | 08:13 |
| *** noisecell has joined #buildstream | 08:36 | |
| tiagogomes | tristan, could you give another look at https://gitlab.com/BuildStream/buildstream/merge_requests/600 | 08:46 |
| tristan | tiagogomes, I don't understand at all how that fixes the race :-S | 08:50 |
| tristan | tiagogomes, last time I saw scheduler changes here, which I was hoping tlater could comment on; those looked appropriate. The original patch also made the calculation more fault tolerant, which *also* seemed sensible in combination with that | 08:51 |
| tristan | but I'm totally lost as to how changing the location of the temp directory changes things | 08:51 |
| tristan | does this exclude the tmpdir from the size calculations ? | 08:52 |
| tiagogomes | Didn't the commit message helped? | 08:52 |
| tristan | tiagogomes, didn't expand it, my bad... | 08:52 |
| tristan | Ummmm, tiagogomes how can a temporary file be reaped while we're calculating the size ? | 08:53 |
| tristan | Does the CAS use deep OS features for the tempdir which can interfere ? | 08:53 |
| tristan | mounting a tempfs or smth with, unforeseeable consequences ? | 08:54 |
| tiagogomes | It is true that linux doesn't finish off with the file if there are processes with a open file descritor to it | 08:54 |
| toscalix | I will be at akademy 2018 spreading the news about buildstream and freedesktop-sdk, among other things: https://toscalix.com/2018/08/08/codethink-is-sponsoring-akademy-2018-and-i-am-attending/ | 08:55 |
| tiagogomes | However, the race is that when you do the os.walk, the file still exits; after when you do a stat the file might have disappeared | 08:55 |
| tristan | tiagogomes, ok and this happens always in the temp area ? | 08:56 |
| toscalix | I will not stay for the whole event, which is similar in length and format to GUADEC but 3 full days | 08:56 |
| tristan | tiagogomes, so iiuc, cache size calculation is not exclusive; and happens while other things might be adding to the artifact cache; which iiuc was intended and alright | 08:56 |
| tiagogomes | I only saw it under those circumstances | 08:57 |
| tristan | tiagogomes, which means that the temp files might be in use and then dissappear, as a result of cache additions during builds and pull | 08:57 |
| laurence | toscalix, I will review the content proposal now | 08:57 |
| tiagogomes | yup, that's what happens. The ElementJob doesn't need to acquire the cache resource | 08:57 |
| tristan | So, basically the scheduler does not have a problem, but including the temp dir causes problems because of this | 08:58 |
| tiagogomes | That were my own conclusions | 08:58 |
| tristan | tiagogomes, I think the ElementJob *must* acquire the cache resources, but not exclusively, but I think that's what you mean | 08:59 |
| tiagogomes | Though you could have another ArtifactCommit job that was responsible for committing the artifact an acquired exclusive access to the resource | 08:59 |
| tristan | and I think that resource.py could probably be made more clear | 08:59 |
| toscalix | laurence: yes please, I would like to add links after merged and then send the first contents to work on to the mailing list | 08:59 |
| toscalix | including those links | 08:59 |
| toscalix | so you probablly will see an additional MR from me today. I am travelling this afternoon so I would like to send the proposal before that | 09:00 |
| tristan | tiagogomes, commits happen without exclusive access, only cleanup jobs require exclusive access; but I *think* we're on the same page here | 09:00 |
| tristan | tiagogomes, commenting, I'm just going to suggest an alternative wording for that commit message | 09:04 |
| tiagogomes | ok :) | 09:07 |
| tristan | tiagogomes, ah, one more thing; it seems the MR did not properly capture that it closes #520 | 09:10 |
| tiagogomes | tristan that's because I didn't want the issue to be automatically closed :) | 09:11 |
| tristan | tiagogomes, please just take care that it is a appropriately closed at the right time (it is mentioned in the MR so traceability is fine, just the automation wont do it) | 09:11 |
| tristan | Aha ! | 09:11 |
| tristan | Ok no worry :D | 09:11 |
| *** jonathanmaw has joined #buildstream | 09:15 | |
| tiagogomes | Were the gitlab irc notifications disabled? | 09:25 |
| tristan | looks like the bot is not around... ironfoot ? | 09:26 |
| tristan | maybe a server restarted ? | 09:26 |
| tristan | gitlab-br-logger seems to be present in #freedesktop-sdk | 09:27 |
| tristan | gitlab-br-bot, rather | 09:27 |
| jjardon | Hi, does the git source plugin supports git lfs ? | 09:31 |
| rdale | no | 09:32 |
| jjardon | :( ok thanks | 09:41 |
| ironfoot | tristan: will have a look soon | 09:45 |
| * jjardon opens https://gitlab.com/BuildStream/buildstream/issues/567 | 09:47 | |
| rdale | i've replied with my patch attached and what i think is wrong with it atm | 10:00 |
| valentind | tristan, I think I have it all for !616 (deterministic staging). Could I have a review to tell me if I forgot things? | 10:05 |
| tristan | valentind, I was going to check that next actually :) | 10:16 |
| valentind | tristan, Great. | 10:16 |
| tristan | valentind, What is the `deterministic` argument added to public API used for, asides from the local source plugin ? | 10:30 |
| tristan | valentind, comments all up | 10:44 |
| valentind | tristan, if false, then we go back to previous behavior, which keeps permissions. | 10:46 |
| valentind | I thought we maybe needed something like that. But I can rename to keep-permissions (and interpret it as opposite). | 10:47 |
| tristan | valentind, no no, do what the comments say; don't add any of that to the core copy_files() | 10:47 |
| tristan | valentind, copy_files() is used throughout; as far as I can see, this is only needed in the local plugin | 10:48 |
| valentind | tristan, Ah to the copy_files OK. | 10:48 |
| valentind | tristan, It is needed for remote in 1.2 | 10:48 |
| tristan | No, remote only needs to chmod(0644) after stage, and that's it | 10:48 |
| tristan | (also in the comments) | 10:49 |
| valentind | tristan, OK. | 10:49 |
| valentind | tristan, so I just write another function to recursively copy files for local? | 10:49 |
| tristan | valentind, the point is that copy_files() needs itself to not be a focal point for handling permissions and such; whatever is supported by the virtual fs stuff (in progress) and CAS, needs to just work and pass through | 10:50 |
| tristan | valentind, I think you dont need that; you just need to first (A) list_recursive_thing_from_utils.py() (B) Use the explicit list and pass it to copy_files(), avoiding a double walk... (C) Iterate over the same list and just set the perms | 10:50 |
| valentind | tristan, OK, I will do that. | 10:51 |
| tristan | What is going to be tricky and confusing, and I don't exactly know how is going to work, is how os.chmod() and such is going to transparently "just work" over the virtual FS, while the CAS based artifact slowly gets fixed to support more file attributes | 10:52 |
| tristan | I suppose there is a strategy in place for all of this, but I still have to get through a ton of stuff before I can get a grasp on what is planned there | 10:53 |
| tristan | oops, missed one more comment, valentind; you forgot to bump the artifact version | 10:54 |
| * tristan adds to issue | 10:54 | |
| valentind | tristan, to force rebuild artifacts? | 10:54 |
| tristan | valentind, that's what I was saying the other day | 10:55 |
| valentind | Sorry, I missed something. OK. | 10:56 |
| tristan | valentind, I said that it would be ideal that you have a single merge request, so that we don't need to bump the artifact version too many times :) | 10:56 |
| valentind | tristan, Ah, right. I did not get it. Sorry. | 10:56 |
| valentind | Found another bug with SafeHardLinks. dup syscall does not work. It gives an fd, but this fd is not useable. | 11:07 |
| valentind | Actually I can seek it. But read gives a EBADF. | 11:08 |
| valentind | I think I know why the mmap and this fail. | 11:10 |
| valentind | Because when you pass O_CREAT, then SafeHardLinks uses O_WRONLY even if you passed also O_RDWR. | 11:10 |
| tristan | valentind, oh sounds like a potentially nice find ! | 11:11 |
| valentind | YEs, that was simpler than I thought. | 11:11 |
| valentind | Now I have to find out how to get those flags from fuse. | 11:11 |
| tristan | it often is; finding out the problem is harder :) | 11:11 |
| valentind | If only mmap gave EBADF instead of a bus error when passing an fd that is not readable when we ask for read rights. | 11:12 |
| *** tiagogomes has quit IRC | 11:28 | |
| *** tiagogomes has joined #buildstream | 11:29 | |
| valentind | tristan, the flags to create fixed the mmap issue. | 11:48 |
| tristan | creat() ? | 11:49 |
| * tristan recalls the one regret of the design of the POSIX system library API | 11:50 | |
| valentind | Well in FUSE API is is called "create" and it used when open was called with O_CREAT. | 11:50 |
| tristan | :) | 11:50 |
| valentind | But I suppose creat calls the same. | 11:50 |
| tristan | valentind, I'm just horsing around | 11:50 |
| tristan | WSalmon, just commented on your issue... I think we have the right solution for the plugins to ensure they are configured properly; they need to properly assert that in their configure() implementations :) | 11:52 |
| tristan | WSalmon, in fact, that seems to just be a detail that was missed for a long time | 11:52 |
| WSalmon | tristan, im just reading thought the notes, many thanks for the detailed reply | 11:56 |
| tristan | WSalmon, my take on this is that; individual plugins have their own configuration, and they already (implicitly) raise LoadError in `Plugin.configure()` when the configuration provided is unsatisfactory | 12:00 |
| tristan | WSalmon, if there is nothing you can do with a source when neither ref or track are specified in the YAML config, it is already a bug that an error is not raised at Plugin.configure() time | 12:00 |
| WSalmon | just looking at the .configure in git.py now | 12:01 |
| WSalmon | :) | 12:01 |
| *** leopi has quit IRC | 12:05 | |
| tristan | valentind, btw, do we have a solution yet for `tar` ? | 12:20 |
| tristan | valentind, I'm seeing in the comments that you removed something for `tar`, but don't understand how it is fixed | 12:20 |
| valentind | tristan, There is nothing broken in tar. I have not found any determinism. | 12:21 |
| valentind | non-determinism. | 12:21 |
| tristan | valentind, even when run as root vs non-root ? | 12:21 |
| valentind | Python's tarfile does not make different between root and non-root. | 12:21 |
| tristan | Eh ? | 12:21 |
| tristan | Hmmm | 12:21 |
| valentind | GNU tar does. | 12:21 |
| valentind | Well, it does with respect to chown. But it does the chown first. | 12:22 |
| tristan | So wil Python's tarfile error out when trying to extract say, a file with extended attributes ? | 12:22 |
| valentind | For the permissions, they are the same whether you use root or not. | 12:22 |
| tristan | Right, and the ownership bits of course | 12:22 |
| tristan | Those will be different | 12:22 |
| valentind | extended attributes are not extracted. | 12:22 |
| tristan | Not *at all* ? Or not *when being a regular user* ? | 12:23 |
| valentind | Not at all. I have not seen anywhere in Python's code where it every does it. | 12:23 |
| tristan | valentind, it seems like a class of bugs to treat separately, but still to keep our eye on | 12:23 |
| valentind | tristan, And I think GNU tar also ignores extended attributes unless you tell it to use them. | 12:23 |
| tristan | valentind, that is surprising, as I am quite sure that for instance, YBD works; supports static device nodes in the artifacts, which are implemented with tar, and requires that you be root for it to work | 12:24 |
| valentind | tristan, yes, there might be reasons to change the behavior in the future. Maybe we do not want the permissions to work like that. But it is deterministic. | 12:24 |
| tristan | valentind, the chown is still disturbing; when you say it does the chown first; that is unconditional ?? | 12:25 |
| valentind | tristan, Maybe it passes "--xattrs". | 12:25 |
| tristan | valentind, I think YBD had a special python code for extracting, iirc | 12:25 |
| tristan | i.e. it walks the archive and does stuff, that's probably why it works | 12:25 |
| tristan | So if you run as root; and extract/stage the tarball; if it does not chown(root), then the owner UID/GID in the sandbox do not match the root UID in the sandbox, afaict | 12:27 |
| valentind | tristan, no, it does it only if uid or euid is 0. | 12:27 |
| tristan | valentind, exactly | 12:28 |
| valentind | But we fix users later. | 12:28 |
| valentind | There is this set_deterministic_user that is run later. | 12:28 |
| tristan | deterministic_user ? | 12:28 |
| tristan | aha | 12:28 |
| tristan | Right, that is a bit unfortunate but required :-S | 12:29 |
| tristan | valentind, I suppose we will need a format semantic when supporting staging directly to the CAS and preserving all attributes; so that it does not suddenly start behaving differently | 12:29 |
| tristan | valentind, I'm satisfied with the "why it's not broken", thanks for explaining that :) | 12:30 |
| valentind | tristan, What is YBD? I would like to see what they do. | 12:31 |
| valentind | Oh I see, it is part of baserock. | 12:32 |
| *** tristan has quit IRC | 12:36 | |
| WSalmon | tristan, could you suggest a good value or if i need a new element for LoadErrorReason for missing ref+track, i was expecting there to be something like "invalid combination of options" | 12:40 |
| paulsherwood | valentind: https://gitlab.com/baserock/ybd/blob/master/ybd/cache.py#L166 is what creates artifacts in ybd | 12:42 |
| valentind | paulsherwood, thanks | 12:43 |
| *** leopi has joined #buildstream | 12:47 | |
| *** tristan has joined #buildstream | 12:54 | |
| valentind | tristan, So I have tested with device file. It just fails as non-root. | 12:59 |
| *** jonathanmaw_ has joined #buildstream | 13:00 | |
| tristan | valentind, lets treat this as a separate class of issues; in any case our intention was to not support static dev nodes from the start so that is not a great example | 13:01 |
| *** jonathanmaw has quit IRC | 13:01 | |
| *** flatmush1 has joined #buildstream | 13:27 | |
| *** toscalix has quit IRC | 13:30 | |
| *** flatmush has quit IRC | 13:47 | |
| *** flatmush1 has quit IRC | 13:51 | |
| valentind | Suddenly with pylink-0.12.1 I get new errors. | 13:54 |
| valentind | Or, no it does not seem to be the issue. | 13:55 |
| valentind | pylint 2.1.1 is an issue. Had to rollback to 1.9.3. | 14:00 |
| valentind | qinusty, I see you had the same issue as me. | 14:02 |
| *** toscalix has joined #buildstream | 14:02 | |
| *** toscalix has quit IRC | 14:04 | |
| qinusty | Yes | 14:05 |
| qinusty | But is it an issue of pylint, or just more things we can tidy up/fix | 14:05 |
| *** flatmush has joined #buildstream | 14:06 | |
| juergbi | valentind: pylint 2.1.1 passes here on master, what issue do you see? | 14:10 |
| valentind | juergbi, https://paste.gnome.org/pdnsvmtjz | 14:11 |
| valentind | Unless there was a fix recently on master. | 14:12 |
| valentind | That I did not see. | 14:12 |
| juergbi | I'm wondering why I don't see any of those | 14:13 |
| juergbi | valentind: how do you invoke it? | 14:13 |
| valentind | python3 setup.py test | 14:14 |
| juergbi | same here | 14:14 |
| juergbi | I'm on Python 3.7 now but I also checked recently with 3.6 and latest pylint | 14:14 |
| valentind | juergbi, Did you see the last issue by qinusty ? | 14:15 |
| juergbi | no | 14:15 |
| valentind | https://gitlab.com/BuildStream/buildstream/issues/570 | 14:16 |
| * tiagogomes points out that we still test against 3.5 in the CI | 14:17 | |
| juergbi | and 3.6 as well, iirc | 14:18 |
| juergbi | but probably not 3.7 yet | 14:18 |
| juergbi | i can reproduce the warnings by directly invoking pylint, but not via setup.py | 14:19 |
| valentind | juergbi, you should add "pylint == 2.1.1" in setup.py to be sure it does not pick the wrong one. | 14:20 |
| valentind | I would not trust too much setuptools. | 14:21 |
| juergbi | no difference, it's the only pylint version on the system | 14:22 |
| valentind | juergbi, how do you get errors calling it manually and not through setup.py? | 14:24 |
| valentind | Do we have a configuration for pylint somewhere? | 14:24 |
| juergbi | .pylintrc in the source directory | 14:29 |
| *** leopi has quit IRC | 14:29 | |
| juergbi | valentind: is it possible that you have a system or user-wide pylintrc that gets used by setup.py test even though it shouldn't? | 14:29 |
| juergbi | btw: the currently used testsuite docker images have pylint 2.0.0, afaict | 14:30 |
| valentind | juergbi, I do not have pylint installed system wide. And no configuration. | 14:30 |
| valentind | I have removed the .eggs directory and tried it again. Same issue. | 14:32 |
| valentind | Have to go soon. Will look at that more later. | 14:33 |
| *** edb has joined #buildstream | 14:33 | |
| WSalmon | valentind i presume you have taken all the stuff in setup.cfg in to account? | 14:41 |
| tiagogomes | what directories buildstream looks for plugins | 14:41 |
| *** noisecell has quit IRC | 14:57 | |
| *** leopi has joined #buildstream | 15:30 | |
| *** noisecell has joined #buildstream | 15:31 | |
| *** toscalix has joined #buildstream | 15:52 | |
| *** noisecell has quit IRC | 15:56 | |
| tiagogomes | Something odd is going on here | 15:56 |
| tiagogomes | If a build failed before, If I run bst again it will just display the results of the last build? | 15:57 |
| *** phildawson has quit IRC | 16:02 | |
| *** phildawson has joined #buildstream | 16:02 | |
| qinusty | fatal-warnings is in the pipeline and almost ready for review tpollard, I'll try and get it merged tomorrow. | 16:03 |
| skullman | huh, found a race condition in source fetching. If two repositories have a common submodule and the fetches run in parallel they can cause an exception when the second finishes fetching and attempts to rename it into place in the cache. | 16:03 |
| * skullman looks to see if there's already an issue | 16:03 | |
| tpollard | qinusty: awesome! look forward to seeing it :) | 16:04 |
| tpollard | tiagogomes: that's expected now | 16:04 |
| tpollard | I think anyway | 16:05 |
| qinusty | Notes about ref-not-in-track: We decided that ref-not-in-track could be associated with a few plugins and not just git, so I have added it as a 'core warning'. I'll let you work it out from the docs since it'll be a good check to see if my docs make sense. | 16:05 |
| qinusty | tpollard ^ | 16:05 |
| tpollard | tiagogomes: https://gitlab.com/BuildStream/buildstream/merge_requests/561 | 16:05 |
| skullman | ah, it's basically the same as issue 5 | 16:08 |
| jmac | Suddenly started getting this: "Error loading user configuration: YAML file has content of type 'CommentedMap' instead of expected type 'dict': " | 16:28 |
| laurence | Nexus, could I ask you to cast your mind back to this - https://gitlab.com/BuildStream/buildstream/issues/166 | 16:29 |
| jmac | After updating master | 16:29 |
| laurence | And ask, did it get back-ported in the end? | 16:29 |
| laurence | I hope that it did and we can close the issue | 16:29 |
| tpollard | jmac: urk | 16:31 |
| Nexus | laurence: what about it? | 16:44 |
| laurence | <laurence> And ask, did it get back-ported in the end? | 16:44 |
| laurence | <laurence> I hope that it did and we can close the issue | 16:44 |
| *** toscalix has quit IRC | 16:45 | |
| qinusty | urgh now the lint issues are happening to me when I use ./setup.py test etc | 16:45 |
| Nexus | laurence: you'd have to ask tristan, i asked the same question on the issue | 16:46 |
| WSalmon | tristan i have updated https://gitlab.com/BuildStream/buildstream/merge_requests/621 and https://gitlab.com/BuildStream/buildstream/merge_requests/628 in line with your comments, any feed back would be great! | 16:49 |
| jmac | This is silly, I'm getting that CommentedMap error on ancient versions of buildstream master now and I haven't updated any packages on my system | 16:50 |
| laurence | Nexus, well, you asked a different question | 16:52 |
| laurence | But now I have an answer to mine | 16:52 |
| qinusty | Nothing better than ancient versions of builstream jmac, while bisecting last week I went through the chunk of commits which don't build without __version__.py existing which WAS created by setup.py. | 16:53 |
| * qinusty pushes to gitlab and hopes his pipeline passes for tomorrow | 16:55 | |
| *** jonathanmaw_ has quit IRC | 17:03 | |
| qinusty | Is the gitlab not dead? | 17:03 |
| qinusty | S/not/not | 17:03 |
| *** bochecha has quit IRC | 17:05 | |
| *** bochecha has joined #buildstream | 17:05 | |
| *** xjuan has joined #buildstream | 17:07 | |
| *** bochecha has quit IRC | 17:10 | |
| *** bochecha has joined #buildstream | 17:10 | |
| jmac | Can anyone tell me what version of ruamel they have on their system? | 17:20 |
| bochecha | jmac: 0.13.14 | 17:21 |
| bochecha | (from the Fedora 28 package) | 17:21 |
| jmac | Right, I've figured out what happened - I ran `pip3 install --user -e` on my system and it's upgraded ruamel to 0.15.52 | 17:23 |
| jmac | Filing an issue now | 17:31 |
| laurence | phildawson, can we close off the ticket for ''bst workspace open -f' does not behave as documented' - https://gitlab.com/BuildStream/buildstream/issues/449 | 17:36 |
| *** hergertme has quit IRC | 17:38 | |
| *** hergertme has joined #buildstream | 17:38 | |
| *** hergertme has joined #buildstream | 17:39 | |
| laurence | jmac, thanks for posting that to the list - enjoy the long weekend ! | 17:45 |
| *** rdale has quit IRC | 17:46 | |
| jmac | Hope it helps! | 17:49 |
| jjardon | related with what jmac just discovered: can I have reviews of https://gitlab.com/BuildStream/buildstream-docker-images/merge_requests/56 , please? | 18:03 |
| *** phildawson has quit IRC | 18:22 | |
| *** leopi has quit IRC | 19:30 | |
| *** cs_shadow has quit IRC | 20:14 | |
| *** edb has quit IRC | 20:23 | |
| *** tristan has quit IRC | 20:28 | |
| *** bochecha has quit IRC | 22:04 | |
| *** cs_shadow has joined #buildstream | 22:54 | |
| *** xjuan has quit IRC | 23:18 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!