worldofpeace changed the topic of #nixos-dev to: NixOS Development (#nixos for questions) | NixOS 20.09 Nightingale ✨ | | | 20.09 RMs: worldofpeace, jonringer |
<Irenes[m]> is there any plausible way to make the notifications less spammy?
<Irenes[m]> and does anybody have a link where I can see what it's doing
<Irenes[m]> purely out of curiosity
bennofs_ has joined #nixos-dev
<ekleog> Does the bot send a message after each push, or only on the first one?
<rmcgibbo[m]> - One thing that was discussed was for me to change r-rmcgibbo to "merge" the two comments it often make (one for aarch64-linux and one for x86_64-linux) into one comment by having it edit its previous commit if it sees that it's already commented
<rmcgibbo[m]> - Another thing was to start using the Github Checks API to display either build successes, build failures, or both. There's some uncertainty about how the suggestions should or should not get incorporated into the Checks API that requires some experimentation.
<rmcgibbo[m]> - Another thing that was discussed is if the bot could post a "review" rather than a comment, although personally I think it's more spammy.
<Irenes[m]> those all seem worth investigating
<rmcgibbo[m]> Right now r-rmcgibbo runs nixpkgs-review only after the initial creation of the github PR, not after each push to the PR. But in general it does run twice, once for each arch it has support for.
<rmcgibbo[m]> (Although it doesn't post it's review for a given architecture if anyone else has already posted a nixpkgs-review review on the PR for that architecture or if the PR modifes only 1 attr and ofborg already built it)
<ekleog> Hmm then at least option 1 and ideally option 2 (which would make it run once after each push) sound like the best option(s) to me too — one message per new PR is OK-ish, but it'd be better if the information were updated on each push IMO
<rmcgibbo[m]> And the super-near-term-like-right-now fix for this is that if you tell me that you don't want me to post notifications for "build successes" on your PRs and you give me your GH username, I can add you to the "block list" and it will take effect within the next 10 minutes.
<supersandro2000> I don't think we need after each push
bennofs__ has quit [Ping timeout: 272 seconds]
<supersandro2000> ofborgs evals are then enough and if not you can manually run it to maybe check a thing or two
<abathur> Irenes[m]: here's one, if you haven't already found one:
<Irenes[m]> thanks for the summary
<Irenes[m]> ah! I hadn't, much appreciated
<supersandro2000> abathur: thanks for linking that
<supersandro2000> rmcgibbo[m]: I think I have another suggestion for nixpkgs-hammering if you are open to it
<supersandro2000> no-flags-array should probably not complain about buildFlagsArray
<Irenes[m]> yeah it seems like that stuff should go in the check output if at all possible
<supersandro2000> even tho it is valid
<hexa-> whats the point of automatically reviewing r-ryantm pull requests?
<rmcgibbo[m]> Can you file a ticket with Jan Tojnar for that, Sandro?
<supersandro2000> rmcgibbo[m]: I can do that
<Irenes[m]> it's clear that the checks API supports detailed reports because some of the existing checks produce them
<rmcgibbo[m]> @hexa-: Well, IMHO it gives the maintainers who should be reviewing the update some information about the fact that it continues to build on a second architecture.
<hexa-> rmcgibbo[m]: ah fair, but a second x86-64 check wouldn't make sense
<rmcgibbo[m]> As well as flagging if updating that package breaks any downstream packages on a second architecture.
<hexa-> r-ryantm already builds dependant packages
<rmcgibbo[m]> That's right, and I've already added a feature to the bot so that it doesn't post a second x86-64 check.
<hexa-> nice
<rmcgibbo[m]> It only posts for architectures that nobody has nixpkgs-review'd for already.
<rmcgibbo[m]> (In the PR body or a previous comment)
<supersandro2000> you probably grep the comments for that, right?
<supersandro2000> so it will most likely break with my modified one. Maybe we should add a hidden string if thats the case
<hexa-> maybe we should invest more in ofBorg though
<rmcgibbo[m]> supersandro2000: I'm grepping the prior comments for `NEEDLE = f"run on {system} [1]("`, so far I don't think there have been any false negatives from that.
<abathur> can't speak for the bot-makers, and I may not be in the know, but I've been interpreting the bot efforts as a sign that there's a missing <automation-incubator>
<supersandro2000> well I heard it can't be easily tested which is not ideal
<supersandro2000> rmcgibbo[m]: that will only work for newer nixpkgs-reviews. Older ones are missing that and I think the one on 20.09 did not receive that change as a backport
<rmcgibbo[m]> Okay, I'll keep an eye out for false positives and can add another search string no problem. I think that's a minor point though.
<supersandro2000> just wanted to let you know.
<Irenes[m]> I think automated lint rules and tests are certainly important
<Irenes[m]> it is also important to be mindful of developer experience
<Irenes[m]> I have no particular authority or anything, I'm just expressing my personal opinions. I'm not a particularly active contributor either, or I'd say thank you for making the bot
<Irenes[m]> I do hope there are ways to make it less of an interruption and more of advice that's there when you need it
<rmcgibbo[m]> Yeah, 100%
<supersandro2000> I think part of the issue could be blamed on GitHub by providing a very strict set of tools which do not fulfill all our needs
<jtojnar> I think for the third point, check annotations might be nicer since they do not result in notifications
<rmcgibbo[m]> Jan Tojnar: Maybe this weekend, I/we should test out the checks API.
<Irenes[m]> I do think that the "checks" functionality meets all the needs, yeah
<Irenes[m]> oh I see, three bullet points
<Irenes[m]> I think checks clearly meet need (1) and honestly, I'd consider using them for (2) and (3) as well
<Irenes[m]> the thing about comments is that it co-mingles programmatic output with human output
<Irenes[m]> and makes it harder to see just the human stuff
<Irenes[m]> even aside from notifications, it makes it harder to catch up on the issue
<Irenes[m]> I've worked in environments where there was a lot of stuff like that, it was really important to have tools for cutting through the clutter
<Irenes[m]> I think if you don't want to make an X that's fine, it can be "check succeeded but please read" or something
supersandro2000 has quit [Disconnected by services]
supersandro2000 has joined #nixos-dev
<rmcgibbo[m]> Yeah, my concern about the checks API -- and maybe this is completely wrong because I just haven't experimented with it and I'm under as misimpression -- is that you can either throw up a red X, a green check mark, or a yellow "in progress" bullet.
<rmcgibbo[m]> And I'm not yet sure if this gives a compelling way to show information that kind of has a "middle ground" quality to it, like a suggestion that you may or may not want to follow or a build failure that could have been due to my infra being flaky.
<rmcgibbo[m]> But that's why I need to experiment with the checks API. Maybe it's perfect.
<supersandro2000> you can also set the status to grey
bennofs__ has joined #nixos-dev
<rmcgibbo[m]> Another thing to think about specifically for posting build failures using the checks API for me is that the check will then be tied to a specific commit sha, and so the existence of the failure will kind of "go away" if the PR author posts another commit to the PR that doesn't actually fix the build failure (e.g. because they're addressing a cosmetic suggestion or something), and that might mean I probably should
<rmcgibbo[m]> switch to re-building every commit to the PR which is going to be more expensive. Not a big deal, just something to think about.
<ryantm> rmcgibbo: One reason to do the x86_64 build again on r-ryantm PRs is you could detect reproducibility issues.
bennofs_ has quit [Ping timeout: 240 seconds]
<supersandro2000> Do reproducibility issues normally propogate?
<ryantm> I think so, so maybe not so useful.
<rmcgibbo[m]> (@ryantm: I am actually running them on x86 even for r-ryantm, I am just stopping right before posting. I could start posting them if they show a failure, or if some other condition is met)
<ryantm> Probably need to wait until reproducibility is more normal.
rajivr has joined #nixos-dev
orivej has quit [Ping timeout: 265 seconds]
mkaito has quit [Quit: WeeChat 3.0]
disasm has quit [Quit: WeeChat 2.0]
disasm has joined #nixos-dev
abathur has quit [Ping timeout: 256 seconds]
abathur has joined #nixos-dev
charukiewicz has joined #nixos-dev
lovesegfault has quit [Quit: WeeChat 3.0]
lovesegfault has joined #nixos-dev
bennofs__ has quit [Ping timeout: 265 seconds]
<srk> I'm wondering why 'systemctl reload postgres' sometimes hangs when used in acme postRun. for nginx there's with comment `# postRun hooks on cert renew can't be used to restart Nginx since renewal runs as the unprivileged acme user.` but as of 67a5d66 it runs as root so I'll fix that comment
bennofs_ has joined #nixos-dev
<srk> reload service looks like a better idea anyway. nixos-mailserver does systemctl reload nginx/postfix/dovecot2 in postRun tho
<srk> not even sure if reload which triggers config reload is enough for postgres to load new certificates
bennofs_ has quit [Ping timeout: 265 seconds]
bennofs_ has joined #nixos-dev
jonringer has quit [Ping timeout: 260 seconds]
bennofs_ has quit [Ping timeout: 272 seconds]
saschagrunert has joined #nixos-dev
bennofs_ has joined #nixos-dev
orivej has joined #nixos-dev
charukiewicz has quit [Quit: charukiewicz]
charukiewicz has joined #nixos-dev
cole-h has quit [Ping timeout: 240 seconds]
<roberth> does anyone use a metasyntax for function parameters? Seems like a useful way to briefly mention them in documentation
<roberth> for example mkDerivation:buildInputs
teto has joined #nixos-dev
<roberth> perhaps mkDerivation{buildInputs}
__monty__ has joined #nixos-dev
<sterni> but don't all functions taking an argument set have so many args that a short hand notation just gets exploded
<infinisil> > generators.toPretty evalModules
<{^_^}> value is a function while a set was expected, at (string):472:1
<infinisil> > generators.toPretty { multiline = false; } evalModules
<{^_^}> "<function, args: {args?, check?, modules, prefix?, specialArgs?}>"
<infinisil> roberth: This is how toPretty shows it
bennofs_ has quit [Remote host closed the connection]
<roberth> sterni: that's for full signatures, which looks good afaic, but I'm trying to refer to individual arguments
<roberth> mkDerivation{buildInputs} does align with `toPretty` notation
<roberth> I guess it looks too much like a function signature
<roberth> mkDerivation:buildInputs looks more like a namespace operator, which isn't as meaningful but less misleading
<V> roberth: I assume you mean like how ruby has #methods
<V> String#include?
bennofs_ has joined #nixos-dev
<roberth> V: kind of like that, yes. Except referring to parameters instead of functions
<roberth> I did consider `#` but using that in url fragments seems like asking for trouble
<sterni> roberth: maybe put a :: between name and signature like in the documentation comments in lib?
<sterni> mkDerivation :: { buildInputs?, ... }
<roberth> sterni: that also look like I'm referring to `mkDerivation` rather than `buildInputs`
<sterni> ohhh
<sterni> sorry missed what you were getting at
<roberth> np
<__monty__> mkDerivationArgs.buildInputs maybe?
<roberth> that's kind of ambiguous
<infinisil> Maybe ->
<infinisil> mkDerivationArgs->buildInputs
<roberth> looks like a function signature
<roberth> maybe just buildInputs in mkDerivation
<__monty__> buildInputs@mkDerivation? The colon doesn't look any more obvious to me than all the other suggestions.
<__monty__> If this is about natural language I'd just go with either that or mkderivation's buildInputs.
<infinisil> Yeah I also like the colon
<roberth> buildInputs@mkDerivation overloads args@{...} but still kind of ok
orivej has quit [Ping timeout: 265 seconds]
<roberth> I'll stick with natural language for now
<roberth> thanks for the input everyone
saschagrunert has quit [Remote host closed the connection]
evanjs has quit [Quit: ZNC 1.8.2 -]
saschagrunert has joined #nixos-dev
evanjs has joined #nixos-dev
bennofs_ has quit [Ping timeout: 246 seconds]
orivej has joined #nixos-dev
bennofs_ has joined #nixos-dev
<qyliss> worldofpeace: should #112879 be a draft maybe, since it looks like there's still stuff to do before it's ready?
<{^_^}> (by worldofpeace, 2 hours ago, open): nixos/dbus: support dbus-broker
bennofs_ has quit [Quit: No Ping reply in 180 seconds.]
bennofs_ has joined #nixos-dev
bennofs_ has quit [Remote host closed the connection]
bennofs_ has joined #nixos-dev
bennofs_ has quit [Ping timeout: 240 seconds]
bennofs_ has joined #nixos-dev
orivej has quit [Ping timeout: 240 seconds]
bennofs_ has quit [Quit: No Ping reply in 180 seconds.]
bennofs_ has joined #nixos-dev
saschagrunert has quit [Remote host closed the connection]
AlwaysLivid has joined #nixos-dev
bennofs__ has joined #nixos-dev
bennofs_ has quit [Read error: Connection reset by peer]
jonringer has joined #nixos-dev
bennofs__ has quit [Ping timeout: 240 seconds]
bennofs_ has joined #nixos-dev
<nbp> hum … this is not reassuring, when you notice that your system depends on 2 openssl: /nix/store/wmhdjm0x0n4ffqh908gkhah3zr1d29fd-openssl-1.1.1i and /nix/store/xivj0vwm0pjpmwsjrc8qcca9fcfyxm4b-openssl-1.1.1i
<nbp> nix-store -qR /var/run/current-system | sort -t- -k2 | less
<nbp> Ok coreutils depends on *4b-openssl-1.1.1i, and the rest on the other.
<nbp> Sounds like a staging artifact.
pmy has quit [Quit: WeeChat 3.0.1]
pmy has joined #nixos-dev
vcunat has joined #nixos-dev
<vcunat> Hydra seems down. I hope it won't remain that way over the whole weekend...
cole-h has joined #nixos-dev
<aminechikhaoui> hm looks like the database server run out of disk space
<aminechikhaoui> well I don't know much about the zfs setup there. gchristensen around ?
mkaito has joined #nixos-dev
AlwaysLivid has quit [Remote host closed the connection]
<supersandro2000> DBIx::Class::Storage::DBI::catch {...} (): DBI Connection failed: DBI connect('dbname=hydra;host=;user=hydra;','',...) failed: could not connect to server: Connection refused
<supersandro2000> that almost leaked credentials
rajivr has quit [Quit: Connection closed for inactivity]
<hexa-> on a private network
<supersandro2000> There are probably hydras which are on a public network and could leak things this way.
<supersandro2000> or maybe it always cuts the password. Just wanted to raise the question.
<cransom> i *think* that the password is configured in a .pgpass file and the debug string you see is the selector for db/host/user to select that password. i don't think it can be logged, at least not from perl
<cransom> and if i look at my hydra, yes, the DBI string host/user/db/dbname. the password is tucked away in pgpass.
<supersandro2000> that sounds not to bad
<hexa-> the only language i know that leaked credentials was php
orivej has joined #nixos-dev
AlwaysLivid has joined #nixos-dev
Synthetica has joined #nixos-dev
<Synthetica> What's going on with
<hexa-> Synthetica: check the backlog
<siraben> this package has no maintainers but I got pinged, probably because of git blame #112929
<{^_^}> (by fruit-in, 18 minutes ago, open): package proxychains is broken
<samueldr> siraben: in the issue body, it's the user that added this, so yeah, maybe they looked at the history of the file :)
<samueldr> but nothing automated
<siraben> heh, right.
<srk> siraben: I'm using it, will take a look
<siraben> srk: thanks
<worldofpeace> <qyliss "worldofpeace: should #112879 be "> there are a few things but I wouldn't consider it a draft. I don't really need to care about apparmor at all, I think that's just relevant if we want to default to it, because we'd have to mention that if you used dbus apparmor before you'll need to disable dbus-broker
<{^_^}> (by worldofpeace, 8 hours ago, open): nixos/dbus: support dbus-broker
<worldofpeace> so I guess it's just "ready for review". It does work but it probably needs a bit of polish that I couldn't see at first try
<qyliss> oh cool okay
<qyliss> I was gonna say the same thing about apparmor
cole-h has quit [Ping timeout: 240 seconds]
orivej has quit [Ping timeout: 240 seconds]
<gchristensen> aminechikhaoui: looking
orivej has joined #nixos-dev
<gchristensen> Fails To Build From Source --
red[evilred] has joined #nixos-dev
<red[evilred]> Yay! Thanks jtojnar (IRC) and Tad Fisher <3
<red[evilred]> (for the gtk4: init at 4.0.2 :-)
rj has joined #nixos-dev
<Mic92> Interesting. I was expecting that they can build gcc from source
orivej has quit [Ping timeout: 246 seconds]
<infinisil> Not sure how I feel about this..
<{^_^}> #108739 (by thiagokokada, 5 weeks ago, open): nixos/redshift: use config file instead of passing parameters, using RFC42
<infinisil> (marvin-mk2 spam)
<infinisil> Shows the same "Reminder: Please review!" message every 3 days
<qyliss> that is definitely spam
<qyliss> cc timokau[m]
rj has quit [Quit: rj]
rj has joined #nixos-dev
<vcunat> gchristensen: thanks :-) (or to anyone who revived Hydra)
<gchristensen> turned out the backup software is a bit silly :)
<samueldr> [17:07:22] <joepie91> hmm... is sending wrong headers. claims to be application/json but it is definitely not that
<timokau[m]> qyliss, infinisil Yes, it should not repeat the message every 3 days. See the disclaimer on the bottom of the message.
<joepie91> oh, good point, should probably have said that here :)
<joepie91> hm. it also does not seem to be xz, and `file` doesn't know either...
<gchristensen> < Content-Encoding: br
<gchristensen> < Content-Type: application/json
<gchristensen> $ curl | brotli -d -> {"version":1,"root
<joepie91> hm, I missed the content-encoding, and so did both httpie, curl, and firefox apparently?
<supersandro2000> displays json in chrome
<gchristensen> brotli is not well supported by most tools
<joepie91> Firefox just yells at me about broken JSON :P
<joepie91> is there a non-brotli version available of this?
<supersandro2000> nixos curl does not support it. debian does
<gchristensen> no, because nix uploads a lot of these files so it picks one that seems to do a realyl good job and sticks with it
<joepie91> supersandro2000: I find a strange sort of irony in that :)
<supersandro2000> going to enable it by default
<adisbladis> Iirc it inflated the curl closure by a ton
<joepie91> gchristensen: hmm, right. if nix-index is any indication, it seems that sometimes xz is available and sometimes brotli, then
<supersandro2000> use curlFull in nix-shell
<supersandro2000> which is silly
<gchristensen> joepie91: yeah it has changed a handful of times through the years
<joepie91> right
<joepie91> now that I'm here anyway, might as well ask: has anyone written a thing for scraping appstream data out of the cache yet?
<supersandro2000> 1.13MB vs 719KB
<adisbladis> supersandro2000: It's not just about the runtime closure
<supersandro2000> it adds libidn, openldap and brotli
<supersandro2000> still not much
<adisbladis> Ummm...
<supersandro2000> at least the curl for end users should be full
bennofs_ has quit [Quit: No Ping reply in 180 seconds.]
<adisbladis> supersandro2000: Wait a minute... What are those sizes?
<adisbladis> They make no sense
<supersandro2000> let me create a PR and I check the stats on how much it increases calculation time.
<supersandro2000> wdym? I missed the i before the b
<adisbladis> I mean what are they a measurement of
rj has quit [Ping timeout: 268 seconds]
<supersandro2000> closure size
<adisbladis> That makes zero sense
<adisbladis> What incantation did you use to get those numbers
<supersandro2000> I probably know why they are disabled by default. They cause an infinite recursion.
<supersandro2000> nix path-info -S with the closure path of a somewhat recent curl
<adisbladis> But a curl closure is significantly more than 1.13M
<adisbladis> So idk how you arrived at those numbers
<supersandro2000> by eg ``nix path-info -S /nix/store/qbylbwp53fk9dy4sn74ylxnnsdadb2m1-curl-7.74.0.drv -h``
<qyliss> supersandro2000: you're checking the closure size of the derivation
<qyliss> not of the built curl
<qyliss> % nix path-info -Shf . curl
<qyliss> /nix/store/0znanflfl1jj9yprja7f1kl47xy7l3li-curl-7.74.0-bin 40.4M
<supersandro2000> oh yeah. makes sense
<supersandro2000> 40.4M vs 56.6M
bennofs_ has joined #nixos-dev
<adisbladis> supersandro2000: I think you're comparing curl/curlFull
<adisbladis> not just enabling brotli?
<supersandro2000> yeah I do
<supersandro2000> the others might not be as useful but my idea was to just swap the full variant with a minimal variant
<supersandro2000> so if you want a small one you can use curlMinimal and normally you get the bigger one
<supersandro2000> but just fiddling around with it currently
<adisbladis> I seem to recall cmake being a culprit in all of this
<supersandro2000> yeah it is
red[evilred] has quit [Quit: Idle timeout reached: 10800s]
<supersandro2000> cmake won't get brotli support for now
<adisbladis> That's not the problem
<adisbladis> Brotli is built using cmake
<supersandro2000> ldap support makes 14M. If we disable that by default we only go to 42.1M which is not much more I think
__monty__ has quit [Quit: leaving]
vcunat has quit [Quit: Leaving.]
<{^_^}> #112947 (by SuperSandro2000, 2 minutes ago, open): curl: enable brotli by default
<hexa-> why would you want ldap support in curl?
<supersandro2000> I am not sure. If we can find a consensus we can remove curlFull.
<supersandro2000> I wanted to add a http3 flag but it either requires boringssl or a patches openssl. Skipping that for now
<hexa-> well, most people use curl for http/https, so brotli and idn are probably in scope, ldap and gss feels weird
<supersandro2000> fully agree with you
<samueldr> remember that curl is the library too
<supersandro2000> just wanted to keep the disruption low
<supersandro2000> I think right now it only enables new things and does not break anything
<supersandro2000> rmcgibbo[m]: jtojnar: what do you think about nixpkgs-hammering asking people to check if hardeningDisable is really required?
<rmcgibbo[m]> that sounds good. to the extent that we can discourage hardeningDisabled that seems like it would benefit everyone
<qyliss> please don't do that
<qyliss> in many cases it is required, and you'll just add to the noise
<adisbladis> What qyliss said
<gchristensen> I don't think it is a good idea to make it a goal to get rid of curlFull
<gchristensen> there is probably an 80/20 rule that applies to featureset and user need
<gchristensen> and a 16M growth is fairly significant in the face of probably almost no users needing it
<samueldr> if it was possible to detect that a comment explaining the need for each hardeningDisabled values, it'd be more acceptable, but we don't have a way to properly "annotate" I guess
<samueldr> is missing*
<qyliss> samueldr: that comment would go out of date, and we'd end up in the same predicament we're currently in
<qyliss> where then we start asking people to check the comment is up ot date
<samueldr> at least there would be an indication as to why it was added
<qyliss> hardeningDisabledLastChecked = version :P
<samueldr> verifying the comment is still needed is a different issue
<samueldr> here I assumed it was about *introducing* new hardening
<samueldr> new disables*
<samueldr> but if it's about all the time... there's no way it's sustainable :)
<qyliss> I'm assuming this is a conversation of the issue I raised in #nixos-security that almost every instance of hardeningDisable appears to be unnecassry (builds succeed without them, at least)
<samueldr> in my experience, a chunk of issues happen at runtime
teto has quit [Quit: WeeChat 3.0.1]
<samueldr> (which makes this a really annoying issue to gauge!)
<samueldr> and if there is no explanation as to why it was added, it's hard to tell whether it is still an issue or not :/
<qyliss> yeah
<qyliss> what I've been doing so far is checking out the commit where it was added, and checking if the build would have failed if it wasn't added
<qyliss> if that's the case, I assume it's safe to remove now, and otherwise I leave it alone
<qyliss> I think that should be pretty bulletproof as a strategy but it's a bit slow and manual
<samueldr> sounds good enough when there is a known failure at build originally yeah
<jtojnar> supersandro2000, rmcgibbo: I think it would be nice to add warning if it builds without it and there is no comment
<jtojnar> need a third category of checks (build in addition to eval and syntax) and multi-modal checks first, though
<rmcgibbo[m]> :+1:
<supersandro2000> 👍