Clarify package (re)loading use cases

[alerque]

A possible negative impact of the change is that the multiple package instantiation model introduced in SILE 0.13-0.14 may cause some havoc in some workflows not using a resilient class.

Originally posted by @Omikhleia in https://github.com/sile-typesetter/sile/issues/1866#issuecomment-1913434930

---

I seem to have lost track about what the perceived the problem is with the current model. I know there were some hiccups even with core packages when we first changed up the loader, but I thought the issues were ironed out. Packages are allowed to accept initialization options, so blocking the initialization would block the most obvious user facing way to re-initialize with new options. As it stands all the core packages are idempotent (or something close) and this hasn't seemed to be an issue yet.

As for 3rd party packages that want to make a different choice and only allow loading once, I don't lee what is to stop them using the current model.

local _initialized

function package:_init ()
    if _initialized then return self end
    ...
    _initialized = true
end

What is there left that is not covered by the current model of allowing re-initialization (either to correct things clobbered by other packages or use new options) and also allowing the package itself to do whatever it wants as far as whether to to actually do anything on initialization?

[alerque]

One of the early issues was that each instance of \use would cause the Lua source to be re-loaded and the existing instantiation of a package to be orphaned. This did leave some cruft around in memory and potentially ruin the shimming process. Also it made it so that a module could not readily tell if it had even been instantiated.

I believe that issue has been addressed. Consider for example:

\begin{document}
\use[module=packages.lorem]
\lua{print(SILE.packages["lorem"])}
\use[module=packages.lorem]
\lua{print(SILE.packages["lorem"])}
\end{document}

$ ./sile repack.sil
SILE v0.14.14.r303-ga8f6166 (LuaJIT 2.1.1700206165) [Rust]
<repack.sil> as sil
<./core/sile.lua:444> as lua
table: 0x55d29d722590
<./core/sile.lua:444> as lua
table: 0x55d29d722590
[1]

Note the table ID is the same even after reloading.

