[PATCH] union: Resolve collisions by stable-sort'ing them.

(warn-about-collision): Renamed to default-collision-resolver.

---

this should work, but i cannot test it, because srfi-43 seems not to be

available on the build side:

unpacking bootstrap Guile to '/home/alendvai/workspace/guix/guix/test-tmp/store/qky0jf68rr7pnsvmhj0ay42rzh4qk6r9-guile-bootstrap-2.0'...

[...] output without sfri-43.go

and then unsurprisingly: "no code for module (srfi srfi-43)"

is tis only a peculiarity of the test environment?

can you please advise how to proceed?

guix/build/union.scm | 26 ++++++++++++++++++++------

guix/gexp.scm | 2 +-

tests/union.scm | 9 +++++++++

3 files changed, 30 insertions(+), 7 deletions(-)

Attila Lendvai schreef op di 28-09-2021 om 23:40 [+0200]:

SRFI-43 is in Guile since Guile 2.0.10, according to Guile's NEWS.

The bootstrap guile is older:

$(guix build -e '(@@ (gnu packages bootstrap) %bootstrap-guile)')/bin/guile --version

guile (GNU Guile) 2.0.9

[...]

Greetings,

Maxime.

thank you for the analysis!

is it easy and desirable to upgrade it to 2.0.10 or newer?

shall i try to do it, or advocate for it if it's not trivial, by

e.g. opening an issue?

- attila

PGP: 5D5F 45C7 DFCD 0A39

Hi,

Am Dienstag, den 28.09.2021, 23:40 +0200 schrieb Attila Lendvai:

Instead of stable-sort!-ing the indices of a vector, what about stable-

sort!-ing (map strip-store-file-name original-files) in more or less

one go?

I don't think the default collision resolver ought to sort the files.

The rationale behind ignoring certain collisions, e.g. icon caches

relies on the fact that Guix will use the correct files because they

are put first in the manifest. The hooks themselves have no special

names that could put them "always first" and profiles are themselves

union-built.

I do however support the addition of sorting methods as collision

resolvers in general and would welcome a way of doing so for profiles

pre-hook.

Regards,

Liliana

Attila Lendvai schreef op wo 29-09-2021 om 16:03 [+0000]:

It's possible by modifying 'bootstrap-guile-url-path' and

'bootstrap-guile-hash'. Apparently, some architectures already

use a newer guile. E.g., aarch64 has 2.0.14. However, this

probably would entail a world-rebuild I think, so this probably

needs to be done on core-updates.

Why limit to guile 2.0.10, why not go for guile 3.0.7 instead?

I don't kow how one would go about updating the bootstrap binaries though.

Anyway, there have been quite a few bug fixes and new features since

2.0.9, and updating to guile 3.0.? would allow dropping some compatibility

code in various guix/build/, so I wouldn't be opposed to such a change.

Greetings,

Maxime

the hash also needs to be dropped from the path for sorting to be

useful, but the return value must be the full path, hence the

complexity with sorting the indices, pointing both to the full paths

and the cut parts.

please note that i almost completely lack the knowledge of the

relevant internals.

with that in mind, do i read you right? this should add a new exported

resolver funtion, and leave the default one as it was?

and use the new resolver somewhere (where?) that will affect only the

union of profiles, i.e. user visible effecs when installing software?

(because DIRECTORY-UNION is called in other contexts also?)

thanks for the insights! i'd appreciate a bit more higher level

guidance, and then i'll resend the patch accordingly.

- attila

PGP: 5D5F 45C7 DFCD 0A39

Attila Lendvai schreef op do 30-09-2021 om 08:10 [+0000]:

You can replace the 'less' argument of 'stable-sort'.

Example sorting by the second character of a string:

(sort '("za" "yb" "xc") (lambda (x y)

(char>? (string-ref x 1)

(string-ref y 1)))))

IIUC, you would need to replace char>? by string> and string-ref

by strip-store-file-name.

Greetings,

Maxime.

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

Agreed. I’d advice using this strategy rather than resorting to

SRFI-43; it should have the desired effect.

BTW, because TeX Live packages rely on ‘union-build’, this patch

triggers a lot of rebuilds, but we can try to squeeze it in the upcoming

‘core-updates-frozen’ rebuild.

Thanks,

Ludo’.

i don't know about the expected size of the collision list here, but

that would cons much more, because that would cons up two substrings

at each comparison, i.e. O(n^2) vs O(n) at least in GC load, probably

in time also.

i think it's not worth it, but let me know, and then i can simplify

the code somewhat at the cost of more consing.

sorting lists is probably also much slower than sorting vectors, but

there may be some tricks i don't know about.

