gchristensen changed the topic of #nixos-borg to: https://www.patreon.com/ofborg https://monitoring.nix.ci/dashboard/db/ofborg?refresh=10s&orgId=1&from=now-1h&to=now "I get to skip reviewing the PHP code and just wait until it is rewritten in something sane, like POSIX shell. || https://logs.nix.samueldr.com/nixos-borg
<samueldr> I see that "breaks listing of package outputs" is `FailWithGist`, is it a limitation of GitHub where failures don't have a "Details" link? https://github.com/NixOS/ofborg/blob/2bf51bcbcf5db21cde9118475964b50e88096aec/ofborg/src/tasks/eval/nixpkgs.rs#L148-L152
<gchristensen> hmm
<gchristensen> samueldr: tbh, I would more likely bet a bug with ofborg. can you link me the PR?
<samueldr> I don't know rust much, a match with Err(eval::Error::Fail) then Err(eval::Error::FailWithGist) couldn't have matched with Fail first since it's listed first, right? no inheritance in matching?
<{^_^}> #59313 (by eyJhb, 9 hours ago, open): displaylink: mark broken on 19.03
<gchristensen> right
<samueldr> and the other day I looked at a couple failed checks and none of them show a gist url for failure to list outputs
<samueldr> (that was the other day when we had a user jump in and ask, then ping timeout; tried to find their PR and check)
<gchristensen> interesting
<gchristensen> iirc a failure to create the gist is logged and ignored
<gchristensen> to the logs!
<gchristensen> ...
<gchristensen> to the other computer, whicd has access to the logs!
<gchristensen> nope, more mysterious
<gchristensen> failure to make a gist blows up the evaluator
<gchristensen> Apr 11 15:00:47 eval-1-lassulus.ewr1.nix.ci ofborg-evaluator-start[24286]: Ok(File { fd: 5, path: "/tmp/#7080299 (deleted)", read: true, write: true })
<gchristensen> Apr 11 15:00:48 eval-1-lassulus.ewr1.nix.ci ofborg-evaluator-start[24286]: Ok(File { fd: 5, path: "/tmp/#7080299 (deleted)", read: true, write: true })
<gchristensen> Apr 11 15:01:34 eval-1-lassulus.ewr1.nix.ci ofborg-evaluator-start[24286]: INFO:ofborg::outpathdiff: Failed to find After list
<gchristensen> Apr 11 15:03:39 eval-1-lassulus.ewr1.nix.ci ofborg-evaluator-start[24286]: INFO:ofborg::tasks::evaluate: Working on 59315
<samueldr> not sure what you mean. It failed to make the gist or didn't?
<gchristensen> it did not fail to make the gist
<gchristensen> but also maybe it didn't try to
<samueldr> oof :)
<gchristensen> at least the evaluator is not 1,000,000,000 lines in a single function now.
<gchristensen> ... ok I don't understand what it is doing.
<gchristensen> oh dammit!
<gchristensen> oaethoaeuthdaontuh
<samueldr> explain for the crowd not-in-your-head?
<samueldr> oh, sorry
<gchristensen> hrm?
<samueldr> I read "ok I [...] understand what it is doing"
<gchristensen> looks good, right?
<samueldr> the don't was critical
<samueldr> yeah
<samueldr> that's why I asked about the match
<gchristensen> https://github.com/NixOS/ofborg/blob/released/ofborg/src/commitstatus.rs#L35-L37 set_url doesn't actually action anything
<gchristensen> if those two lines were swapped, it'd Do The Thing
<samueldr> isn't it set_url that makes the details link work on successes?
<gchristensen> set_url then set
<samueldr> hm ah!
<samueldr> `set` is like `yes tell github about it`, right?
<gchristensen> yep
<gchristensen> this is a horrible API! :D
<samueldr> yeah, confirmation asked from just after reading that bit
<samueldr> that's... I wouldn't expect set to do such an action :)
<samueldr> (from the name)
<gchristensen> yeah :(
<gchristensen> horrible!
<samueldr> apparently you didn't either ;)
<gchristensen> :D
<gchristensen> hmm I had some good code uncommitted in my local ofborg repo
<gchristensen> do I dare deploy that too tonight?
<samueldr> how grumpy do you expect to be when you wake up? :)
<{^_^}> [ofborg] @grahamc pushed to fix/fail-with-github « set_with_description actually does the thing, set_url is lies »: https://git.io/fjq7t
<samueldr> before saying failures, I mean
<gchristensen> :P
<samueldr> seeing failures*
<{^_^}> [ofborg] @grahamc opened pull request #348 → set_with_description actually does the thing, set_url is lies → https://git.io/fjq7q
<{^_^}> [ofborg] @grahamc pushed 2 commits to released: https://git.io/fjq7m
<{^_^}> [ofborg] @grahamc merged pull request #347 → Add lua topic paths → https://git.io/fjqul
<{^_^}> [ofborg] @grahamc merged pull request #345 → Add Taneb to config.extra-known-users.json → https://git.io/fjUzh
<{^_^}> [ofborg] @grahamc pushed 2 commits to released: https://git.io/fjq7Y
<gchristensen> samueldr: https://github.com/NixOS/ofborg/pull/342/files shouldn't this point to nixos not nixpkgs?
<{^_^}> [ofborg] @grahamc merged pull request #340 → Enable tagging for TeX PRs → https://git.io/fjJHq
<{^_^}> [ofborg] @grahamc pushed 2 commits to released: https://git.io/fjq7s
<samueldr> gchristensen: uh? what do you mean "point to nixos"?
<{^_^}> [ofborg] @grahamc closed pull request #274 → README.md: switch to workaround for running tests → https://git.io/fpRa3
<gchristensen> ah
<gchristensen> shouldn't it import ./nixos/release-combined.nix or something instead of nixpkgs' release?
<samueldr> right
<samueldr> my bad
<samueldr> pkgs/top-level/release-combined.nix isn't even a thing
<samueldr> terrible since in my PR body I used the right filename :o
<gchristensen> :)
<gchristensen> wow, new clippy has really fairly ugly suggestions
<samueldr> just pushed the fixed one
<samueldr> (this time actually copy/pasted the strings from the command in a local nix-instantiate invocation!)
<gchristensen> .map(|k| k.to_owned()) -> .map(std::borrow::ToOwned::to_owned)
<samueldr> I see why, but that's more bytes
<samueldr> s/bytes/chars/
<gchristensen> I wouldn't actually want to type that
<samueldr> that's one syntaxic sugar bit I like from ruby, enumerable.map(&:something) is like enumerable.map { |x| x.something }
<samueldr> with as many previously rubyists on rust, I though they would have had a similar sweetened syntax already :)
<gchristensen> they do, but it has to be carefully typed
<gchristensen> 319 | .filter(|line| line.is_ok())
<gchristensen> | ^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::result::Result::is_ok`
<gchristensen> they're moving my cheese, samueldr!
<samueldr> though, not sure about how "it has to be carefully typed" matters :/ here `line` is presumably a `std::result::Result`, anything like 🧀::is_ok would work, where the compiler resolves the cheese to the type?
<gchristensen> not sure
<samueldr> there's probably a huge gap in my rust knowledge where it would make sense not to have something like that
<samueldr> but considering the whole "don't use a closure" clippies, such a syntax would make sense to me?
<samueldr> though, I sure agree that "adding syntaxes for the sake of syntaxes" is also a good thing not to do
<{^_^}> [ofborg] @grahamc pushed to fix/fail-with-github « clippy: disable redundant_closure »: https://git.io/fjq7K
<{^_^}> [ofborg] @grahamc pushed to fix/fail-with-github « clippy: disable redundant_closure »: https://git.io/fjq7X
<{^_^}> [ofborg] @grahamc merged pull request #348 → set_with_description actually does the thing, set_url is lies → https://git.io/fjq7q
<{^_^}> [ofborg] @grahamc pushed 3 commits to released: https://git.io/fjq7Q
<{^_^}> [ofborg] @grahamc pushed 0 commits to fix/fail-with-github: https://git.io/fjq77
<gchristensen> samueldr: https://github.com/NixOS/nixpkgs/pull/59313 done
<{^_^}> #59313 (by eyJhb, 10 hours ago, open): displaylink: mark broken on 19.03
<samueldr> nice
<samueldr> <3 gchristensen doing good work
<{^_^}> gchristensen's karma got increased to 101
<gchristensen> samueldr: however, the problem is surprisingly obvious. obvious enough that I was blind to it
<{^_^}> [ofborg] @grahamc pushed 4 commits to github-token: https://git.io/fjq5l
<{^_^}> [ofborg] @grahamc pushed to github-token « fixups »: https://git.io/fjq5M
<{^_^}> [ofborg] @grahamc opened pull request #349 → GitHub token vending machine → https://git.io/fjq5y
orivej has quit [Ping timeout: 252 seconds]
andi- has quit [Ping timeout: 258 seconds]
andi- has joined #nixos-borg
andi- has quit [Excess Flood]
andi- has joined #nixos-borg
andi- has quit [Ping timeout: 250 seconds]
andi- has joined #nixos-borg
orivej has joined #nixos-borg
andi- has quit [Ping timeout: 252 seconds]
andi- has joined #nixos-borg
andi- has quit [Ping timeout: 264 seconds]
andi- has joined #nixos-borg
<{^_^}> [ofborg] @grahamc merged pull request #349 → GitHub token vending machine → https://git.io/fjq5y
<{^_^}> [ofborg] @grahamc pushed 6 commits to released: https://git.io/fjqx9
<{^_^}> [ofborg] @grahamc pushed to fix-build « travis: run nix-build on release.nix »: https://git.io/fjqpI
<{^_^}> [ofborg] @grahamc opened pull request #350 → travis: run nix-build on release.nix → https://git.io/fjqpL
<{^_^}> [ofborg] @grahamc pushed to fix-build « travis: run nix-build on release.nix »: https://git.io/fjqpG
jtojnar has joined #nixos-borg
<{^_^}> [ofborg] @grahamc merged pull request #350 → travis: run nix-build on release.nix → https://git.io/fjqpL
<{^_^}> [ofborg] @grahamc pushed 2 commits to released: https://git.io/fjqpR
<{^_^}> [ofborg] @grahamc pushed 0 commits to fix-build: https://git.io/fjqpE
orivej has quit [Ping timeout: 264 seconds]
orivej has joined #nixos-borg
<gchristensen> w00t my nice looking code worked a treat
orivej has quit [Ping timeout: 246 seconds]
<{^_^}> [ofborg] @grahamc pushed 0 commits to github-token: https://git.io/fjqhR
orivej has joined #nixos-borg
<LnL> :D
<gchristensen> one step closer to ofborg being able to do CI for NixOS/nix
<samueldr> gchristensen: about that PR you asked me to fix, I did fix it yesterday (the one testing nixos/release-combined.nix)
<gchristensen> ah right
<gchristensen> I held off on deploying it last night for the reasons you mentioned (not waking up to 100 broken PRs)
<gchristensen> let's give it a go
<samueldr> though yeah with the right path it will work fine :)
<samueldr> it will catch those few that break release-combined evals without affecting eval for nixpkgs
MichaelRaskin has joined #nixos-borg
orivej has quit [Ping timeout: 252 seconds]
<gchristensen> ok friends, question about publishing statistics
<gchristensen> I was thinking it'd be good to publish the statistics for the outpaths.nix evaluation with checkMeta = true
<gchristensen> but we don't checkMeta before and after, only after
<gchristensen> should we checkMeta before and after?
<gchristensen> should we publish statistics for outpaths with checkMeta=false?
<gchristensen> some of this code is so ugly =)
<infinisil> gchristensen: Soo, you have before/checkMetaTrue, before/checkMetaFalse, after/checkMetaTrue and after/checkMetaFalse?
<infinisil> (the possible measurements)
<gchristensen> those are all the possible ones
<gchristensen> at the moment, I do before/checkMetaFalse, after/checkMetaFalse after/checkMetaTrue
<infinisil> Why does checkMetaFalse matter at all though?
<infinisil> Just for getting rid one of the major performance bottlenecks?
<gchristensen> I think it is good to verify it evaluates properly with check meta = false
<gchristensen> I'm not sure why we chose to do checkMeta = false before/after instead of =true
<gchristensen> possibly to start least strict and become more strict
<infinisil> Oh I see
orivej has joined #nixos-borg
<infinisil> For performance statistics, I don't think after/checkMetaTrue is useful on its own, because you can't compare it to anything
<gchristensen> it isn't
<gchristensen> it is only useful for a "did this PR mess up meta checks?"
<gchristensen> so maybe the thing to do is do checkmetatrue before/after
<gchristensen> and do a checkmetafalse in the after
<infinisil> But then you don't have the less strict -> more strict thing
<gchristensen> yeah
<gchristensen> I could stop doing that, that probably isn't super important
<samueldr> is the hydra eval with checkMeta true?
<gchristensen> no
<samueldr> then yeah, I'm not sure with true has much worth, though I don't know for sure
<samueldr> (stats with it true, checking is important!)
<infinisil> before/checkMetaFalse succeeds but after/checkMetaFalse fails -> Error is in your change, not in meta
<gchristensen> also a valid point
<infinisil> after/checkMetaFalse succeeds but after/checkMetaTrue false -> Error is in your change, in meta
<infinisil> And I think before/checkMetaFalse and after/checkMetaFalse is good enough to record performance over time
<infinisil> imo at least :)
<gchristensen> I wouldn't count on these #'s being useful "over time" just an before vs. after diff in the PR
tilpner has quit [Remote host closed the connection]
tilpner has joined #nixos-borg
<gchristensen> ok I'm sold
<gchristensen> I was thinking checkMeta=true for "worst case" stats, but
orivej has quit [Ping timeout: 250 seconds]