P6-LibraryMake icon indicating copy to clipboard operation
P6-LibraryMake copied to clipboard

percent encoding causes installs to fail

Open andinus opened this issue 1 year ago • 6 comments

Context: https://github.com/ugexe/zef/issues/457#issuecomment-1586021352, https://github.com/ugexe/zef/issues/457

Summary: LibraryMake percent encoding causes installs to fail on systems that use llvm-lld.

I assume you are on an architecture where the linker is LLVM ld,
otherwise known as ld-lld in OpenBSD (some older architectures
still use ld-bfd).

In llvm/lib/Support/Path.cpp, there is code that acts just like you describe:

void createUniquePath(const Twine &Model, SmallVectorImpl<char> &ResultPath,
                      bool MakeAbsolute) {
...
  // Replace '%' with random chars.
  for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
    if (ModelStorage[i] == '%')
      ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
  }


It apppears in the LLVM universe if you try to create a file with % in the
name, it has a different interpretation of what that % means, different than
what you want it to mean.

https://docs.hdoc.io/hdoc/llvm-project/f1FB0DB2307A8013C.html

Other than that, I can find no documentation.

When I try to install any module that uses LibraryMake (Digest::SHA1::Native) install fails because llvm-lld replaces the percent with a random character.

andinus avatar Jun 13 '23 06:06 andinus

So if I get this correctly, this fails when a specific module is installed in a specific architecture, OpenBSD, right? Is there anything specific in that architecture that makes this fail?

JJ avatar Jun 13 '23 09:06 JJ

I believe it is triggered when any module that relies on LibraryMake is installed. Geo::Hash installation also fails.

From what I understand, OpenBSD uses llvm-lld as linker and the linker replaces '%' to a random character which causes the linker to think that the path doesn't exist. It is supposed to be a feature to get unique paths: https://docs.hdoc.io/hdoc/llvm-project/f1FB0DB2307A8013C.html

Create a potentially unique file name but does not create it. Generates a unique path suitable for a temporary file but does not open or create the file. The name is based on \a Model with '%' replaced by a random char in [0-9a-f]. If \a MakeAbsolute is true then the system's temp directory is prepended first. If \a MakeAbsolute is false the current directory will be used instead. This function does not check if the file exists. If you want to be sure that the file does not yet exist, you should use use enough '%' characters in your model to ensure this. Each '%' gives 4-bits of entropy so you can use 32 of them to get 128 bits of entropy. Example: clang-%%-%%-%%-%%-%%.s => clang-a0-b1-c2-d3-e4.s

andinus avatar Jun 15 '23 18:06 andinus

Let me see if I can wrap my head around this. From what I understand, it's a test error, because modules can be installed correctly, is that correct? Second, I am not totally sure where that percent encoding is. It's used in the Makefile.in to substitute variables and so on, but theoretically, there should be none of that in the output Makefile. Mine looks like this:

all: ./test

./test: test.o
	gcc -fPIC -o ./test test.o

test.o: test.c
	gcc -c -std=gnu99 -Wextra -Wall -Wno-unused-parameter -Wno-unused-function -Wno-missing-braces -Werror=pointer-arith -Werror=vla -O3 -DNDEBUG  -D_REENTRANT -D_FILE_OFFSET_BITS=64 -fPIC -DMVM_HEAPSNAPSHOT_FORMAT=2 -D_GNU_SOURCE -o test.o test.c

So if I had to make a hunch, I would say that some variable is not substituted properly from the Makefile.in? And that happens only when you use llvm-ld in BSD for some reason?

JJ avatar Oct 11 '23 12:10 JJ

I'm trying to follow the different issues, and this is the only one open, so I guess I'll comment here. TL;DR: I think the main issue is how zef names temporary directories. This library does not make any attempt to create directories and/or encode their names in any way.

Rationale for this

In the original issue, the error goes when you clone and compile. When you do so, you are using this library and it's not issuing any error.

On the other hand, different pastes here indicate that zef is createing directories that use uri-encoding for names, such as this one:

 /tmp/.zef/1682613110.9516/Crypt%3A%3ABcrypt%3Aver%3C1.3.2%3E%3Aauth%3Cgithub%3Askinkade%3E.tar.gz/p6-Crypt-Bcrypt-master/Build.pm line 13), 

