-
-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
notes from nixpkgs#173885 #80
Comments
Preserving this since there are at least a few things in here that need to get individually fixed ~ASAP. |
🤔 what if, for every fatal error, resholve added a link to the wiki in the message, as shellcheck does.
Like: https://github.com/koalaman/shellcheck/wiki/SC1118 (Range SC1000~SC3060) It could even link to a blank page, GitHub allows you to toggle the option to "Restrict editing to collaborators only". |
That (or something similar) is the long-term intent. There are a few boxes I'd like to check there...
I tend towards over-thinking, and error messages and systems of appropriate codes are both things I can easily waste a lot of time what-ifing about. I tried to sneak up on this about a year ago, as you can see in this diff, but I was spinning my wheels on specifics and decided to just freeze that effort for a bit (I think it makes sense to privilege features over docs in the short run, and at the time I was still trying to land all of this "lore" work). This fall/winter I sorted out the basics of the documentation single-sourcing part, and started modularizing resholve's code a bit (to make it easier to generate the exceptions). I guess that's all to say that the time is close to right. Probably a this-year sort of thing. |
I'm making 2 posts addressing this. The first focuses on things you noted that I've fixed today. The 2nd will just record some thoughts on the open questions...
Correct. You found a bug in resholve's parser for timeout. For illustration I addressed it in separate test/fix commits:
Good spot. I fixed it in 99cd196. Both of these will need to get released and propagated to nixpkgs; not sure how quickly I'll cut a release. |
Yes. Sorry that one's a bit unhelpful, but I'm intentionally making this a little disruptive. I'm hoping to make more people aware of the underlying problem to build pressure for trying to find a fix up at the Nix/nixpkgs layer.
I noticed this one as well. I'm not quite sure what the "right" fix will be. The ideal way to do it would be something like: fix = {
# ...
"$scriptfolder" = [ "lib/airgeddon/" ];
"$language_strings_file" = [ "language_strings.sh" ];
"$known_pins_dbfile" = [ "known_pins.db" ];
# ...
}; But...
But as I write this out, I wonder if this would work (on macOS atm, not easy to just test...): fix = {
# ...
"$scriptfolder" = [ "./" ];
"$language_strings_file" = [ "lib/airgeddon/language_strings.sh" ];
"$known_pins_dbfile" = [ "lib/airgeddon/known_pins.db" ];
"$rc_file_name" = [ "share/airgeddon/.airgeddonrc" ];
# ...
};
I'm not a tmux user so I'm just kinda shooting from the hip here. If the tmux arguments are very regular, it may just be a matter of creating a parser for it in resholve. Searching through the codebase for tmux invocations didn't make it terribly obvious what all commands are getting passed through it, since it seems like most of these are in utility functions. But, I did see a lot of run-time code-generation behavior (such as https://github.com/v1s1t0r1sh3r3/airgeddon/blob/master/airgeddon.sh#L3732-L4142) that looks like the kind of thing I'd usually mark in my notes as as too dynamic for resholve to have much hope of handling. This may still be fine if you don't actually need to resolve commands down in these dynamic structures (for example, if the entire thing is just using shell functions that do get resolved)? If they do hold external executable references, resholve will likely end up being a ~code-intel tool for you here, with the real solution needing to be some wrapping/patching combo? resholve also has
I liked what you came up with--I've been doing the same. I haven't quite decided if resholve needs a special lever for cases like this (there's a related case with scripts that will use the first found of N alternative programs for something--curl OR wget and is a common example) or just better documentation that fits the scenarios.
Aha. Hehe. Should've kept reading. Yes--the heredocs aren't really intelligible as Shell to resholve. If a general wrapper approach doesn't work for some reason, perhaps patching it to inject a generated PATH after the |
I'll give you a step-for-step introduction of what I did and what I found, maybe this can help you to help me:
I first took a look at Kokadas' derivation and saw that it was primarily rounded on that "solutions" entry after adding the "mkDerivation". So I searched how to write one of those, which led me to this README that I've used as the source of truth.
My first doubt was, "Should I use the paths relative to input or output?" and well, that was written there, "$out-relative paths".
The second was an observation that I've seen in Kokadas' code. He was using "sh" instead of "bash" as interpreter, which led me to ask him and then discovering bash includes a posix-mode.
Then a small try after just adding the "scripts", "interpreter", and the "inputs" I've already known lead me to that regex parsing issue of oilshell, which I solved with the sed
s|^(.+) =~ ([^\$].+) ]]|regexp='\2'; \1 =~ $regexp ]]|
that I based in a commit from you in 2021.After that I had to deal with a
Can't resolve dynamic argument in 'source'
-- that's because airgeddon sources its scripts withsource "${scriptfolder}${rc_file_name}"
and well, I don't know how to correctly solve these even now 🙈 -- Should I replace this with relative-hardcoded-paths, I know that I can't use absolute-hardcoded-paths -- Kokada pointed me to "keep.source
" but this didn't help, "fix
" somehow did the trick for the "strings" and "known_pins", and for the "plugins" in a little hacky way. But I had to leave ".airgeddonrc" out of this and use "keep
" on it. (very :lost: even now)At this point, I had already taken a look at the resholve docs. And well, I've started to add all the deps that weren't documented and resholve was finding 🏆 . That lead me to these issues in the way:
"There is not yet a good way to resolve 'ping' in Nix builds." (Which don't come up with any suggestion, I've added it as an external)
"timeout" had a "15" where the lore was expecting an "executable argument" (No clue how to say it to skip only this parameter, so I changed it to "cannot", which I'm pretty sure I should not)
you repeat
"execer"
twice here:resholve/docs/examples/lore.nix
Line 14 in f3faceb
"tmux" is executing its arguments, even worse, airgeddon uses "send-keys" to throw commands in it. (What should I do here? Again I went with "cannot")
I had to come up with a hacky solution to keep optional dependencies optional, using the "external".
In the end, it ran, but the scripts in heredocs inside it were not using absolute references. They were even using the unpatched shebang. Airgeddon starts running one of these inside tmux and that wasn't finding the executables.
Originally posted by @PedroHLC in NixOS/nixpkgs#173885 (comment)
The text was updated successfully, but these errors were encountered: