| 7 Oct 2025 |
lovesegfault | i don't want to be blocked on ci for the s3 work to land | 23:35:37 |
lovesegfault | i need to be done with this stuff, it's been almost two months | 23:36:08 |
Sergei Zimmerman (xokdvium) | Yeah sure, let me see what we can do about enabling your work | 23:36:08 |
lovesegfault | i'm worried shifting prios at work will force me to just drop it entirely, which would be a shame | 23:36:38 |
lovesegfault | for me at least, i've loved working on this stuff | 23:36:45 |
Sergei Zimmerman (xokdvium) | Eh, I guess we can go ahead and clean that up later :) | 23:36:47 |
lovesegfault | i'll time box fixing ci to tonight | 23:37:10 |
lovesegfault | but let me give it a shot at least | 23:37:30 |
lovesegfault | i want to leave things better than i found them | 23:37:36 |
Sergei Zimmerman (xokdvium) | Merged for now :) Sorry for being a pain. We can do incremental improvements | 23:39:11 |
lovesegfault | not a pain at all, i share your goals and concerns here :) | 23:39:31 |
lovesegfault | btw, does this help with perf concerns? https://github.com/NixOS/nix/pull/14171/files#diff-d6730623c42446aa1f33e3177f7146d342ca8d5103e0814bb0b7ac212af5576dR90-R99 | 23:41:43 |
lovesegfault | idk if the str scan is fast enough to help | 23:41:54 |
Sergei Zimmerman (xokdvium) | I was thinking that we could just special-case '.' and '..': '.' does nothing and '..' does a pop. Also I don't think the rhs of this operator is ever supposed to have a '/' there | 23:43:07 |
Sergei Zimmerman (xokdvium) | I see you've added the tests for such cases, but isn't that supposed to be handled by CanonPath::extend? | 23:45:46 |
Sergei Zimmerman (xokdvium) | There's the comment:
/**
* Add a path component to this one. It must not contain any slashes.
*/
| 23:46:27 |
Sergei Zimmerman (xokdvium) | Maybe the right thing is to instead fix the user code? | 23:46:38 |
lovesegfault | i wasn't sure what could end up there realistically, just wanted to avoid having that weird assert thrown at users | 23:46:50 |
lovesegfault | yeah, okay, maybe we only handle dots | 23:46:57 |
| @adam:robins.wtf left the room. | 23:48:18 |
Sergei Zimmerman (xokdvium) | Also maybe it's the downstream code that needs fixing. I think the idea for CanonPath (and this function) was to not accept . .. at all | 23:48:19 |
lovesegfault | that i'm not sure about. I'll do your idea of special-casing ./.. and we can bring it up in the next meeting? | 23:48:55 |
lovesegfault | done | 23:54:15 |
lovesegfault | now onto the ci work | 23:54:41 |
| 8 Oct 2025 |
lovesegfault | oh, that was easy | 00:03:18 |
lovesegfault | i think | 00:03:20 |
lovesegfault | https://github.com/NixOS/nix/pull/14184 | 00:05:40 |
lovesegfault | Sergei Zimmerman (xokdvium): i think i achieved what you wanted, unless you wanted there to only be one allTests attrset that has componentTests and vmTests | 00:11:17 |
lovesegfault | nice, addressing the review | 00:13:22 |
Mic92 | @xokdvium:matrix.org: you will notice something about this merge: https://github.com/NixOS/nix/pull/14180 | 04:02:51 |