random note: civodul said on IRC that there's a certain reluctancy to

update the opaque binary blobs, and the bootstrap guile will be

replaced by mes anyway.

so, if we want to have this merged, then we will need to give up on

testing it until mes becomes the bootstrap scheme (and assuming it

will have srfi vectors).

in the lights of the above, i think i'll stop pursuing this patch, but

i'd be happy to implement something if you can give me highlevel

guidance.

with the above pointed out, are you still happy with the list sorting?

i'd love to have idris packaged so that it has bin/idris symlinked to

bin/idris-1.2.3, and installing multiple versions of it would pick the

newest one for bin/idris.

--

• attila lendvai

• PGP: 963F 5D5F 45C7 DFCD 0A39

--

“Have the courage to take your own thoughts seriously, for they will shape you.”

— Albert Einstein (1879–1955)

Attila Lendvai schreef op do 30-09-2021 om 14:12 [+0000]:

I don't see any consing here, or any increase in complexity?

I was suggesting replacing the 'less' procedure with a procedure

like (lambda (x y) (char>? (string-ref x 1) (string-ref y 1)))

instead of doing the sorting in two steps.

I wasn't suggesting using lists. The '("za" "yb" "xc") was only

for demonstration, a vector should work as well.

Greetings,

Maxime.

Am Donnerstag, den 30.09.2021, 10:42 +0200 schrieb Maxime Devos:

You could also store a mapping of long file name to stripped file name

in a hash table for fast lookup, so as to not compute (strip-store-

file-name) over and over. That way you can stable-sort! the long

names, but don't forget to list-copy them at least for functional

purity.

Greetings,

Liliana

Hi Attila

Am Donnerstag, den 30.09.2021, 08:10 +0000 schrieb Attila Lendvai:

This thing has received some replies already to which I already dropped

a comment, so I'll try not to repeat myself here. If something is

still unclear, go down the sub-thread started by Maxime.

Yes, I think that is safer than actually overriding the default for now

at least.

I must admit, I'm at a loss of knowledge myself here. Perhaps someone

else can jive in and tell you which union-build is done without hooks,

but I fear there might also be none. For now, simply having the

resolver is in my opinion enough if we don't find the right location as

well. We can default it later with "first-come-first-serve" exceptions

where needed.

Cheers

exported define-constant util.

---

guix/build/graft.scm | 5 +----

guix/build/utils.scm | 12 ++++++++++++

2 files changed, 13 insertions(+), 4 deletions(-)

(store-path-prefix-length): Factor out the calculation of the total store

prefix length.

---

guix/build/graft.scm | 2 +-

guix/build/utils.scm | 11 +++++++++--

2 files changed, 10 insertions(+), 3 deletions(-)

This prepares the stage for new collision resolvers, but no semantic changes.

(resolve-collision-and-maybe-warn): New function.

(default-collision-resolver): New function.

---

guix/build/union.scm | 16 ++++++++++------

guix/gexp.scm | 2 +-

2 files changed, 11 insertions(+), 7 deletions(-)

It is currently not used anywhere, only exported. The tests are boken, because

guile is too old in the test environment, at least on 'x86_64-linux' (guile

2.0.9 doesn't have srfi-43, aka vectors). Probably it's also broken because

testing errors with `no code for module (guix build utils)`.

---

I think the previous 3 patches in this patchset are worthy of inclusion,

but this one is more of a good idea than a worked out change, to be picked

up later, if at all.

The primary issue is that the test framework uses a guile that is too old,

but it's also not used anywhere. It would be nice if this was used for

resolving conflicts for profiles, i.e. for the user's bin/ directory.

guix/build/union.scm | 12 ++++++++++++

guix/build/utils.scm | 27 +++++++++++++++++++++++++++

tests/union.scm | 9 +++++++++

3 files changed, 48 insertions(+)

sorry for being slow with understanding the suggested solution. i have automatically dismissed everything with leaky abstractions, and that made me blind to it.

i didn't realize that i could either introduce constants for the hash length, and the store path, or straight out export a special comparator function to be used.

either way, i have organized the patches so that the first 3 are useful, and the 4th one is more of a demo/inspiration for prosperity.

apply 1-3 as you see fit, and ignore or finish the 4th.

- attila

PGP: 5D5F 45C7 DFCD 0A39

Ho Attila,

sorry for the long wait.

Am Sonntag, dem 03.10.2021 um 12:59 +0000 schrieb Attila Lendvai:

I've applied 1-3 with some changes to core-updates (particularly

reducing the number of indirections in the third patch). I verified

that gcc-toolchain builds and the collision is still resolved as-is.

If you still wish to stable-sort things, go ahead.

Cheers