asymmetric has quit [Read error: Connection reset by peer]
asymmetric has joined #nixos-dev
FRidh has joined #nixos-dev
justanotheruser has quit [Ping timeout: 268 seconds]
<qyliss>
I've noticed staging PRs being added to a GitHub Project recently. What does that mean? Is there some kind of special process for staging PRs? Should I not be merging them like before?
<gchristensen>
probably keep doing your thing, maybe someone is experimenting
<qyliss>
okie
<FRidh>
qyliss: I started with this recently. It's mostly just for myself to keep track of all the PR's for staging so I don't forget about any
<qyliss>
keep track of them for what reason?
<FRidh>
qyliss: to get them in in a reasonable time. It's not uncommon they linger when no one pushes for them.
* qyliss
nods
<qyliss>
I worry that doing this with a project just called "Staging" will make them linger even longer -- people might assume somebody else is taking care of them
<qyliss>
Or even, is explicitly claiming responsibility for them.
<qyliss>
That's how it felt to me, at least.
averell has quit [Remote host closed the connection]
<FRidh>
Maybe. So far the majority of people don't merge them themselves in staging, either because they're not comfortable with it, don't know they're allowed to, forget about it, or indeed think someone will do it for them. Let's see if people comment about it.
<FRidh>
I would just like to see more reviewing of these PR's, with an explicit approved. Maybe if they see "needs review" that will trigger them?
<gchristensen>
the staging process to me still feels a bit washy
<gchristensen>
I mean, I don't understand it
<FRidh>
yea, we really need a small staging job
<FRidh>
Even without its been a big improvement though. Before it would happen almost all the time that during early stabilisation someone would push another mass-rebuild postponing the whole delivery by another day or two. Now that does not happen anymore.
<hyperfekt>
Does anyone have an inkling how much of an effect wrapping the stdenv cp with reflink=auto would have?
<hyperfekt>
I keep finding myself using that argument in build scripts but I can't tell if it's actually worth it - or if it is, maybe it should be in nixpkgs....
<gchristensen>
what filesystems support that?
<qyliss>
I've always understood that I should merge to staging just like I merge to master
<hyperfekt>
the CoW ones (ZFS, btrfs, bcachefs, possibly XFS?)
<qyliss>
And then ??? somehow those changes end up in master
<qyliss>
There's staging-next, and somebody (FRidh?) fixes up all the build failures
<qyliss>
And then they arrive in master and there's a new staging-next
<qyliss>
That's my understanding.
<gchristensen>
zfs doesn't support cp with reflink
<hyperfekt>
But auto falls back to normal copy if it's not supported. My understanding is that this only isn't the default behavior because it breaks the expectation of redundancy people have of cp.
<qyliss>
But it is not a very solid understanding
<qyliss>
I'd love to see a talk on it.
<hyperfekt>
gchristensen: Well, I guess ZoL doesn't support a lot of things..
<qyliss>
The bot can only do it for them if they checked the appropriate checkbox, anyway
<qyliss>
But we don't need heuristics -- we should be able to accurately detect when this has happened.
<gchristensen>
if we do it after it has happened, it is too late
<qyliss>
If you're proposing merging commits into master that are already in staging, or vice versa, but are also adding new commits, that's almost certainly a bad base.
<qyliss>
gchristensen: not necessarily
<qyliss>
some code owners get notified when the PR is opened
<qyliss>
nothing we can really do about that
<gchristensen>
yeah but that is fine
<qyliss>
But _even more_ get notified when it's rebased improperly
<gchristensen>
right
<qyliss>
I'd say most of the notifications I get that I shouldn't get come from it being rebased
<gchristensen>
and how do we detect an improper rebase before it happens?
<qyliss>
We can't in the general case
<qyliss>
But we can detect the case where somebody is trying to merge 300 commits from master into staging
<gchristensen>
ah
<qyliss>
Which happens a lot and will almost always lead to a bad rebase because of GitHub's terrible UI
<qyliss>
This isn't a silver bullet
<qyliss>
But a big proportion of the invalid notifications I get would be addressed by this.
<qyliss>
Not 100%, or even 50%, but more than 10% I reckon.
<qyliss>
The other thing we could do is detect invalid rebases and notify people after the fact of how to do it next time
<qyliss>
So at least it should only happen once per person.
<FRidh>
gchristensen: indeed, those suggestions
<FRidh>
gchristensen: by the way...I *really* would like to see a way to give information based on paths that are changed. E.g. , if something in python-packages.nix is changed, the post will contain a snippet with instructions or so about that
<globin>
worldofpeace, jtojnar: I'm writing a blog post on how structured attrs works
<globin>
as a lot of people aren't really aware of even its existence
<globin>
there is also a .attrs.sh
<worldofpeace>
globin: right, some docs will be needed eventually though. but a blog post will be more accessible atm
<jtojnar>
globin++++
<jtojnar>
globin++
<{^_^}>
globin's karma got increased to 9
<globin>
yeah docs are needed when a lot of the work on the branch builds & probably an rfc even berfore that
<globin>
but at least I think I'm aware of the issues there are with it and how to fix them now
<jtojnar>
I would also like to see some explanation how does `placeholder` actually work
<gchristensen>
it writes out special marker bytes which are nulled during hashing
<jtojnar>
I am mostly interested in when is the marker rewritten to the actual output path
<gchristensen>
ah
<globin>
jtojnar: during eval placeholder "out" is replaced by the hash of out (primops.cc) and then nix-build iterates over all drvs outputs and replaces them back (libstore/build.cc), grep for hashPlaceholder in nix for the exact code location
<jtojnar>
hmm, that means we cannot really pass placeholder of a package to substituteAll
<clever>
its a constant string, no matter what package its a placeholder for
<clever>
and ive found it eratic as to which attrs will even get it replaced in, havent tracked down why
<jtojnar>
it would be nice to extend it with derivation argument, but that still would not work for the substituteAll use case, since substituted patch would depend on the package and package on the substituted patch
<clever>
in the past, ive wanted to put an absolute path into a .desktop file
<clever>
but the function to make the .desktop file is a derivation
<clever>
and i cant copy that drv into the thing it has a path to
<clever>
infinisil: i would have done { some.option.foo = "foo"; some.option.bar = mkIf "bar"; };
<clever>
infinisil: that avoids the // and i believe still behaves the same way
<clever>
though your PR to solve the other errors is still good
<infinisil>
clever: Ah true, could be done that way in that case
<clever>
another thing, is that with mkMerge from your example, nixos cant know what keys exist in some.option, and that may cause infinite recursion
<clever>
none of the values in some.option can depend on the others (i think)
<clever>
while with `some.option.bar = mkIf bool "bar"`, it knows that .bar may have a value (which might be default or undefined)
<clever>
and lazyness can route around it
<infinisil>
Yeah I've had to debug this too at some point
<clever>
part of it, is that attribute sets are strict in the key names
<clever>
so, you must be able to resolve every key in the set, and create thunks for each value, before you can index into a certain key
<clever>
so you must know if .bar is present or not, before you can read .foo
<clever>
some.option.bar = mkIf bool "bar"; means that .bar technically does exist
<clever>
but if the bool was false, and there is no default, accessing it will throw an exception