| zyga | good morning | 06:19 |
|---|---|---|
| zyga | arnatious: hey, can you tell me how you created the container and where is it running? | 06:20 |
| zyga | arnatious: host info, lxd version, container info, etc | 06:20 |
| mvo | hey zyga | 06:21 |
| zyga | :) | 06:21 |
| zyga | how are you doing? | 06:21 |
| zyga | mvo: back to fighting that user test? | 06:22 |
| zyga | mvo: meanwhile: https://github.com/snapcore/snapd/pull/7337 is simple and green | 06:22 |
| mup | PR #7337: tests: re-enable mount-ns test on classic <Created by zyga> <https://github.com/snapcore/snapd/pull/7337> | 06:22 |
| zyga | if you agree it is simple, that is :) | 06:23 |
| mvo | zyga: yeah, that and one more flaky one | 06:23 |
| Aavar | Where is my config stored when using irssi as a snap? | 06:23 |
| zyga | I had a look at the two leaking lets that are on core | 06:24 |
| zyga | Aavar: most likely in ~/snap/irssi/current/.irssi | 06:24 |
| mvo | zyga: looking at the prs now | 06:24 |
| zyga | mvo: and one of them was very puzzling, the one I pasted last night | 06:24 |
| zyga | but I'll re-read that with fresh minnd | 06:24 |
| zyga | *mind | 06:24 |
| zyga | the pastebin is https://paste.ubuntu.com/p/Pdx9qmqbdH/ | 06:24 |
| zyga | mvo: btw, one surprise from yesterday is ordering of prepare calls | 06:26 |
| zyga | mvo: prepare-project-each is before prepare-suite-each | 06:26 |
| zyga | I was somewhat assuming that prepare-project-each is like prepare-task-each which doesn't exist | 06:26 |
| zyga | mvo: in that pastebin I linked to the part I don't understand is where did the revision 1001 and 1002 come from | 06:27 |
| zyga | where normally the revision map shows 36 and similar numbers | 06:27 |
| mvo | zyga: from our fake store maybe? | 06:31 |
| mvo | zyga: just guessing at this point, we setup a fake store for some tests | 06:31 |
| zyga | Ahhh, interesting. That must be it! Thank you | 06:33 |
| zyga | hey pedronis | 06:48 |
| zyga | I'm just looking at unit test failure in one of your PRs | 06:48 |
| zyga | https://www.irccloud.com/pastebin/CKcptuEn/ | 06:48 |
| zyga | I've seen this a few times recently | 06:48 |
| pedronis | weird error, I wonder why is not deterministic | 06:50 |
| pedronis | anyway I'm working in that area | 06:50 |
| zyga | thanks :) | 06:50 |
| pedronis | so don't think you should spend time on it | 06:50 |
| zyga | ok | 06:50 |
| zyga | I'll focus on review | 06:50 |
| zyga | pedronis: https://github.com/snapcore/snapd/pull/7329 can land, I think | 06:51 |
| mup | PR #7329: snap: explicitly forbid trying to parallel install from seed <Created by pedronis> <https://github.com/snapcore/snapd/pull/7329> | 06:51 |
| mup | PR snapd#7329 closed: snap: explicitly forbid trying to parallel install from seed <Created by pedronis> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7329> | 06:51 |
| === pstolowski|afk is now known as pstolowski | ||
| pstolowski | hello | 06:59 |
| zyga | good morning :) | 07:00 |
| mvo | hey pstolowski - goooood morning :) | 07:00 |
| mup | PR snapd#7340 closed: tests: adding support for arm devices on ubuntu-core-device-reg test <Simple 😃> <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7340> | 07:09 |
| mvo | zyga: 7338 has conflicts | 07:14 |
| mup | PR snapd#7339 closed: tests: don't look for lxcfs in mountinfo <Simple 😃> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7339> | 07:14 |
| zyga | mvo: on it | 07:16 |
| zyga | done | 07:16 |
| mup | PR snapd#7335 closed: tests: add check for snap_daemon user/group <Test Robustness> <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7335> | 07:17 |
| zyga | pedronis: https://github.com/snapcore/snapd/pull/7323 is something you should review I think | 07:19 |
| mup | PR #7323: features, overlord: make parallel-installs exported, export flags on startup <Created by bboozzoo> <https://github.com/snapcore/snapd/pull/7323> | 07:19 |
| zyga | it deals with feature flags and startup behavior | 07:19 |
| pedronis | added to my list | 07:20 |
| pedronis | zyga: that failing test is weird | 07:20 |
| pedronis | it's broken by design | 07:21 |
| pedronis | but a bit unsure why it fails only rarely | 07:21 |
| zyga | those kinds are most interesting :) | 07:21 |
| pedronis | pstolowski: did you add the MissingBase test to first boot stuff? | 07:25 |
| pstolowski | pedronis: checking | 07:27 |
| pstolowski | pedronis: yes, TestPopulateFromSeedMissingBase | 07:30 |
| pedronis | pstolowski: something is odd with it | 07:30 |
| mvo | zyga: 7336 should be an easy review | 07:30 |
| pedronis | pstolowski: unasserted goes into seed.yaml, not the snap | 07:30 |
| pedronis | but is working anyway, most of the time | 07:30 |
| pedronis | but sometimes it fails | 07:30 |
| zyga | mvo: mhm | 07:30 |
| pedronis | trying to understand why | 07:30 |
| pedronis | pstolowski: anyway, mostly pointing out that unasserted: true is in the wrong place | 07:31 |
| zyga | mvo: +1 | 07:31 |
| * zyga reviews https://github.com/snapcore/snapd/pull/7313 | 07:31 | |
| mup | PR #7313: many: add the start of Core 20 extensions support to the model assertion <Created by pedronis> <https://github.com/snapcore/snapd/pull/7313> | 07:31 |
| pstolowski | pedronis: hmm, sorry about that, shall i investigate? | 07:34 |
| pedronis | pstolowski: no, I'm looking into it | 07:35 |
| pstolowski | ok, thanks | 07:40 |
| Aavar | thank you zyga ;) | 07:43 |
| zyga | Aavar: for what? :) | 07:47 |
| pedronis | pstolowski: here's the cleanup/fix, fyi: https://github.com/snapcore/snapd/pull/7341/commits/709e0bd10c78fc74334d7660c7549647d04d5232 | 07:51 |
| mup | PR #7341: many: introduce package seed and seedtest <Created by pedronis> <https://github.com/snapcore/snapd/pull/7341> | 07:51 |
| pedronis | pstolowski: it was creating variant of local both unasserted and asserted (under the foo* stuff), and sometimes they diverged (not sure why) | 07:56 |
| pedronis | pstolowski: anyway, a bit of confusion all around there :) | 07:57 |
| pstolowski | pedronis: aaah i see | 07:58 |
| mup | PR snapcraft#2688 closed: Build base types <Created by sergiusens> <Closed by sergiusens> <https://github.com/snapcore/snapcraft/pull/2688> | 08:12 |
| zyga | pedronis: https://github.com/snapcore/snapd/pull/7313#pullrequestreview-280011112 | 08:13 |
| mup | PR #7313: many: add the start of Core 20 extensions support to the model assertion <Created by pedronis> <https://github.com/snapcore/snapd/pull/7313> | 08:13 |
| zyga | mvo: hey | 08:15 |
| zyga | mvo: look at this: | 08:15 |
| zyga | + getent passwd snap_daemon | 08:15 |
| zyga | snap_daemon:x:584788:584788::/nonexistent:/usr/bin/false | 08:15 |
| zyga | mvo: full log https://api.travis-ci.org/v3/job/576917244/log.txt | 08:15 |
| zyga | 2019-08-27 07:47:28 Error restoring google:arch-linux-64:tests/main/system-usernames-illegal : | 08:15 |
| zyga | also suspicious there | 08:16 |
| zyga | + snap remove test-snapd-illegal-system-username | 08:16 |
| zyga | snap "test-snapd-illegal-system-username" is not installed | 08:16 |
| zyga | typo? | 08:16 |
| zyga | mvo: this was in https://github.com/snapcore/snapd/pull/7316 | 08:16 |
| mup | PR #7316: tests: add unit tests for cmd_whoami <Created by ardaguclu> <https://github.com/snapcore/snapd/pull/7316> | 08:17 |
| * zyga reviews https://github.com/snapcore/snapd/pull/7341 | 08:17 | |
| mup | PR #7341: many: introduce package seed and seedtest <Created by pedronis> <https://github.com/snapcore/snapd/pull/7341> | 08:17 |
| mup | PR snapd#7344 opened: snapstate: use strutil.VersionCompare() in checkVersion() <Created by mvo5> <https://github.com/snapcore/snapd/pull/7344> | 08:21 |
| mup | PR snapd#7333 closed: tests: fix system version check on listing test for external backend <Created by sergiocazzolato> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7333> | 08:22 |
| mup | PR snapcraft#2689 opened: schema: kernel and snapd types do not use bases <Created by sergiusens> <https://github.com/snapcore/snapcraft/pull/2689> | 08:24 |
| mvo | zyga: nice, thanks, let me check | 08:24 |
| mvo | zyga: so the catcher of the test thing did find something | 08:24 |
| zyga | indeed | 08:24 |
| zyga | look at the log in detail, I'm sure there's something there that's useful | 08:24 |
| mvo | zyga: yeah, look | 08:25 |
| sergiusens | xnox: mvo does `type: kernel` always require a base or will any kernel snap work with any base? | 08:25 |
| sergiusens | mvo: pedronis and from last week, do we want the `type: snapd` to NOT specify a base or do we want it to specify a base (even if none or bare)? (unrelated to using a build-base) | 08:27 |
| mvo | sergiusens: I think we do not need a base for snapd, its self contained | 08:29 |
| mvo | sergiusens: the kernel is tricky, if it uses hooks it needs a base so that we know how to run the hooks | 08:29 |
| mvo | hey Chipaca, good morning! | 08:30 |
| Chipaca | mvo: 👋 ! | 08:30 |
| mvo | Chipaca: can I pester you with a question already? I got: | 08:30 |
| mvo | $ snap switch test-snapd-assumes --edge | 08:30 |
| mvo | "test-snapd-assumes" switched to the "edge" channel | 08:30 |
| mvo | Channel edge for test-snapd-assumes is closed; temporarily forwarding to candidate. | 08:30 |
| mvo | Chipaca: today which is a bit odd because the edge channel is open (and was open at the time of this message). does that ring any bells? | 08:30 |
| Chipaca | mvo: was this in a spread test, or somewhere you have debug logs? | 08:31 |
| Chipaca | huh | 08:32 |
| mvo | Chipaca: I ran this manually, was doing some exploring about "assumes:" this morning | 08:32 |
| Chipaca | it's reproducible | 08:32 |
| mvo | cool! | 08:32 |
| mvo | zyga: this is a fun issue, the logs show that the snap_daemon user is added *before* test-snapd-illegal-system-username is POSTed to the daemon. very puzzling | 08:33 |
| zyga | POSTed? | 08:34 |
| zyga | as in installed? | 08:34 |
| zyga | whaat? | 08:34 |
| zyga | :D | 08:34 |
| zyga | mvo: I'll be back soon, let me make some tea | 08:34 |
| mvo | zyga: yeah, at least if the journald logs are in order | 08:35 |
| mvo | zyga: yeah, no worries, just wanted to share my puzzlement | 08:35 |
| Chipaca | mvo: is 381b45e27773eff87bca4f8ed9b96d027bfb8ee5 part of 2.41~pre1 ? | 08:36 |
| Chipaca | mvo: i suspect that's the issue | 08:36 |
| mvo | zyga: found the issue, its "fun" | 08:38 |
| mvo | Chipaca: let me check | 08:38 |
| mvo | Chipaca: it is part of 2.41 it seems :/ | 08:39 |
| Chipaca | mvo: maybe we should back that out until we have time to fix it | 08:39 |
| mvo | Chipaca: could we just revert it in 2.41? | 08:39 |
| mvo | Chipaca: yeah, I think that makes sense | 08:40 |
| zyga | mvo: oh | 08:40 |
| zyga | mvo: tell me :) | 08:40 |
| mvo | zyga: pr will be up shortly but its easy once spotted | 08:42 |
| pedronis | zyga: I tried to answer to some of your model assertion comments | 08:42 |
| zyga | pedronis: thanks, let me look | 08:42 |
| sergiusens | mvo: so for kernel we will require a base for now (it can be bare) | 08:44 |
| zyga | pedronis: I replied to some but it all makes sense | 08:44 |
| zyga | pedronis: LGTM and iterate or iterate and LGTM, as you prefer :) | 08:45 |
| mup | PR snapd#7345 opened: overlord/devicestate,seed: small step introduce seed.LoadAssertions and use it from firstboot <Created by pedronis> <https://github.com/snapcore/snapd/pull/7345> | 08:50 |
| mup | PR snapd#7346 opened: snapstate: validate all system-usernames before creating them <Created by mvo5> <https://github.com/snapcore/snapd/pull/7346> | 08:51 |
| === Greyztar- is now known as Greyztar | ||
| mvo | zyga: ^- thats the one | 08:51 |
| zyga | /o\ | 08:51 |
| zyga | mvo: did we ship this? | 08:53 |
| mvo | zyga: yeah, also kind of annoying that we overlooked this in the code review | 08:53 |
| mvo | zyga: well | 08:53 |
| zyga | :E | 08:53 |
| mvo | zyga: its in beta | 08:53 |
| zyga | mvo: should such usernames be made at the time the snap is linked? | 08:54 |
| zyga | mvo: I'm looking at this now and it's unclear when this runs | 08:54 |
| pedronis | it runs early but this usernames are shared | 08:55 |
| pedronis | so they will end up staying around either way | 08:55 |
| pedronis | in many cases | 08:55 |
| zyga | pedronis: that's okay, I worry that the sequence now is [check-foo, check-users, check-bar] | 08:55 |
| zyga | and that check-bar fails but we already acted a little on the system, which feels wrong | 08:56 |
| pedronis | as I said, for the current one support user is ok | 08:56 |
| pedronis | but when we add more we need to revisit this | 08:56 |
| pedronis | it probably merits a TODO in that PR though < mvo | 08:56 |
| zyga | mvo: how is the patch making things better tough | 08:56 |
| zyga | it's unclear to me | 08:56 |
| pedronis | we either create all users or none | 08:57 |
| pedronis | we might still fail to install the snap | 08:57 |
| pedronis | later | 08:57 |
| zyga | ah, indeed | 08:57 |
| pedronis | but given that the one support user | 08:57 |
| pedronis | is shared | 08:57 |
| pedronis | it's not too bad | 08:58 |
| pedronis | but it might be worth revisiting at some point | 08:58 |
| mvo | pedronis: indeed, let me add this todo | 08:58 |
| mup | PR snapd#7347 opened: gadget: do not error on gadget refreshes with multiple volumes <Created by mvo5> <https://github.com/snapcore/snapd/pull/7347> | 09:17 |
| zyga | mvo: multi volume as in multi partition? | 09:19 |
| zyga | mvo: or multiple disks | 09:19 |
| mvo | zyga: multiple disks | 09:20 |
| zyga | mvo: do we have customers using that? | 09:20 |
| zyga | or is this just a precaution? | 09:20 |
| mvo | zyga: we have customers using this and this is breaking ondra | 09:20 |
| zyga | +1 | 09:20 |
| zyga | thanks for explaining | 09:20 |
| zyga | mvo: left a suggestion, +1 anyway | 09:22 |
| === vicamo_ is now known as vicamo | ||
| popey_ | zyga: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923500#10 | 09:44 |
| mup | PR #10: Update README.md <Created by chipaca> <Closed by chipaca> <https://github.com/snapcore/snapd/pull/10> | 09:44 |
| zyga | popey_: looking | 09:44 |
| popey_ | is that comment still accurate? | 09:44 |
| zyga | ah that one | 09:44 |
| zyga | it didn't move forward, we discussed accepting partial confinement with actual apparmor enabled but it was nacked for existing distributions | 09:45 |
| zyga | as we don't have the time to deal with regressions if that broke people | 09:46 |
| zyga | that's what the state of this is now | 09:46 |
| popey_ | Worth updating the bug? | 09:46 |
| zyga | it's not a great message but yeah | 09:46 |
| zyga | if only updating bugs in debian didn't involve email | 09:46 |
| pedronis | mvo: approved #7346 with a comment | 09:51 |
| mup | PR #7346: snapstate: validate all system-usernames before creating them <Created by mvo5> <https://github.com/snapcore/snapd/pull/7346> | 09:51 |
| mvo | pedronis: thanks, looking in a wee bit | 09:54 |
| === pedronis_ is now known as pedronis | ||
| * Chipaca takes a break | 10:05 | |
| arnatious | zyga: Container created on Ubuntu 18.04, LXD stable/3.16 snap, container config https://paste.ubuntu.com/p/JrcC89rwN2/ | 10:05 |
| mup | PR snapd#7348 opened: snapstate,store: check assumes early before downloading the snap <Created by mvo5> <https://github.com/snapcore/snapd/pull/7348> | 10:16 |
| mvo | Chipaca: bad news, it seems like reverting 7204 still gives the same error message, I can bisect | 10:25 |
| Chipaca | mvo: rats | 10:26 |
| Chipaca | mvo: is it actually a regression? | 10:26 |
| mvo | Chipaca: not sure, let me try | 10:26 |
| mvo | Chipaca: aha, I think I'm a muppet, hold on, let me retest this | 10:27 |
| Chipaca | mvo: Rowlf? | 10:29 |
| mvo | Chipaca: that sounds about right | 10:29 |
| mvo | Chipaca: maybe "beaker" just needs to be renamed to "breaker" | 10:30 |
| mup | PR snapd#7349 opened: snap: revert PR#7204 as breaks `snap switch` output <Created by mvo5> <https://github.com/snapcore/snapd/pull/7349> | 10:33 |
| mvo | pstolowski: your input here would be great :) | 10:33 |
| Chipaca | mvo: https://www.youtube.com/watch?v=VnT7pT6zCcA | 10:35 |
| pstolowski | mvo: oh :( interesting, saying 'stable' instead will be confusing as well | 10:37 |
| mvo | pstolowski: aha, this is latest/stale vs stable? the new default-channel stuff? | 10:45 |
| mvo | Chipaca: lol - this is great - his expression matches my mood perfectly | 10:45 |
| pstolowski | mvo: yes... and with this revert we'll be showing *wrong* (imprecise/misleading) information when the snapd-side logic of keeping the track kicks in | 10:46 |
| cachio | zyga, hey, when you have time could you please take a look to #7110 | 10:47 |
| mup | PR #7110: tests: new test to check the output after refreshing/reverting core <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7110> | 10:47 |
| cachio | zyga, hope last one | 10:47 |
| mvo | pstolowski: meh, ok | 10:48 |
| mvo | pstolowski: so we need something else I guess :/ | 10:48 |
| mvo | pstolowski: there is a spread test in there that might be useful | 10:48 |
| mvo | pstolowski: the other parts can probably be removed then | 10:48 |
| === alan_g_ is now known as alan_g | ||
| pstolowski | mvo: thanks, +1 and i'll work on this soon | 10:51 |
| mvo | pedronis: I updated 7346 with a unit test, thanks for the suggestion! | 10:55 |
| zyga | cachio: in an hour | 10:55 |
| cachio | zyga, sure, thanks | 10:55 |
| mvo | pedronis: would love your opinion on 7348 too :) but no rush | 11:08 |
| pedronis | mvo: finishing lunch break | 11:09 |
| mvo | pedronis: yeah, no rush, I will soon have lunch too - I was just surprised how small it was | 11:10 |
| pedronis | mvo: commented | 11:17 |
| mup | PR snapd#7350 opened: tests: clean snap_daemon user and group on system-usernames-illegal test <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7350> | 11:19 |
| mup | PR snapd#7351 opened: tests: retry checking until the written file on desktop-portal-filechooser <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7351> | 11:47 |
| xnox | sergiusens: at the moment, `type: kernel` works with any bases, but which base is used matters. As initramfs.img is build with binaries from the base. Thus one should be specified, and it does matter if it's core16, core18, or core20 cause then the initramfs.img has libc6 and etc. from those matching releases xenial / bionic / $devel | 11:52 |
| zyga | re | 11:58 |
| pedronis | xnox: the plan is that kernels should use/specify build-base: not a base: | 12:07 |
| xnox | pedronis: yes, that is good. initramfs.img is self contained / can be used by anyone...... but it matters what it is built out of. More like "Built-Using: bionic" in "dpkg speak", despite having no "Depends:". | 12:09 |
| mup | PR snapd#7346 closed: snapstate: validate all system-usernames before creating them <Test Robustness> <Created by mvo5> <Merged by pedronis> <https://github.com/snapcore/snapd/pull/7346> | 12:20 |
| === ricab is now known as ricab|lunch | ||
| mup | PR snapd#7352 opened: tests: always say 'restore: |' <Simple 😃> <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/7352> | 12:37 |
| zyga | are draft branches visible by default? | 12:38 |
| zyga | I meant draft PRs | 12:39 |
| zyga | https://github.com/snapcore/snapd/pull/7337 needs a 2nd review | 12:44 |
| mup | PR #7337: tests: re-enable mount-ns test on classic <Created by zyga> <https://github.com/snapcore/snapd/pull/7337> | 12:44 |
| pstolowski | jdstrand: hey, one correction to what i said yesterday: i was wrong when i said 'snap connect' could also be optimized for multiple connections at once like i did with automatic connections, i misremembered the code that resolves plug/slot names for connect (was thinking we can resolve multiple plugs/slots if you omit connect arguments, but we only resolve a single plug/slot if you omit them) | 12:45 |
| ijohnson | has anyone seen debian-sid-64 fail to prepare like this: https://pastebin.ubuntu.com/p/GQjg4jK4rw/ ? | 12:52 |
| ijohnson | I've seen it fail to prepare on numerous builds now and it doesn't look like a transient error | 12:53 |
| mup | PR snapd#7353 opened: tests: simplify interfaces-account-control test <Simple 😃> <Test Robustness> <Created by zyga> <https://github.com/snapcore/snapd/pull/7353> | 12:53 |
| mup | PR snapd#7354 opened: tests: rename fuse_support to fuse-support <Simple 😃> <Created by zyga> <https://github.com/snapcore/snapd/pull/7354> | 12:55 |
| mup | PR snapd#7355 opened: [RFC] interfaces/apparmor: load multiple profiles in a single batch <Complex> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7355> | 13:00 |
| Chipaca | augh, 2fa | 13:01 |
| mup | PR snapd#7110 closed: tests: new test to check the output after refreshing/reverting core <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7110> | 13:04 |
| === alan_g_ is now known as alan_g | ||
| === ricab|lunch is now known as ricab | ||
| jdstrand | pstolowski: thanks. it sounded like an exciting feature that I missed :) | 13:55 |
| pstolowski | jdstrand: :) | 14:11 |
| mup | PR snapd#7352 closed: tests: always say 'restore: |' <Simple 😃> <Test Robustness> <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7352> | 14:14 |
| zyga | why does it have to be so hot | 14:17 |
| mup | PR snapcraft#2674 closed: file_utils: introduce link_or_copy_files <Created by cjp256> <Closed by cjp256> <https://github.com/snapcore/snapcraft/pull/2674> | 14:27 |
| mup | PR snapd#7203 closed: i18n, vendor, packaging: drop github.com/ojii/gettext.go, use github.com/snapcore/go-gettext <Created by bboozzoo> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7203> | 14:46 |
| pstolowski | Chipaca: i'm working on a fix for the confusing snap switch message ("Channel %s for %s is closed; temporarily forwarding to..) caused by my #7204; i don't understand the logic of this temporary forward meesage but i assume it's fine to simply skip it for op = "switch" in showDone(), this will basically make "switch" behave as before, modulo the new track/risk behavior that will be shown correctly (thanks to | 14:48 |
| pstolowski | showDone) | 14:48 |
| mup | PR #7204: cmd/snap: use showDone helper with 'snap switch' <Created by stolowski> <Merged by stolowski> <https://github.com/snapcore/snapd/pull/7204> | 14:48 |
| pstolowski | Chipaca: sounds sensible? | 14:48 |
| Chipaca | pstolowski: sounds like the quickest way forwards yes | 14:48 |
| Chipaca | the logic probably assumes 'refresh' | 14:48 |
| === ricab is now known as ricab|brb | ||
| pstolowski | Chipaca: ok, thamks | 14:51 |
| pstolowski | *thanks | 14:51 |
| mup | PR snapd#7356 opened: i18n, vendor, packaging: drop github.com/ojii/gettext.go, use github.com/snapcore/go-gettext (2.41) <Created by mvo5> <https://github.com/snapcore/snapd/pull/7356> | 14:53 |
| zyga | pstolowski: could you please do a pass over the three simple PRs I have | 14:57 |
| zyga | they are all tiny and I'd love to merge them | 14:57 |
| pstolowski | zyga: sure | 15:01 |
| zyga | thanks | 15:01 |
| zyga | there's one -0,+0 PR :D | 15:01 |
| mup | PR snapd#7357 opened: Fix snap switch message <Simple 😃> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7357> | 15:01 |
| pstolowski | zyga: which one? | 15:03 |
| zyga | pstolowski: 7354 | 15:04 |
| mup | PR snapd#7358 opened: Rename die() -> die_reboot() to fix build failures with LTO enabled <Created by dcermak> <https://github.com/snapcore/snapd/pull/7358> | 15:06 |
| zyga | Chipaca: can you look at that, it looks good-ish but I'm worried about it slightly | 15:10 |
| Chipaca | zyga: the switch one? | 15:11 |
| zyga | no, the die one above | 15:11 |
| pstolowski | zyga: did two others, not sure if these were the ones you wanted first. let me know if you need any more | 15:12 |
| zyga | pstolowski: that's great, thank you | 15:12 |
| Chipaca | zyga: the intention was to use the same die from the lib, wasn't it? | 15:18 |
| zyga | Chipaca: original intention or one in this PR | 15:25 |
| cachio | zyga, when you have time, could you please atke a look to #7159 | 15:25 |
| zyga | Chipaca: the original die is not flexible enough | 15:25 |
| mup | PR #7159: tests: add functions to make an abstraction for the snaps <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7159> | 15:25 |
| zyga | Chipaca: I'm looking at fixing that | 15:25 |
| === Luke_ is now known as Guest27501 | ||
| cachio | zyga, and this one also #7256 please | 15:26 |
| mup | PR #7256: tests: adding retry command and use it to delete $XDG directory <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7256> | 15:26 |
| === Guest27501 is now known as Luke | ||
| zyga | cachio: ok | 15:30 |
| cachio | thanks | 15:30 |
| zyga | cachio: does retry argument parsing really work correctly? | 15:31 |
| zyga | cachio: "$@" is expanded once | 15:31 |
| zyga | and it contains all the stuff | 15:31 |
| zyga | you can look at "${1:-}", and while that is not empty parse arguments | 15:31 |
| zyga | the problem with this method is that it just zooms through all the arguments, shifting some (not the one looked at) | 15:31 |
| zyga | ignoring things it doesn't handle | 15:32 |
| cachio | checking | 15:32 |
| zyga | cachio: please | 15:32 |
| zyga | cachio: perhaps it's time to use python :) | 15:32 |
| zyga | cachio: there it's trivial | 15:32 |
| cachio | zyga, works well | 15:34 |
| cachio | zyga, https://paste.ubuntu.com/p/R9mrmSBq65/ | 15:34 |
| zyga | cachio: what happens when you pass call retry rm foo --max-attempts 2 | 15:35 |
| cachio | fails | 15:36 |
| zyga | cachio: but that's not how it was supposed to work | 15:36 |
| cachio | seq: unrecognized option '--max-attempts' | 15:36 |
| zyga | cachio: the command was not supposed to be quoted | 15:36 |
| zyga | cachio: you call it as retry rm -rf "$XDG" | 15:36 |
| zyga | but your example is different | 15:36 |
| zyga | I'm not super happy about the parser, I can fix it tomorrow morning | 15:36 |
| cachio | that works | 15:37 |
| cachio | what fails is when you use --xx at the end | 15:37 |
| cachio | zyga, see that https://paste.ubuntu.com/p/XGNy3ZJRgx/ | 15:38 |
| zyga | cachio: what does shift shifts from? | 15:40 |
| zyga | cachio: when you are mid-iteration in that for loop | 15:41 |
| zyga | cachio: I thought that shift shifts from the left of the "$@" array | 15:41 |
| pedronis | pstolowski: Chipaca: it was very weird that this was landed withotu spread tests: https://github.com/snapcore/snapd/pull/7199/files | 15:43 |
| mup | PR #7199: overlord/snapstate: keep current track if only risk is specified <Created by stolowski> <Merged by chipaca> <https://github.com/snapcore/snapd/pull/7199> | 15:43 |
| pedronis | pstolowski: Chipaca: we should add some | 15:43 |
| pedronis | both for refresh and switch | 15:43 |
| pstolowski | pedronis: ok, i can work on it | 15:46 |
| pedronis | pstolowski: I'm adding a card | 15:47 |
| cachio | zyga, don't get you | 15:47 |
| zyga | cachio: what's the semantics of shift | 15:47 |
| zyga | cachio: and how does it impact the loop | 15:47 |
| zyga | cachio: if you have a loop that goes over 1 2 3 4 5 6 | 15:47 |
| zyga | cachio: and you are at 3 | 15:48 |
| zyga | cachio: what does shift do? | 15:48 |
| pedronis | pstolowski: https://trello.com/c/6oPPAjLD/312-add-spread-tests-for-https-githubcom-snapcore-snapd-pull-7199-for-both-switch-and-refresh | 15:48 |
| zyga | cachio: does it remove "3" or "1" | 15:48 |
| zyga | cachio: I think the loop and the parser is incorrect | 15:48 |
| cachio | zyga, ok, let me check that | 15:48 |
| zyga | cachio: I'm not sure if that's "standard" but when I'm doing shell parsing I'd using a while loop | 15:49 |
| zyga | cachio: checking to see if there are more arguments in "$@" | 15:49 |
| zyga | cachio: looking at the one up front | 15:49 |
| zyga | cachio: handling it, shifting it away | 15:49 |
| zyga | cachio: terminating argument parsing (typically on --) | 15:49 |
| zyga | cachio: or reporting an error | 15:49 |
| zyga | cachio: there are some helpers like getopt as well but those don't always have the right precision | 15:50 |
| cachio | zyga, https://paste.ubuntu.com/p/C94tCPm6F7/ | 15:51 |
| zyga | Chipaca: now swap retry argument order | 15:52 |
| zyga | er, cachio ^ | 15:52 |
| zyga | cachio: the parser is wrong because it shifts something else than it is looking at | 15:52 |
| zyga | cachio: it shifts "$@" - aka the magic array | 15:53 |
| zyga | cachio: it looks at an item out of a snapshot of that array | 15:53 |
| cachio | zyga, I dont see in which case it could fail | 15:54 |
| cachio | when we change the order of the parameters? | 15:54 |
| zyga | cachio: yes, but it's more fundamental, it's just not right in what it is doing | 15:54 |
| mvo | pstolowski: with 7357 - should I close 7349 ? | 15:55 |
| cachio | zyga, to fix that I should create another array and remove items instead of shift? | 15:56 |
| zyga | cachio: I think you can use shift but you must look at the first element at all times | 15:56 |
| zyga | cachio: not at the i-th element | 15:56 |
| pstolowski | mvo: if we are happy about resolution, yes | 15:56 |
| zyga | cachio: but honestly, you can just drop the parsing entirely and hard-code those | 15:56 |
| zyga | cachio: or just use python | 15:56 |
| zyga | cachio: where such things are easier | 15:57 |
| cachio | zyga, rightin python is much easier | 15:57 |
| mvo | pstolowski: I close mine for now then, I'm not sure I understand all the implications yet but it looks like your changes are small and targeted so hopefully | 15:57 |
| mvo | pstolowski: hopefully all fine | 15:57 |
| cachio | zyga, I'll try that | 15:58 |
| mup | PR snapd#7349 closed: snap: revert PR#7204 as breaks `snap switch` output <Created by mvo5> <Closed by mvo5> <https://github.com/snapcore/snapd/pull/7349> | 15:58 |
| cachio | zyga, thanks for reviewing the change! | 15:58 |
| * cachio lunch | 15:59 | |
| zyga | cachio: sure | 15:59 |
| mup | PR snapd#7356 closed: i18n, vendor, packaging: drop github.com/ojii/gettext.go, use github.com/snapcore/go-gettext (2.41) <Created by mvo5> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7356> | 16:00 |
| * zyga breaks to rest until the call later today | 16:04 | |
| mvo | zyga: thanks for holding the fort later! | 16:05 |
| zyga | :) | 16:07 |
| === pstolowski is now known as pstolowski|afk | ||
| pedronis | cachio: this could land https://github.com/snapcore/snapd/pull/7259 but probably needs a master merge to get green | 16:25 |
| mup | PR #7259: tests: just build snapd commands on go-build test <Simple 😃> <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7259> | 16:25 |
| mup | PR snapd#7260 closed: tests: add a runtime scripts generation to generate scripts to call functions <Created by sergiocazzolato> <Closed by pedronis> <https://github.com/snapcore/snapd/pull/7260> | 16:28 |
| mup | PR snapd#7338 closed: tests: move restore-project-each code to existing function <Simple 😃> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7338> | 16:28 |
| mup | PR snapd#7353 closed: tests: simplify interfaces-account-control test <Simple 😃> <Test Robustness> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7353> | 16:28 |
| cachio | pedronis, merged, waiting for test results in green | 16:39 |
| cachio | pedronis, thanks | 16:40 |
| mup | PR snapd#7354 closed: tests: rename fuse_support to fuse-support <Simple 😃> <Created by zyga> <Merged by mvo5> <https://github.com/snapcore/snapd/pull/7354> | 16:42 |
| * ijohnson lunches, bbiab | 17:43 | |
| mup | PR snapd#7359 opened: overlord/snapstate: check channel names on install <Created by anonymouse64> <https://github.com/snapcore/snapd/pull/7359> | 17:44 |
| mup | PR snapd#7337 closed: tests: re-enable mount-ns test on classic <Created by zyga> <Merged by zyga> <https://github.com/snapcore/snapd/pull/7337> | 18:11 |
| === Luke__ is now known as Luke | ||
| mup | PR snapd#7259 closed: tests: just build snapd commands on go-build test <Simple 😃> <Created by sergiocazzolato> <Merged by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7259> | 18:30 |
| mup | PR snapd#7360 opened: snap: use deterministic paths to find the built deb <Created by sergiusens> <https://github.com/snapcore/snapd/pull/7360> | 19:28 |
| sergiusens | mvo: https://github.com/snapcore/snapd/pull/7092#issuecomment-525448848 | 19:29 |
| mup | PR #7092: packaging: use snapd type and snapcraft 3.x <â›” Blocked> <Created by stolowski> <https://github.com/snapcore/snapd/pull/7092> | 19:30 |
| sergiusens | mvo: cachio I added a link to a built snapd there ... a comment yay or nay on the snapcraft PR mentioned there would be grand. | 19:30 |
| cachio | sergiusens, ok, thanks | 19:32 |
| mup | PR snapcraft#2690 opened: remote-build: error when --user is required <Created by cjp256> <https://github.com/snapcore/snapcraft/pull/2690> | 20:31 |
| mup | PR snapd#7361 opened: tests: enabling ubuntu 19.10-64 on spread.yaml <Created by sergiocazzolato> <https://github.com/snapcore/snapd/pull/7361> | 21:03 |
| mup | PR snapd#7362 opened: cmd: unify die() across C programs <Created by zyga> <https://github.com/snapcore/snapd/pull/7362> | 21:13 |
| mup | PR snapd#7363 opened: cmd/snap-confine: fix group and permission of .info files <Bug> <Created by zyga> <https://github.com/snapcore/snapd/pull/7363> | 21:17 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!