stdenv: make symlinks that refer to the same output relative #74253

andir · 2019-11-26T08:49:00Z

Motivation for this change

While looking at the graph of all the outputs in my personal binary
cache it became obvious that we have a lot of self references within the
package set. That isn't an isssue by itself. However it increases the
size of the binary cache for every (reproducible) build of a package
that carries references to itself. You can no longer deduplicate the
outputs since they are all unique. One of the ways to get rid of (a few)
references is to rewrite all the symlinks that are currently used to be
relative symlinks. Two buildd of something that didn't really change but
carry each a self-reference can the be stored as the same NAR file again.

I quickly hacked together this change to see if that would yield and
success. My bash scripting skills are probably not great but so far it
seem to somewhat work.

Things done

Notify maintainers

cc @Ericson2314

andir · 2019-11-26T12:15:37Z

Are there valid use cases where this is not desirable? I was thinking of providing an escape hatch if there are any...

7c6f434c · 2019-11-26T12:34:44Z

Are there valid use cases where this is not desirable? I was thinking of providing an escape hatch if there are any...

I guess a weird enough upstream build system of a reverse depedency could copy something to its own installation directory, which was OK when added (back when it was a plain file), works with absolute symlinks but might break for relative ones with some flags? This sounds weird, but maybe something close is posible.

B4dM4n

The concept sounds nice, but I found two issues in the bash script.

Are there valid use cases where this is not desirable? I was thinking of providing an escape hatch if there are any...

I can definitely see issues with scripts that expect an absolute path from readlink and will be broken by this change, so being able to disable this hook would be nice.

B4dM4n · 2019-11-26T12:55:36Z

pkgs/build-support/setup-hooks/make-symlinks-relative.sh

+    local relativeTarget
+    while IFS= read -r -d $'\0' f; do
+        symlinkTarget=$(readlink "$f")
+        if [ "${symlinkTarget:0:${#prefix}}" != "$prefix" ]; then


This check has the potential to produce false negatives when two outputs share the same prefix:

/nix/store ├── 1111-out │ └── file -> /nix/store/1111-out2/1234 └── 1111-out2 └── 1234

In this case /nix/store/1111-out/file would be rewritten to file -> 1234.

Suggested change

if [ "${symlinkTarget:0:${#prefix}}" != "$prefix" ]; then

