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
improve perl shebang lines by switching to use lib ...;
#55786
Conversation
# TODO, handle this better? | ||
# `perl5.28.1-Test-Simple-1.302141` bakes line numbers into the expected errors in tests | ||
# inserting a "use lib ...;\n" into the file offsets everything and makes the tests fail | ||
if [[ "$name" == *"Test-Simpl"* ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're going to have to fix TestSimple before merging this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we can just disable it's tests until we have a better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better solution, just patch things after testing!
6d489ac
to
313a50d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! If we do decide to go this route, I would recommend adding some test or a setup-hook to make sure these lines don't exceed the 128 limit. I seem to think python had a similar issue?
fwiw @matthewbauer upstream is reverting or fixing the commit to retain the previous behavior, so it won't be such a hard blocker again |
Only 10kb =) (wrt https://gist.githubusercontent.com/dtzWill/5d4324eb3ef9d3c7dfbb0f690768dc91/raw/217d62d8ff88a2579eaf79e5a7c35445044f0be3/biber)
|
Somewhat off-topic. How about the latency due to stat calls with the old and new method? I know in case of Python we should avoid many different |
yeah, i was initially going to merge everything together with |
Are you sure that searching one lndir would be noticeably more efficient that searching multiple directories? |
the merged directory will basically act as an index, the same as what |
But... following the symlink is almost like the original search, isn't it? EDIT: I mean, it feels difficult to be sure without measuring it, as it might be a negligible difference. |
@vcunat following the symlink to a single dir is still cheaper then checking 20 different dirs |
It is cheaper, but you end up checking (usually) most of them through those symlinks anyway. Without symlinks you'll do it more times than with them (over the same set), but I'd suspect that kernel dentry caching might make the difference negligible. Still, I do not know for sure. |
Well, symlink to a directory of symlinks allows to open files via a single syscall, instead of unsuccesful `open` on a dozen locations. I would expect two symlinks lookups (at least with warm block-level caches) to be cheaper than an extra context switch.
|
Right, certainly significantly cheaper in terms of context switches. |
What are the chances someone in here is going to implement a .withPackages for perl? This would be really useful. I would love to implement this, but am not yet skilled enough with nix to do so... :-( |
Apparently I cannot assign to non-team-members. |
@volth thank you very much! I'm also not able to assign the issue to you... |
This should probably be based on staging due to the amount of rebuilds. |
There is a |
Motivation for this change
this stops the perl shebang lines from reaching 30kb in size
Things done
confirmed that
nix-build -A biber
builds and the existing tests in the perl stack still passsandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)