WIP: feat: trivial builders V1 #24
No reviewers
Labels
No labels
Compat
Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: auxolotl/labs#24
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "austreelis/feat/trivial-builders-v1"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds trivial builders:
trivial.text(nixpkgs.writeTextFile)runtime dependenciestrivial.command(nixpkgs.runCommand)PATHconfigurationpreferLocalBuildtrivial.concattrivial.text(nixpkgs.concatTextFile)trivial.join(nixpkgs.symlinkJoin)I'll be expanding/checking that todo-list as I go.
commits are supposed to be reviewed independantly (though I might cherry-pick restructure them later to make them cleaner).
Examples were added to showcase how these can be used, where
aux.dis ajoinof all of them:closes #10
trivial.join0bd20a9502This is highly non-definitive, but I don't really know if the build function is quite right (I don't know what I'm doing).
Also, the example package (
d.nix) is broken until I get the other trivial builders to produce something useful for it to consume.0bd20a9502tof780a7698c@ -0,0 +15,4 @@description = "Derivations to join under a single store path";type =letinitial = lib.types.attrs.of lib.types.derivation;What's the motivation for accepting an attrset at all here?
I just followed the package's
deps.xx.yy, and I agree with its design that will allow someyhing likeWhich really is saner than
with ...; [ x y z ];Hmmm, fair enough. I don't have a particular preference towards either (although I will admit
withis a little yucky), but it makes sense to have the option.@ -0,0 +108,4 @@passAsFile = [ "content" ];});in# FIXME: surely we have an assertion system to use instead of this ? right ?I don't think we have one yet. Probably should do that somewhere...
opened aucolotl/lib #8
@ -0,0 +42,4 @@Path of the program used to execute this file as a script.If set, a shebang is prepended to the file.'';type = lib.types.nullish lib.types.path;Should support passing flags to the interpreters as well.
Added it, I'd like to know how you like it :)
f780a7698cto9e30c5b82f9e30c5b82fto5bfeb1cecbI initially wanted to support
nixpkgs.writeShellApplication-styleruntimeDeps, but if we don't want to assumesettings.interpreter.executableis a shell (bash and co), we'd need to wrap the resulting script to setPATH, so I'll be leaving it for later, and won't do it in this PR.5bfeb1cecbtoe841e5ea11And here I go double-posting to say that I thing this PR only needs cleanup and aftercare, I have implemented everything I wanted ! I'd appreciate a review pass, I don't plan to touch the API again or change the builders' behavior :3
I would suggest quoting variables always, using
x()overfunction x, using[[over[, and moving thethen/dointo the same line as theif/for. That's the most common formatting/constructs used for bash and afaik there's no reason to use[over[[ever. And it looks like you wanted to unify the log functions, I would also recommend that.(Also I'm personally not a fan of set -u because it needs you to write weird constructs like ${var:-} and most people are not used to it, so imo it's trading one pitfall for another, but fine enough if you want the additional strictness)
@ -0,0 +16,4 @@echo -n "$1$sep" >&2doneecho "${1:-}" >&2}This entire function is not used, is it?
@ -0,0 +22,4 @@echo "[PANIC] $1" >&2while shift && [ -n "${1:+x}" ]do echo -e "$1" >&2doneWhy break if the argument is empty? That seems like a pitfall.
@ -0,0 +34,4 @@echo -n "$1, " >&2doneecho "${1:-}" >&2}@ -0,0 +38,4 @@function debug { true; }if ((NIX_DEBUG > 0 ))then {@ -0,0 +46,4 @@echo -n "$1, " >&2doneecho "${1:-}" >&2}This is the same as info() except for the tag. Is that what log() was supposed to be for?
upsy yes, I intended
debug,infoandpanicto uselog@ -0,0 +96,4 @@local path="${3:-}"entry="${entry:+"${entry%%/}/"}$path"entry="${entry:-/}"# entry="$(normalize_path "$entry")"Was a normalize_path function supposed to be implemented here?
@ -0,0 +143,4 @@while read -d $'\n' -r pathdopush_entry "$origin" "$entry" "$path"done < <( \Pretty sure you don't need most of the \ here.
@ -0,0 +173,4 @@# # shellcheck disable=2154 # $out is an input# if ! mkdir "$out" 2>/dev/null# then panic "output directory $out already exists"# fiWhy is this here commented out?
Yeah I need to clean this up, I'll get to it when I do a cleanup pass.
I'm not used to writing bash, so re
function, quoting,[[etc. I'll take your word for it. The only thing I know for sure is that[ -z ${myVar:+x} ]never requires quoting (it's either""or"x"so it cannot word-split again, and[ -z ]evaluates to false). But I guess quoting doesn't hurt.I do prefer setting -u, I know it's a convention I've seen in nix spaces, and I'd rather document bash's weirdness than try to avoid it I think (though I'm definitely open to changing my mind if I'm the only one holsing that belief).
Thanks for reviewing the scripts, it's dedinitely not my strong point !
e841e5ea11to35741de622I stuffed this through hydra even though it's a WIP - it's failing with the same eval errors as #22, I guess it needs a rebase?
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.