[PATCH] Don't fix git executable location during Elixir build

The removed code below affected two locations where Elixir uses Git:

is added to the build information that Elixir returns with `System.build_info()`. The code

uses PATH + `git` so the Git version from the inputs is used which is just fine.

is not the normal case, and in that sense Git is an optional dependency of Elixir - most

projects will work just fine without Git being present. Here, too, PATH + `git` is used

so if there is a need on a user's system to use Git-based dependencies, the user can just

add the package to the profile and things will just work.

I feel that leaving this alone is cleaner than trying to fix the dependency (if we want to

keep it, it needs to be in propagated-inputs in order to keep Git around; I stumbled upon

this building a project with Git dependencies after doing a `guix gc`. However, given that

using Git dependencies is the exception and I think that minimizing dependencies is nicer

I opted to just remove the substition completely).

---

gnu/packages/elixir.scm | 5 -----

1 file changed, 5 deletions(-)

--

2.34.0

Cees de Groot schreef op di 08-03-2022 om 20:52 [+0000]:

Instead of propagation, I recommend substitution, like done currently.

Propagation is not necessary to ‘keep git around’ (assuming that the

store reference is not obfuscated by gzipping or the like).

If it's used at build-time instead of run-time, then git should be

moved to native-inputs, for cross-compilation reasons.

Greetings,

Maxime

Cees de Groot schreef op di 08-03-2022 om 20:52 [+0000]:

That cannot ever have worked in the first place, since Guix does not

save the '.git' directory when downloading elixir's source code.

Greetings,

Maxime.

Cees de Groot schreef op di 08-03-2022 om 20:52 [+0000]:

Due to cross-compilation reasons, this is not fine, it should use the

git from native-inputs instead.

Greetings,

Maxime.

Cees de Groot schreef op di 08-03-2022 om 20:52 [+0000]:

If it's optional and not used most of the time, removing

it seems fine to me, though this seems useful information for the

description. Also, it would be nice if 'elixir' were patched

to emit a nice message when 'git' is not found. Suggestion:

This project required 'git', but it is not in $PATH. It can be

installed into the user profile with `guix install git`.

You might want to contact whoever added the substitution originally,

to see what the original reasons were.

Greetings,

Maxime.

Cees de Groot schreef op wo 09-03-2022 om 02:39 [+0000]:

How can being completely broken be correct and fine?

Greetings,

Maxime.

From the docs:

