improve perl shebang lines by switching to use lib ...;#55786
Closed
cleverca22 wants to merge 1 commit intoNixOS:masterfrom
Closed
improve perl shebang lines by switching to use lib ...;#55786cleverca22 wants to merge 1 commit intoNixOS:masterfrom
use lib ...;#55786cleverca22 wants to merge 1 commit intoNixOS:masterfrom
Conversation
grahamc
reviewed
Feb 14, 2019
Member
There was a problem hiding this comment.
we're going to have to fix TestSimple before merging this :)
Member
There was a problem hiding this comment.
Perhaps we can just disable it's tests until we have a better solution.
Contributor
Author
There was a problem hiding this comment.
better solution, just patch things after testing!
GrahamcOfBorg
added
10.rebuild-darwin-stdenv
cleverca22
force-pushed
the
fix-perl-shebang
branch
from
6d489ac to
313a50d
Compare
matthewbauer
approved these changes
Feb 14, 2019
Member
matthewbauer
left a comment
matthewbauer
left a comment
There was a problem hiding this comment.
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?
Member
|
fwiw @matthewbauer upstream is reverting or fixing the commit to retain the previous behavior, so it won't be such a hard blocker again |
Member
|
Member
|
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 |
Contributor
Author
|
yeah, i was initially going to merge everything together with |
Member
|
Are you sure that searching one lndir would be noticeably more efficient that searching multiple directories? |
Contributor
Author
|
the merged directory will basically act as an index, the same as what |
Member
|
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. |
Contributor
Author
|
@vcunat following the symlink to a single dir is still cheaper then checking 20 different dirs |
Member
|
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. |
Member
|
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.
|
Member
|
Right, certainly significantly cheaper in terms of context switches. |
Member
|
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... :-( |
Member
|
Apparently I cannot assign to non-team-members. |
Member
|
@volth thank you very much! I'm also not able to assign the issue to you... |
10 tasks
10 tasks
Member
|
This should probably be based on staging due to the amount of rebuilds. |
Member
|
There is a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation for this change
this stops the perl shebang lines from reaching 30kb in size
Things done
confirmed that
nix-build -A biberbuilds and the existing tests in the perl stack still passsandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)