[06:11] morning [06:19] hi! [06:20] mardy: hey [06:20] mardy: anything i can help you with today? :) [06:24] mborzecki: yeah, I'm about to push a fix for the unit tests, but I can't really say that I like how the code in that commit looks like :-) But I'll ping you soon [06:25] mardy: https://github.com/snapcore/snapd/pull/10266 to this pr? [06:25] PR #10266: wrappers/services.go: do not restart disabled or inactive services [06:28] mborzecki: yes, please see the last commit I just pushed [06:30] mardy: hmm, fwiw there's `systemctl try-restart` which seems to be doing what we want, but i'm not sure how far back it is supported (eg. on 14.04?) [06:59] mardy: posted some comments in the PR [07:08] morning [07:08] pstolowski: hi! [07:08] mborzecki: thanks, I applied some changes, and added a couple of questions to your comments :-) [07:16] pstolowski: hey [07:16] mardy: yeah, sorry missed that, perhaps adding a little comment in the test would be helpful so that we don't have to wonder for too long when one revisits the test in a longer while [08:09] hi. I'm having "Unexpected error when obtaining your account information" when trying to snapcraft login via my CI pipelines, any idea what that means? [08:12] thresh: temporary store issues maybe (looks ok right now per https://status.snapcraft.io)? or is it a permanent failure? [08:14] pstolowski, looks like it's been happening consistently for a few days now. I guess it's time to refresh the tokens... [08:15] thresh: i see, or ask on the forum [08:26] indeed, it was probably an expired login information [08:26] ok, good you sorted that out [09:13] more selinux fun, ehh [09:36] heh, filed https://bugzilla.redhat.com/show_bug.cgi?id=1960576 [09:36] looks like we shoudl tweak the tests a bit to make it less painful [09:43] mborzecki: nice bug report / analysis [10:28] igor's LAS talk is coming up soon on https://www.youtube.com/watch?v=iJeANBfLVHQ [10:30] mborzecki zbyszek replied quickly [10:36] zyga: yeah, sounds like a weekend project for me ;) [10:36] mborzecki s/weekend/workday/ but yeah : [10:37] zyga: not sure about workday, the snapd todo queue is long [10:43] mardy: btw. we try not to force push to a branch once there have been some comments added to already present code [10:45] mardy: also that means for branches taking a while to land, merging master into the branch is preferred to rebasing (unless nobody reviewed it yet, then rebase is ok) [10:45] github made some improvemens to the ui for presenting diffs when rebase occured, but it's still not quite there yet [10:47] mborzecki: ah, I see. I was using gitlab before, and there things work quite nicely [10:49] yeah, gitlab has it's own set of ui quirks ;) [10:59] mardy: you'll need to run gofmt -w -s .. over the files you touched in your snap restart PR; the catch though is you need to use gofmt from go-1.10 (because this is the version we use in our tests, newer version may produce different formatting) [11:01] Igor is live now [11:01] at https://www.youtube.com/watch?v=iJeANBfLVHQ [11:03] zyga: cool, thanks for the link [11:20] mardy: a few comments to your PR [11:30] pstolowski: thanks! Is there a way to locally run the code formatter on the spread tests? [11:30] mardy: which formatter? gofmt? [11:32] mardy: if it's about go code formatting, you can just run gofmt commnad (or gofmt -w which will reformat and write the file) [11:33] mardy: if you're using emacs then you can use hooks 😛 no clue about other editors though [11:33] s/editors/operating systems with text editing capabilities/ [11:44] mborzecki: no, I was wondering about bash formatting (the spread tests) [11:45] ah, then we have no rules, i know there are some tools but haven't tried anything [11:45] there's shfmt but I haven't used it myself [11:46] mborzecki: it's about https://github.com/snapcore/snapd/pull/10266#discussion_r632443590 [11:46] PR #10266: wrappers/services.go: do not restart disabled or inactive services [12:04] mborzecki: will I be able to squash the commits before the merge? Just trying to understand how I should format the fixup commits now [12:05] mardy: i see, you can run spread-shellcheck tests/main/services-restart and it will do the right checks [12:08] ah, but I don't have my spread account yet [12:13] mardy: yes, you can git rebase -i master at any time (and force-push), or squash-merge when merging [12:14] but we generally only squash-merge when needed for easy cherry picking [12:16] mardy: do no't need spread account for that, spread-shellcheck is just a thin wrapper around shellcheck that extract the 'shell' bits from spread task definitions and feeds them to shellcheck to verify correctness [12:19] mborzecki: where do I find it? [12:19] mardy: it's at the root of snapd tree [12:20] mardy: https://github.com/snapcore/snapd/blob/master/spread-shellcheck [12:20] mardy: then run `./spread-shellcheck tests/main/services-restart` [12:28] oh :-) I ran a `git grep` and didn't find anything! With git sometimes I forget that I can just run `ls` :-) [12:34] PR snapcraft#3523 opened: autotools v2 plugin: adding cross compilation support === Eighth_Doctor is now known as Conan_Kudo === Conan_Kudo is now known as Eighth_Doctor [13:18] mborzecki: do you know any reason we don't fail unit tests if dbus-launch is missing? seems a bit risky, we wouldn't even notice [13:20] pstolowski: hm the packaging runs unit tests (not our but downstream) and dbus-launch may not be there (or be runnable) [13:20] at least the packaging for EPEL7 or amazon linux may be tricky in that regard [13:21] right [13:48] cachio_: can you check whether there are any images avialble for opensuse 15.3? [13:50] the release is due in june, it'd be great to have it part of the spread suite by that time [13:50] mborzecki, let me check [13:52] hmm hmm we set snap-setup on 2 tasks during install, but the first one is prerequisites [13:52] mborzecki, there are images for 15.1 and 15.2 [13:52] cachio_: ah ok, thanks for checking! [13:53] also no images here https://download.opensuse.org/repositories/Cloud:/Images:/ [15:32] mborzecki, as debian 9 is required [15:33] it is not possible to merge the fedora-34 pr [15:33] we need mvo [15:33] to remove debian-9 as required [15:33] or perhaps force marge it [17:14] mborzecki, could you please +1 to #10249 [17:14] PR #10249: tests: adding support for fedora-34 [17:14] Bug #10249: gpm: FTBFS in sarge: Documentation fails to build because of mismatched parentheses [20:15] * cachio_ afk === RzR is now known as rZr