":revision - short Git revision hash. If Git was not available at building time, it is set to """ (https://hexdocs.pm/elixir/System.html#build_info/0)

So if git is not available - either because the executable is not there or because the build checkout is not from Git, the revision hash is set to the empty string, which is in accordance with the docs and what the Guix build has been doing all along (because, as you said, at build time, .git isn't there).

So for build time, cross-compiling or not, we don't need Git.

------- Original Message -------

On Wednesday, March 9th, 2022 at 02:31, Maxime Devos <maximedevos@telenet.be> wrote:

I think changing the behaviour of a programming language just to be helpful to developers is something that one may consider for upstream, but certainly not in the context of a packaging system like Guix - that just will breed confusion.

As it is, the error message thrown by Elixir when Git is not there is very easy to interpret for developers (and _only_ at development time can this happen!), no need to go overboard here. Unless, of course, you think it is useful to submit an upstream patch, feel free to do so.

------- Original Message -------

On Tuesday, March 8th, 2022 at 17:22, Maxime Devos <maximedevos@telenet.be> wrote:

Well.... the whole reason that I stumbled upon this was that the compile-time substitution referred to a Git version that got GC'd. And I think that that is the different between regular inputs and propagated inputs, whether inputs become eligible for GC at run-time or not. If I'm misunderstanding something there, something else caused that version of Git to get GC'd and I'd be more than happy to be corrected in my assumptions.

On 9 March 2022 17:35:25 UTC, Cees de Groot <cg@evrl.com> wrote:

This simply Should Not happen. Guix's GC model (inherited from Nix) is brutally simple: if the raw string /gnu/store/xxx occurs anywhere within a protected /gnu/store/yyy (e.g. in a binary), then xxx is also protected, recursively.

Assuming those assumptions hold here it's a 'mystery' why git got GC'd. From experience and for our collective peace of mind I'd say it's more likely that they didn't hold somehow, than that there's a serious GC bug :-)

So, no. Completely unrelated. The GC has no concept of time at all. This isn't Gentoo.

Propagated inputs are 'oh, here's this extra thing I was unable to properly patch, please add it to the profile and treat it as a GC reference anyway'.

This is ugly and regular, non-propagated inputs are always preferred. Some software just makes them too painful.

Cees,

Kind regards,

T G-R

Sent on the go. Excuse or enjoy my brevity.

Cees de Groot schreef op wo 09-03-2022 om 17:31 [+0000]:

Developers are users too and users can be developers. Furthermore,

Elixir is an implementation of a programming language, so aren't all

users of Elixir automatically developers?

I'm not sure how exactly Elixir integrates with git but I assume the

integration can be very useful.

Also, this does not change the behaviour of the language, it only

adjusts a detail of the implementation: the error message for

indicating that git is missing is adjusted.

How could better error messages breed confusion?

Also, this seems a change that would absolutely _not_ be considered

upstream. Why would upstream have distro-specific error messages? If

the new error message is Guix-specific, wouldn't Guix be the logical

place to have it?

Greetings,

Maxime.

On Wednesday, March 9th, 2022 at 12:59, Tobias Geerinckx-Rice <me@tobias.gr> wrote:

So how does Guix figure out what to keep and what not? I mean, in this case the patched string will land in a compiled .BEAM file which very well may have the string obfuscated (I need to check how the VM stores strings in binaries).

Cees,

Yeah, this happens sometimes and will hide these references from what is a simple and therefore very fast string search.

It's why we install uncompressed .jar files and disable an obscure (and arguably unnoticable) GCC optimisation that hacks strings into incontiguous chunks. If BEAM somehow obfuscates store references we'll have to find a similar work-around to stop it. Let us know what you find.

Kind regards,

T G-R

Sent on the go. Excuse or enjoy my brevity.

On Wednesday, March 9th, 2022 at 16:49, Tobias Geerinckx-Rice <me@tobias.gr> wrote:

Yup. BEAM files have literals stored in a single Deflated blob (https://beam-wisdoms.clau.se/en/latest/indepth-beam-file.html).So the scanning won't work.

Which means that either my patches should be merged or Git should be propagated. I'm not a super big fan of propagating optional dependencies, which is why I submitted the patch like this.

Hi,

Cees de Groot <cg@evrl.com> skribis:

If we know what set of store references go in those blogs, we could

create a text file or something in #$output whose sole purpose is to

keep a reference to those store items.

For example, if we know those BEAM files contain a reference to Git, we

can add a phase like:

(lambda* (#:key inputs outputs #:allow-other-keys)

(symlink (search-input-file inputs "git")

(string-append (assoc-ref output "out")

"/libexec/elixir/.git")))

That way the GC will keep Git around.

Thoughts?

Ludo’.

Ludovic Courtès schreef op vr 11-03-2022 om 22:51 [+0100]:

Doesn't that need to be "bin/git" instead of "git"?

That is not sufficient if git acquires a graft.

Can elixir be patched to not compress the literals table (*)?

Alternatively, does deflate have some kind of "compression

level=0" setting -- perhaps we could do the same trick as

done for JARs?

(*) Preferably submitted upstream in some form, maybe with a

configuration flag?

Greetings,

Maxime.

Can’t we just run the patch? Then everything works just fine. I’m a full time Elixir developer and I can guarantee you that nothing will break :)

Sent from ProtonMail for iOS

On Fri, Mar 11, 2022 at 16:57, Maxime Devos <maximedevos@telenet.be> wrote:

Cees de Groot schreef op vr 11-03-2022 om 22:01 [+0000]:

FWIW, I'm not convinced of the value of removing 'git', as I don't

expect a significant decrease in closure size and it makes Elixir a

little less usable (only a tiny bit because git can be installed

manually, but still a bit).

The only remaining reasons for removing it, appear to be avoiding

having to tackle the reference obfuscating issue -- sooner or later

(*), I expect there will eventually be an elixir-build-system and a few

elixir packages. These elixir packages might need a few 'substitute*'-

style hardcoding. So eventually, I expect the issue will reappear

again and frequently at that, so we might as well try to fix it now.

(*) Especially since there's an Elixir developer in Guix!

Greetings,

Maxime.

Well, feel free to do the patching work. Realize that you are going to do work for a) a very small group of developers (Elixir is a small language and Guix isn’t hardly a widely spread system to begin with), b) a subset of those that need git dependencies, and c) a subset of _those_ that need these dependencies on a project source tree that itself was not checked out with git (otherwise the git executable will already be in the environment and everything will work).

