search

latex3 / l3build

A testing and building system for LaTeX
LaTeX Project Public License v1.3c
84 stars 14 forks source link

dep_install→unpack_deps #373

Closed jlaurens closed 6 days ago

jlaurens commented 2 weeks ago

Having a function that claims to install something whereas it use the unpack target instead of the existing install target is highly misleading

josephwright commented 2 weeks ago

Not sure I see this as 'highly-misleading' (we are installing in the local build tree), but I also am not tied to the current name. Here, I think you could just make local - this is likely one we didn't look at when things were tightened up a bit.

jlaurens commented 2 weeks ago

Changing the name helps in understanding the code

This function cannot be made local because it is used in different modules. It can be hidden from the global level but l3build code seems not yet completely ready for that.

FrankMittelbach commented 1 week ago

Am 30.06.24 um 12:21 schrieb LAURENS Jérôme:

Changing the name helps in understanding the code

  • "install" = install globally
  • "unpack" = install locally

maybe to you, but actually not to me.

With the current names you have to understand that "unpack" refers to "unpacking within l3build" (and I hope that is documented) and with your names you have to understand that "install globally" really means "install into your local texmf tree" which is in my eyes counterintuitive. Of course, that could be documented to do just that, but it doesn't make the name better, imho.

And there are cases when one wants to run just "unpack", e.g., when to check that the unpacking generates the right files.

u-fischer commented 1 week ago

I do not care much about the name of local, internal functions. But I do find it rather dangerous to change names of functions which are (for whatever reason) visible on the global level for quite some time as you can't know if not someone uses them. As an example: the PR contains a todo: -- TODO: Hide bundleunpack from the global level But the LaTeX base build.lua actually redefines the function and uses a custom bundleunpack, and so any change here must be checked against existing build scripts.

jlaurens commented 1 week ago

@u-fischer Absolutely. This TODO should have been removed.

u-fischer commented 1 week ago

@jlaurens

Absolutely.

Hm. But you seem not have grapped the full implication of my remark. If it is unsafe to change the global bundleunpack then it is also unsafe to change the global dep_install. For example a short search found this: latex3/latex3/l3kernel/build.lua: dep_install(checkdeps).

jlaurens commented 1 week ago

@u-fischer Lines 175+ of l3build-aux.lua

A supplemental question is whether to document if officially or not.