Is the issue that we need to also stash the actual Penlight class instances for each module? It does appear that we are not doing that (I actually thought we were but couldn't find it right now).

[Omikhleia]

Script:

\begin{document}
\lua{
print("1a. In SILE packages (class)")
for k, v in pairs(SILE.packages) do
  print(k,v)
end
print("1b. In document class (instances)")
class = SILE.documentState.documentClass
for k, v in pairs(class.packages) do
  print(k, v)
end
}

\use[module=packages.url]
\lua{
print("2a. In SILE packages (class)")
for k, v in pairs(SILE.packages) do
  print(k,v)
end
print("2b. In document class (instances)")
class = SILE.documentState.documentClass
for k, v in pairs(class.packages) do
  print(k, v)
end
}

\use[module=packages.verbatim]
\lua{
print("3a. In SILE packages (class)")
for k, v in pairs(SILE.packages) do
  print(k,v)
end
print("3b. In document class (instances)")
class = SILE.documentState.documentClass
for k, v in pairs(class.packages) do
  print(k, v)
end
}
\use[module=packages.url]
\lua{
print("4a. In SILE packages (class)")
for k, v in pairs(SILE.packages) do
  print(k,v)
end
print("4b. In document class (instances)")
class = SILE.documentState.documentClass
for k, v in pairs(class.packages) do
  print(k, v)
end
}
\end{document}

Commented output from SILE v0.14.16 (LuaJIT 2.1.1702233742)

(1. Initial state)

1a. In SILE packages (class) => EMPTY

1b. In document class (instances)
raiselower  ...
footnotes   ...
infonode    ...
leaders ...
insertions  ...
counters    ...
masters ...
twoside ...
bidi    ...
folio   ...
tableofcontents ...

(2. After \using url)

2a. In SILE packages (class) 
url table: 0x7f9ca2d651b8

2b. In document class (instances)
bidi    ...
inputfilter inputfilter: 0x7f9ca2d6f578 => LOADED BY URL
raiselower  ...
verbatim    verbatim: 0x7f9ca2d6a5c8 => LOADED BY URL
footnotes   ...
infonode    ...
leaders ...
insertions  ...
counters    ...
masters ...
twoside ...
pdf pdf: 0x7f9ca2d79cc0 => LOADED BY URL
folio   ...
tableofcontents ...

(3. After using \verbatim)

3a. In SILE packages (class)
url table: 0x7f9ca2d651b8
verbatim    table: 0x7f9ca2d6a3d0
3b. In document class (instances)
bidi    ...
inputfilter inputfilter: 0x7f9ca2d6f578
raiselower  ...
verbatim    verbatim: 0x7f9ca2d6a5c8 => UNCHANGED
footnotes   ...
infonode    ...
leaders ...
insertions  ...
counters    ...
masters ...
twoside ...
pdf pdf: 0x7f9ca2d79cc0 => UNCHANGED
folio   ...
tableofcontents ...

(4. After \using url)
4a. In SILE packages (class)
url table: 0x7f9ca2d651b8
verbatim    table: 0x7f9ca2d6a3d0
2b. In document class (instances)
bidi    ...
inputfilter inputfilter: 0x7f9ca2d7ffa0 => NEW INSTANCE
raiselower  ...
verbatim    verbatim: 0x7f9ca2d7f568 => NEW INSTANCE
footnotes   ...
infonode    ...
leaders ...
insertions  ...
counters    ...
masters ...
twoside ...
pdf pdf: 0x7f9ca2d80588 => NEW INSTANCE
folio   ...
tableofcontents ...

None of this seems logical to me:

• SILE.packages doesn't contain the package classes loaded from other paths than \use
• Packages loaded by \use are not seen as class-loaded, but their dependencies are.
• Some of these dependencies of the \used one are re-instantiated (and itself is too likely).

[Omikhleia]

By the way your example above is slightly misleading:

\begin{document}
\lua{print(SILE.packages["lorem"])}% This does actually load the package,
                                   % even without prior `\use` (but doesn't instantiate it)
\end{document}

SILE v0.14.16 (LuaJIT 2.1.1702233742)
<temp/pack.sil> as sil
</usr/share/sile/core/sile.lua:444> as lua
table: 0x7f61d47c27b8

[Omikhleia]

And a "reasonable" use case.

\begin{document}
% In my preamble...
\use[module=packages.url]
\define[command=urlstyle]{<\process>}% My style for this book.

% Later in some included document...
See \url{https://sile-typesetter.org/}

% Much later in some other included document, also used in another book...
\use[module=packages.url]
See also \url{https://sile-typesetter.org/}

\end{document}

Do you really think this is what people expect? Make it worse, the second \use[module=packages.url] could be indirect (i.e. loading another packages, that wants to ensure "url" is loaded as it will use it in its own commands).

The question is not related to my packages (whether they'd hack a if _initialized then return self end or whatever for their own sake). I'm only using core packages in this example, and they get re-instantiated multiple times, and overrides got cancelled, just because one typist included something apparently innocuous in a deeply nested sub-document. And this gets past that included file, and affects the rest of the document...

You may say it's per design. I might say this is a wrong and broken paradigm.

[Omikhleia]

Make it worse (...) indirect (...)

\begin{document}
\use[module=packages.url]
\define[command=urlstyle]{<\process>}% My style for this book.
See \url{https://sile-typesetter.org/}

\use[module=packages.markdown]
\begin[type=markdown]{raw}
Hello from _Markdown_.
\end{raw}
\par% Unrelated crust.

See \url{https://sile-typesetter.org/}
\end{document}

Oops:

= "markdown" internally loads "url" (via self:loadPackage("url") in some init) just to ensure \href is available. (It doesn't even use \url (yet)).

[alerque]

Thanks for the feedback. There are some points here that take me by surprise because that is not what I thought was going on, and also a couple points that I disagree with (e.g. yes in some cases I expect the results you suggest are unexpected), but either way this gives me something to work through.

[alerque]

One major issue I just uncovered is that class:loadPackage() (which I use extensively) is handling instantiation differently than sile -u, SILE.use() or \use. Hence why I haven't noticed some issues with the latter. The former is storing instantiated copies of packages, the latter is storing the package class not the instance.

[alerque]

@Omikhleia Have you used SILE develop at all since #1908? It looks like I need to apply that stuff to SILE.use() is it gets picked up elsewhere too, but at a guess will those changes to instantiation cover your usage expectations?

[Omikhleia]

How can this be tagged as a "documentation" (only) issue, since:

There are some points here that take me by surprise because that is not what I thought was going on

I had problem with this since July 2022 (https://github.com/sile-typesetter/sile/pull/1366#issuecomment-1200265306), which was unanswered

I then opened a dedicated issue in August 2022, https://github.com/sile-typesetter/sile/issues/1531 which got closed though the above-mentioned concerns were unaddressed.

I was bugged here in Dec 2022, and mentioned the multiple instantiation problem again in my feedback.

I reported numerous related issues (let's pick just one, February 2023, #1708)... That were fixed without concern for the global re-instantiation picture.

Honestly, my sile·x stuff was mostly created in Jul 30, 2023 because of this very pain point, as I wanted to remove hacks from other components. Sure, it then received other stuff (either in-advance experiments or backported PRs), but for everything new I've tried to play fair and also propose upstream updates too when I felt them to be ready. The multiple re-instantiation issue was the initial forking point.

I was bugged again in Nov. 2023 -- though it does not fix anything...

I was yet bugged again in Nov. 2023 for the very way sile·x worked -- and I addressed it, though it implies other workflows as described above might be affected.

Frankly, I don't know what you expect me to do here -- I've said numerous times I was ill-at-ease with this behavior and could not find a way to sort it out.

[Omikhleia]

Erm, re-reading my previous answer, it might sound harsh, which wasn't the intent. I'm really at loss on the topic (for a long time indeed, but that's life). I don't even have any subtle idea what would be 'the couple points" where you would disagree based on the above scenarios, and what are the good reasons for this (but if it's for the very few packages that truly have init options, they might be an edge-case which could have been addressed differently without re-instantiation)

[alerque]

How can this be tagged as a "documentation" (only) issue, since:

It isn't. I also tagged it https://github.com/sile-typesetter/sile/labels/todo. As in it isn't a https://github.com/sile-typesetter/sile/labels/bug or https://github.com/sile-typesetter/sile/labels/enhancement but does require some code work. At this point at the very least fixing the use loader to work the same as the class method, and probably more to protect the existing instances if present. At the same time it also needs https://github.com/sile-typesetter/sile/labels/documentation and also is an open https://github.com/sile-typesetter/sile/labels/question because I didn't (maybe don't) fully understand the use cases.

[alerque]

Frankly, I don't know what you expect me to do here

At this point the main gist of my expectation is in regard to this comment: Does the behavior in the develop branch slated to become v0.15.0 address your concerns or not? Your previous issue #1531 was closed by #1908, but your comments in this issue are evidently from the 0.14.x release series and still showing the old behavior. For example this MWE:

And a "reasonable" use case.

\begin{document}
% In my preamble...
\use[module=packages.url]
\define[command=urlstyle]{<\process>}% My style for this book.

% Later in some included document...
See \url{https://sile-typesetter.org/}

% Much later in some other included document, also used in another book...
\use[module=packages.url]
See also \url{https://sile-typesetter.org/}

\end{document}

This is not the result you should be getting with the develop branch. I get this:

While for this particular sample it gives the same results, for more advanced cases #1979 takes things a step farther and makes the new behavior more robust to allow reloads of modules to load the same instance as the original load (so that anything in the package's self is not lost even when explicitly asking for a reload that resets commands and settings).

[Omikhleia]

Does the behavior in the develop branch slated to become v0.15.0 address your concerns or not?

Quick answer: I don't know yet - I am slowly getting there, but I haven't tried the develop branch much. All my above examples were on (as-of-now current) SILE 0.14.16. (My production workflow (for both the book I made in Nov. 2023 as well as the book I'm currently typesetting) is still in on a custom 0.14.11 build with many patches, and not even using my own latest "resilient" modules ^^ -- My development workflow just migrated from 0.14.14 to 0.14.16 (skipping all other versions in between), with no hiccups so far, so I plan to update my production workflow, but I am very cautious here.)

[alerque]

@Omikhleia Did you ever get a chance to test the develop branch in relation to package reloading to see if it actually addresses your usage concerns like I think it does?