[Octave-bug-tracker] [bug #45369] pkg build leads to duplication of PKG_ADD and PKG_DEL files

[Carnë Draug]

Follow-up Comment #15, bug #45369 (project octave):

To ease discussion of this, I think there's an important point to be
addressed:

== Important point ==

Despite the name, PKG_ADD and PKG_DEL scripts are not scripts that are meant
to be run when a package is loaded or unloaded.  They are scripts that are run
when a specific directory is added or removed from the path. It is not pkg
that handles execution of those files.  Note that Octave itself uses those
files. Simply starting octave will execute a bunch of them:


/usr/local/lib/octave/4.0.0/oct/x86_64-unknown-linux-gnu/PKG_ADD
/usr/local/share/octave/4.0.0/m/deprecated/PKG_ADD
/usr/local/share/octave/4.0.0/m/image/PKG_ADD
/usr/local/share/octave/4.0.0/m/optimization/PKG_ADD



Olaf Till on comment #3
> I think disallowing PKG_ at package root is not the right solution.
>
> Suggestion: PKG_ at package root always only into one directory, either
arch-dependent or -independent. PKG_ directives from source-files or installed
files also each directive only in one of arch-dep. or -indep. directory.
Doesn't actually matter into which. And then: All PKG_ADD directives of a
package should be called only after having added both arch-dep. and -indep.
directory to the path, and correspondingly for PKG_DEL. 

I don't understand why disallowing PKG_ at the root of the package is bad. I
can't see any case where requiring it to be on a specific directory is worse.
You suggest to support this by making sure to copy the files only to one of
the directories. But we can drop this whole ambiguity by require the package
developer to place the file in the directory that makes the most sense.


== Julien Bect suggestion ==

I'm not familiar with the binary distributions side of it, but the proposal
for the source releases is in line with my idea.

> I just realized that when you said "I think the best fix is to no longer
accept PKG_ADD and PKG_DEL files at the root of the package" (see #1), perhaps
did you actually mean "not at the root of the package, ./inst and ./src
instead" ? In which case this is very close to my proposition.

Yes. This is exactly what I proposed. Point is, I can't see a user-case where
it makes sense to have the same PKG_ADD and PKG_DEL scripts in both
arch-dependent and independent directories.  This means that the only reason
to support the files at the root of the package, is when there is only one of
such directories. But if that's the case, why not require the files to be
place in the correct directory? If package developers place the PKG_ADD on the
directory that is meant to be execute from, none of this problems would
happen.

> 1) No longer accept PKG_ADD/PKG_DEL files at the root of the package
tarball.

Agreed.

> 2) Accept PKG_ADD/PKG_DEL files in ./inst, or in any subdirectory of ./inst.
Those files are copied at the corresponding location in the arch-independent
directory.

Not sure if I understood all the implications of it correctly, but I kind of
agree.  Agree with accepting files in ./inst and its subdirectories (seems
like pkg() removes non .m files, that should not happen).  Do not agree on
having PKG_ADD on subdirectories executed when loading the package. It should
be the PKG_ADD at the root of inst/ that is responsible for calling the others
(most likely by adding the subdirectories to the path).

> 3) Also, accept PKG_ADD/PKG_DEL files in ./src (for source packages) or in
./inst/ARCH-NAME (for binary packages). Those files are copied in the
arch-dependent directory.

I'm not that familiar with this binary distributions but sounds reasonable.

> 4) Keep accepting PKG_ADD/PKG_DEL directives as well.

Yes please.

> 5) When building a binary package, copy PKG_ADD/PKG_DEL directives into
PKG_ADD/PKG_DEL files (this is what we already do) but leave the resulting
files where they are (in particular, do not move them at the root of the
tarball)
>
> 6) When installing a binary package, do not copy PKG_ADD/PKG_DEL directives
into PKG_ADD/PKG_DEL files a second time (once is enough !)

This requires pkg() knowing if it's installing a binary package or not and
adds extra logic dependent on the type of installation. Ideally, we would keep
differences to the minimum. Maybe the opposite is better, to not have binary
distributions handling PKG_ADD at all, and leave that job for the actual pkg
install. Problem is, a binary install no longer has the source. There is
another issue related it. demos and tests from functions in src/ are lost
during install because the source files are removed after building. If they
are kept around for this purpose, they would still be available in binary
distributions. This would also make them available for the binary installs.

== Removal of PKG_ and PKG_DEL directives by Philip ==

This is not going to happen. Even core makes use of it. On the io and stk
package, you may be using PKG_ADD as a script that gets run when loading a
package. But that's not its main use and original purpose.  The original idea
was code that runs when a specific function is added to the path.  Because of
this, it makes a lot of sense to have it on the respective file and not on a
general PKG_ADD file.

> * PKG_ADD / PKG_DEL are easily tracked. What if some function containing
PKG_ADD/PKG_DEL is removed from a package, or if functions accidentally
containing PKG_* commented code are moved between packages?

In that case use a PKG_ADD script. pkg() will append any PKG_ADD directives to
that script. But code that is related to a specific function should go into
that function. Your argument ignores the opposite side (removing a function
from a package and forgetting to remove its code from PKG_ADD script).

> * It is simpler and more clean. I do not like to have special code in a
function file that needs to be there for purposes the function itself isn't
meant for. (Carnë suggested to have PKG_ADD/PKG_DEL in one particular .m-file
in the io package)

I guess that's a opinion.  On my view, the io package is about IO of different
formats, not about xls support. In that view, it makes sense to check for xls
only when such function gets added to the path.  But I can see the other side
too. What I don't see is how it's preferable to have a workaround that removes
PKG_ADD from the oct-files directory (mostly because I don't trust pkg() and
post_install() enough to do it right in all situations. The risk of messing up
and removing user files is too large). And what will you do once you actually
need a PKG_ADD on the oct files directory?

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?45369>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/