Re: [pkg.m] Load order of dependencies

[Juan Pablo Carbajal]

Here is one nasty solution, but it works

We use a register function (that is, a function that is called with
PKG_ADD directive, e.g. see [1]) to force the correct loading order.
I explain (call PKG the package and DEP the dependency)

1. When the register function is called PKG is already in the path,
but not DEP (bug!)
2. We unload PKG and load DEP
3. we load PKG

Because at runtime of 3. DEP is already loaded, luckily pkg.m is not
forcing a reload of it when it load PKG.

The code looks like this (user case PKG=geometry, DEP=matgeom)

## PKG_ADD: __geometry_package_register__ (1);
## PKG_DEL: __geometry_package_register__ (-1);

function __geometry_package_register__ (loading = 0)

  if (loading > 0)
    ## Check if matgeom is loaded
    if (pkg('list','matgeom'){1}.loaded == 0)
      ## Shadow matgeom functions
      ## i.e. first load matgeom then geometry
      pkg unload geometry  # because pkg.m has already loaded it,
before its dependencies!
      pkg load matgeom     # force first load of dependency to shadow
      pkg load geometry    # reload the package
    endif
  elseif (loading < 0)
    # we do not care about unloading
  endif

endfunction

Because this solves the problem at hand, I am convinced that the
current behavior of pkg.m is buggy

[1]: 
https://sourceforge.net/p/octave/matgeom/ci/master/tree/octave_pkg/inst/__matgeom_package_register__.m

On Thu, Dec 12, 2019 at 2:26 AM Juan Pablo Carbajal
<address@hidden> wrote:
>
> Thanks Philip for looking into this.
> As said many times pkg.m should be replaced: burn to the ground, and
> reborn like the fenix.
> I am not convinced pkj does a much better job, but it at least enjoys
> a more maintainable structure.
>
> > 2. Taking into account the contents of other posts here, pkg.m is actually
> > doing the exact right thing, i.e., when loading a package it first adds the
> > desired package directories to the path, and only then the directories of
> > the package dependencies, AFAICS all in the proper order. But see 4., below.
>
> As far as I understand it , this is actually all wrong. The
> dependencies should be added first and then the desired package.
> Only this way the packages functions are found first.
> Only then you get consistent behavior with what happens if the package
> shadows an octave function (I hope we all agree octave is a dependency
> of the package).
> That is, you insert the latest loaded package on the top of the path.
> So if you start by loading dependencies from the leaf nodes, this is
> properly resolved.
> But of course there might be other problems waiting hidden in that
> implementation.
>
> >
> > 3. But what you want to accomplish is exactly the opposite. You want that to
> > make sure that functions in some package can shadow similarly named
> > functions in its dependencies.
>
> Exactly, which is consistent to what happens if you shadow a core function.
> And as I see it, the desired and expected behavior (principle of no
> surprise: why are paths to core/m treated differently than paths to
> packages?).
>
> >
> > 4. Dependency directories are at the top of the path and searched first, as
> > one normally would expect and desire, because of the thinking that
> > dependencies may be needed during initialization of the desired package.
> > However, the weak point in this reasoning is that it is valid for *all*
> > packages. Some, if not most, packages don't need dependencies for
> > initialization, they just need dependencies to supply functionality for them
> > at run time, not initialization time.
>
> I guess it depends on the definitions here, but as of now, I do not
> see any troubles in accepting that:
>
> 1. During "initialization" dependencies are available, but the package
> itself isn't (call it pre_load if you want).
>
> 2. After initialization the package is actually loaded and its folders
> added to the top of the search path. At this point dependencies are
> shadowed if the package defines homonymous functions.
>
> >
> > Looking at how pkg.m works, getting together point 3. above would be
> > difficult because of the following;
> >
> > * Process:
> > If we adapt pkg.m, it would very probably only apply to Octave-6.1.x, and
> > the packages matgeom, geometry (and mapping) (the actual packages concerned
> > here) would depend hard on the latest version of Octave and on the latest
> > version of each other. I think that would be an undesirable situation.
> > Packages should be backward-compatible with a few older octave releases.
> >
> I do not generally agree with this, since it is only some
> functionality that is lost when using older version of octave. Most of
> the package still works.
> For the user case at hand: for octave < 6.1 geometry::clipPolygon is
> not implemented.
>
> > * Technically:
> > Adapting pkg.m to first load dependencies and only then the desired package,
> > if so desired for some package, is surely possible.
> > But the story is more convoluted: just think of the possibility that one of
> > the dependencies *does* need another dependency for its initialization so
> > needs that dependency earlier in the path (= loaded later).
>
> As described before, afai understand it, this is a no problem, as the
> dependencies were already resolved during pre_load of the "another
> package"
>
> > The implication is that the whole thing is going to be relying on the needs
> > of individual packages and cannot be catched in e.g., another flag for
> > pkg.m.
> > The most reliable way to get it together would be to add another field to
> > the package DESCRIPTION file or amend the "Depends" field to indicate how
> > some package wants to deal with each of its individual dependencies. That
> > has to be done in such a way that all currently existing packages aren't
> > affected.
> > I fear that is going to be a big headache at this moment.
>
> This is a no-go. Specially when all other dependency manager I
> mentioned (of course, with proper namespaces!) handle this correctly
> and do not require this extra information.
>
> >
> > Now as regards matgeom and geometry:
> > * matgeom is a dependency of geometry
> > * matgeom has been split from geometry for several reasons, mostly for ease
> > of upgrading geometry and matgeom separately, (IIRC) license stuff,
> > perceptions regarding external or community packages.
> > * nevertheless geometry and matgeom are heavily interdependent.
> > I suppose matgeom and geometry won't be used much separately.
>
> matgeom should be completely oblivious to geometry. Matgoem is a
> stand-alone package, it does not depend on geometry.
>
> > Taking this into account, for the time being I'd advise to simply remove the
> > offending functions in matgeom (or rename them).
> > Maybe that can be done in some init script in the geometry package that
> > checks if matgeom is installed (should be as it's a dependency of geometry)
> > and if so, processes the pertinent matgeom function files. Hopefully that
> > won't give trouble, as this will rename a file actually in the search path
> > (cached?).
>
> Nah, this is a no-go either.
> If this is the only solution, then I would rename geometry's functions
> to avoid the whole problem.