Those directories are created by zef, not LibraryMake. The fact that they fail when using LibraryMake is that this actually launches a compilation procedure, which would use said ld-lld, which would then fail due to the path name.

If this hunch is correct, any distribution with no colons would install correctly with zef.

Conclusion

Unless I'm very wrong, this would be a possible enhancement for zef, using a different encoding for colons in distro names. I'd be happy to admit otherwise, though, or accept any help or PRs in this distro should it not be that issue.

JJ avatar Oct 11 '23 12:10 JJ

I see, I opened this issue in zef repo initially, an idea from there:

LibraryMake should probably generate commands using a relative path instead of absolute paths instead. https://github.com/ugexe/zef/issues/457#issuecomment-1587168543

Does that seem like an option?

Here is the Makefile:

andinus@cadmium /t/.z/1/D/raku-digest-sha1-native-master [SIGINT]> cat Makefile
.PHONY: clean test

all: /tmp/.zef/1697031419.72860/Digest%3A%3ASHA1%3A%3ANative%3Aver%3C0.06%3E%3Aauth%3Cgithub%3Abduggan%3E.tar.gz/raku-digest-sha1-native-master/resources/libraries/libsha1.so

clean:
        -rm /tmp/.zef/1697031419.72860/Digest%3A%3ASHA1%3A%3ANative%3Aver%3C0.06%3E%3Aauth%3Cgithub%3Abduggan%3E.tar.gz/raku-digest-sha1-native-master/resources/libraries/libsha1.so /tmp/.zef/1697031419.72860/Digest%3A%3ASHA1%3A%3ANative%3Aver%3C0.06%3E%3Aauth%3Cgithub%3Abduggan%3E.tar.gz/raku-digest-sha1-native-master/*.o

/tmp/.zef/1697031419.72860/Digest%3A%3ASHA1%3A%3ANative%3Aver%3C0.06%3E%3Aauth%3Cgithub%3Abduggan%3E.tar.gz/raku-digest-sha1-native-master/resources/libraries/libsha1.so: sha1.o
        clang -shared -fPIC  -O3 -DNDEBUG -Wl,-rpath,"//usr/local/rakudo/lib" -o /tmp/.zef/1697031419.72860/Digest%3A%3ASHA1%3A%3ANative%3Aver%3C0.06%3E%3Aauth%3Cgithub%3Abduggan%3E.tar.gz/raku-digest-sha1-native-master/resources/libraries/libsha1.so sha1.o

sha1.o: src/sha1.c
        clang -c -fPIC -fno-omit-frame-pointer -fno-optimize-sibling-calls -Werror=vla -fno-ret-protector -O3 -DNDEBUG -Wno-logical-op-parentheses -D_REENTRANT -D_FILE_OFFSET_BITS=64 -fPIC -gdwarf-4  -DMVM_HEAPSNAPSHOT_FORMAT=3 -D_GNU_SOURCE -o  sha1.o src/sha1.c

test: all
        prove -e "raku -Ilib" t

Replacing that with the Makefile below & running make seems to work:

andinus@cadmium /t/.z/1/D/raku-digest-sha1-native-master> cat Makefile
.PHONY: clean test

all: ./resources/libraries/libsha1.so

clean:
        -rm ./resources/libraries/libsha1.so ./*.o

./resources/libraries/libsha1.so: sha1.o
        clang -shared -fPIC  -O3 -DNDEBUG -Wl,-rpath,"//usr/local/rakudo/lib" -o ./resources/libraries/libsha1.so sha1.o

sha1.o: src/sha1.c
        clang -c -fPIC -fno-omit-frame-pointer -fno-optimize-sibling-calls -Werror=vla -fno-ret-protector -O3 -DNDEBUG -Wno-logical-op-parentheses -D_REENTRANT -D_FILE_OFFSET_BITS=64 -fPIC -gdwarf-4  -DMVM_HEAPSNAPSHOT_FORMAT=3 -D_GNU_SOURCE -o  sha1.o src/sha1.c

test: all
        prove -e "raku -Ilib" t

andinus avatar Oct 11 '23 13:10 andinus

That might be doable. Thanks for the suggestion!

JJ avatar Oct 11 '23 15:10 JJ