if [[ "$symlinkTarget"/ != "$prefix"/* ]]; then

Thanks! While that technically should never happen within /nix/store that might be a good safeguard.

I'll add that to the next version.

B4dM4n · 2019-11-26T13:05:14Z

pkgs/build-support/setup-hooks/make-symlinks-relative.sh

+        relativeTarget="${symlinkTarget:${#prefix}}"
+        echo "rewriting symlink $f to be relative to $prefix"
+
+        rm "$f"
+        ln -rs "$prefix/$relativeTarget" "$f"


~~The ln destination should be "$prefix/$f".~~
Never mind, f is already absolute. Updated code block below.

To simplify the process this block can be replaced with these ln flags:

echo "rewriting symlink $f to be relative to $prefix" ln -snrf "$symlinkTarget" "$f"

as ln -r produces the minimal possible relative link.

7c6f434c · 2019-11-26T16:39:08Z

If we are going to find all symlinks and resolve them, it would be nice to fail the build with a meaningful error message if the target (file or dir at the location a symlink points to) does not exist.

… and also to provide and ways way to opt out of this behaviour. I do have some uses for symlinking to nonexistent files; for example when something wants to write to some path that is in store (but can be redirected to a proper location via a symlink)

andir · 2019-11-26T18:42:45Z

If we are going to find all symlinks and resolve them, it would be nice to fail the build with a meaningful error message if the target (file or dir at the location a symlink points to) does not exist.
… and also to provide and ways way to opt out of this behaviour. I do have some uses for symlinking to nonexistent files; for example when something wants to write to some path that is in store (but can be redirected to a proper location via a symlink)

Please note that I am only checking for references into the same output. So in general those shouldn't be the problem. You can still create arbitrary symlinks to directories that do not exist as long as they do not point into the output. Would that work for you?

I feel like the enforcement of valid symlinks is yet another topic compared to what I wanted to accomplish here.

7c6f434c · 2019-11-26T19:15:19Z

Please note that I am only checking for references into the same output. So in general those shouldn't be the problem. You can still create arbitrary symlinks to directories that do not exist as long as they do not point into the output. Would that work for you?

Ah, right, same-output symlinks stay broken or valid after installation, and the situation where existence of a broken symlink to a nonexistent location is preferrable to plain absence of a directory entry sounds like a small component part of an unspeakable horror.

andir · 2019-11-27T14:31:34Z

I added some of the review comments. I removed the hard failure on dangling symlinks and just left the warning in there. Does that make sense?

I think failing on invalid symlinks is really a more complicated topic that we shouldn't try to solve right now. Warning is probably okay but failing is yet another different topic from this approach. We can add it to this hook in another PR once there is consensus about the desired behavior.

andir · 2019-11-27T20:01:53Z

@GrahamcOfBorg build bash

pkgs/build-support/setup-hooks/make-symlinks-relative.sh

FRidh · 2020-01-06T10:59:06Z

Can this go in?

While looking at the graph of all the outputs in my personal binary cache it became obvious that we have a lot of self references within the package set. That isn't an isuse by itself. However it increases the size of the binary cache for every (reproducible) build of a package that carries references to itself. You can no longer deduplicate the outputs since they are all unique. One of the ways to get rid of (a few) references is to rewrite all the symlinks that are currently used to be relative symlinks. Two build of something that didn't really change but carries a self-reference can the be store as the same NAR file again. I quickly hacked together this change to see if that would yield and success. My bash scripting skills are probably not great but so far it seem to somewhat work.

andir · 2020-01-06T11:08:02Z

@FRidh Looks good to me :-) If @Ericson2314 is now happy I see no reason why we can't merge this.

andir · 2020-01-15T13:27:43Z

Thanks @FRidh !

Ericson2314 · 2020-01-17T15:45:29Z

Woo! Sorry I didn't have the time to vet this further myself. Thanks @FRidh and @andir

Artturin · 2022-12-05T18:57:56Z

pkgs/build-support/setup-hooks/make-symlinks-relative.sh

@@ -0,0 +1,28 @@
+fixupOutputHooks+=(_makeSymlinksRelative)


wlroots has

wlroots> +++ find /nix/store/3a0xwszw8n5dzzhsgfnilvsqi4hk565s-wlroots-0.15.1-examples -type l -print0 wlroots> find: '/nix/store/3a0xwszw8n5dzzhsgfnilvsqi4hk565s-wlroots-0.15.1-examples': No such file or directory

because the examples output is created in postFixup while this hook runs in fixupPhase

if this is changed to postFixupHooks then we shouldn't get a error like that in many builds

andir requested review from Ericson2314 and matthewbauer as code owners November 26, 2019 08:49

ofborg bot added 6.topic: stdenv Standard environment 10.rebuild-darwin-stdenv 10.rebuild-linux-stdenv 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Nov 26, 2019

B4dM4n reviewed Nov 26, 2019

View reviewed changes

andir force-pushed the relative-links branch from 11a151e to c6c26e7 Compare November 27, 2019 14:28

7c6f434c approved these changes Nov 27, 2019

View reviewed changes

FRidh added this to Needs review in Staging Nov 30, 2019

FRidh added this to the 20.03 milestone Dec 10, 2019

Ericson2314 reviewed Dec 17, 2019

View reviewed changes

pkgs/build-support/setup-hooks/make-symlinks-relative.sh Outdated Show resolved Hide resolved

andir force-pushed the relative-links branch from c6c26e7 to 437b024 Compare January 6, 2020 11:07

FRidh requested a review from Ericson2314 January 6, 2020 12:06

FRidh merged commit cb007e6 into NixOS:staging Jan 15, 2020

Staging automation moved this from Needs review to Done Jan 15, 2020

andir deleted the relative-links branch January 29, 2020 00:08

bkchr mentioned this pull request Feb 3, 2020

browserpass: Fix usage as Firefox plugin #79190

Closed

10 tasks

andir mentioned this pull request Apr 26, 2020

Using fast symlinks instead of slow symlink on /nix/store NixOS/nix#1415

Closed

Artturin reviewed Dec 5, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stdenv: make symlinks that refer to the same output relative #74253

stdenv: make symlinks that refer to the same output relative #74253

andir commented Nov 26, 2019 •

edited

andir commented Nov 26, 2019

7c6f434c commented Nov 26, 2019 via email

B4dM4n left a comment

B4dM4n Nov 26, 2019 •

edited

andir Nov 26, 2019

B4dM4n Nov 26, 2019 •

edited

7c6f434c commented Nov 26, 2019 via email

andir commented Nov 26, 2019

7c6f434c commented Nov 26, 2019 via email

andir commented Nov 27, 2019

andir commented Nov 27, 2019

FRidh commented Jan 6, 2020

andir commented Jan 6, 2020

andir commented Jan 15, 2020

Ericson2314 commented Jan 17, 2020

Artturin Dec 5, 2022

Artturin Dec 5, 2022

	if [ "${symlinkTarget:0:${#prefix}}" != "$prefix" ]; then
	if [[ "$symlinkTarget"/ != "$prefix"/* ]]; then

stdenv: make symlinks that refer to the same output relative #74253

stdenv: make symlinks that refer to the same output relative #74253

Conversation

andir commented Nov 26, 2019 • edited

Motivation for this change

Things done

Notify maintainers

andir commented Nov 26, 2019

7c6f434c commented Nov 26, 2019 via email

B4dM4n left a comment

Choose a reason for hiding this comment

B4dM4n Nov 26, 2019 • edited

Choose a reason for hiding this comment

andir Nov 26, 2019

Choose a reason for hiding this comment

B4dM4n Nov 26, 2019 • edited

Choose a reason for hiding this comment

7c6f434c commented Nov 26, 2019 via email

andir commented Nov 26, 2019

7c6f434c commented Nov 26, 2019 via email

andir commented Nov 27, 2019

andir commented Nov 27, 2019

FRidh commented Jan 6, 2020

andir commented Jan 6, 2020

andir commented Jan 15, 2020

Ericson2314 commented Jan 17, 2020

Artturin Dec 5, 2022

Choose a reason for hiding this comment

Artturin Dec 5, 2022

Choose a reason for hiding this comment

andir commented Nov 26, 2019 •

edited

B4dM4n Nov 26, 2019 •

edited

B4dM4n Nov 26, 2019 •

edited