Please let me know when you found that unicorn developer :)

Sent from ProtonMail for iOS

On Fri, Mar 11, 2022 at 17:11, Maxime Devos <maximedevos@telenet.be> wrote:

Cees de Groot schreef op vr 11-03-2022 om 22:36 [+0000]:

This is not only for the subset that uses git dependencies. It is for

anyone writing and using Elixir packages, see my paragraph about

reference baking (in general, not git-specific):

More specifically, you could look at how, say, 'info-reader' uses

'substitute*', 'search-input-file', 'gunzip' and 'gzip'

Greetings,

Maxime.

If Guix ever gets elixir packages, that can be tackled.

I’m bowing out of this discussion. If my patch gets accepted then all good, if not, I’ll move it to my private channel and keep being happy there.

Sent from ProtonMail for iOS

On Fri, Mar 11, 2022 at 17:45, Maxime Devos <maximedevos@telenet.be> wrote:

Cees de Groot schreef op vr 11-03-2022 om 22:36 [+0000]:

I'm just here reviewing things, looking for possible better

alternatives, etc.

a) You're an Elixir developer, using Guix, and doing packaging work for

Elixir in Guix. Also, how is the popularity of Guix relevant on

guix-patches@, and how is the popularity of Elixir relevant when

this whole patch is about improving the Elixir package?

b) does not seem relevant to me, see other e-mails (about removing a

feature not worth the decrease in closure size IMO, and about

non-git references)

c) It's a bit niche; it would be a very nice thing of Guix to support

that niche.

Also, being checked out with git does not imply that git is

in the environment, it's possible to first set up a git repository

and later enter a pure environment for development (guix shell --

pure elixir elixir-foo elixir-bar). I often do this (not with

elixir, but with some other packages).

TBC, what do you mean with ‘unicorn developer’ here?

a) someone implementing this

b) someone in the subset

If (a), I was kind of hinting that you seem to be in a perfect position

to do so (you're an Elixir developer (*) in Guix and you seem to have

some knowledge of the serialisation formats).

(*) I'm assuming "Elixir developer" = "a developer of the Elixir

implementation" here.

Ok, but I'm a bit confused on why you don't seem interested in fixing

the reference issue and why you write ‘ever’ instead of ‘when’ because

you're both an Elixir and Guix developer. (You don't have to answer,

not answering is fine.)

Greetings,

Maxime.

On Friday, March 11th, 2022 at 18:16, Maxime Devos <maximedevos@telenet.be> wrote:

I am interested in fixing the bug in the current package. And, I'm quite honest there, I'm in itch-scratching mode so I'm not interested in fixing potential, future, maybe issues :). My patch works, and yes, I'm more than happy to improve and iterate on it _if_ - and to me, that's a very big if - more issues appear.

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

Argh, good point.

Now, I agree with Cees that we need to be pragmatic here and opt for a

“good enough” solution.

So I went ahead and applied the initial patch that Cees posted.

However, that ‘replace-paths’ phase also touches references to /bin/sh;

are they also a problem after GC?

Thanks,

Ludo’.

These references only touch shell scripts so I think they should be just fine. I’ll give it a look when I’m back at work after the weekend to be sure though.

Sent from ProtonMail for iOS

On Sun, Mar 13, 2022 at 18:14, Ludovic Courtès <ludo@gnu.org> wrote: