notebook icon indicating copy to clipboard operation
notebook copied to clipboard

Entry-point conflict between notebook <=6.5 and nbclassic

Open bnavigator opened this issue 3 years ago • 24 comments
trafficstars

nbclassic has the jupyter-nbextension, jupyter-bundlerextension and jupyter-serverextension entrypoints but notebook 6.5.x also still has them.

https://github.com/jupyter/notebook/blob/8794be872e5428b82dd57601da3a5e51dbab7c78/setup.py#L136-L143

https://github.com/jupyter/nbclassic/blob/82a5768f5f2dbbf405ced200a0b7bd66ca9f1468/setup.py#L133-L140

This means that one package replaces the files of the other, depending on which one gets installed later. In distribution packages like RPM based distros this is caught early and prohibits installing them together. That's especially bad since notebook 6.5 will require nbclassic for the static assets.

bnavigator avatar Aug 02 '22 17:08 bnavigator

Even worse, jupyterlab <4 depends on both nbclassic and notebook <7: https://github.com/jupyterlab/jupyterlab/pull/12767

bnavigator avatar Aug 02 '22 18:08 bnavigator

@bnavigator nblcassic>=0.4 is a replacement to notebook-6.x, so both should not be installed at the same time. Do you have cases where you need both?

We were planning to fail nbclassic>=0.4 to start if notebook-6.x was found.

echarles avatar Aug 05 '22 07:08 echarles

You may have planned that, but the reality is different:

  • Despite your PR having a different title, the current release of jupyterlab requires nbclassic and notebook <7. nbclassic >= 0.4 and notebook 6.4 satisfies these constraints and get installed in a regular new install.
  • In the 6.5 branch of notebook, there is even a requirement of nblassic >= 0.4 for the static assets.

bnavigator avatar Aug 05 '22 07:08 bnavigator

You may have planned that, but the reality is different:

This was an idea to clarify things and any feedback can change that.

Despite your PR having a different title, the current release of jupyterlab requires nbclassic and notebook <7. nbclassic >= 0.4 and notebook 6.4 satisfies these constraints and get installed in a regular new install.

Yes, the title of that PR does not reflect the discussions. @fcollonval Would you update the title of https://github.com/jupyterlab/jupyterlab/pull/12767

In the 6.5 branch of notebook, there is even a requirement of nblassic >= 0.4 for the static assets.

True, users install notebook 6.5 inherit from the nclassic>=0.4 static assets. However, the goal for those users is to launch notebook, not nbclassic, therefore my intution to fail fast in case nbclassic would be launched with jupyter nbclassic. In that scenario, the entry-points should not collide as being defined differently, see e.g. https://github.com/jupyter/nbclassic/blob/82a5768f5f2dbbf405ced200a0b7bd66ca9f1468/setup.py#L133-L139

echarles avatar Aug 05 '22 07:08 echarles

The entry-points do have the same name and get installed in $prefix/bin. Of course they do collide. One overrides the other, depending on install order.

I referenced exactly those lines in my initial post.

bnavigator avatar Aug 05 '22 07:08 bnavigator

This was an idea to clarify things and any feedback can change that.

Be aware that this is not a discussion about your future plans but about an existing problem of released packages.

bnavigator avatar Aug 05 '22 07:08 bnavigator

OK I see now, 3 of the 4 entrypoints collide. Thx for raising that.

If a user installs notebook-6.5, the nbclassic dependency will be first installed, then the notebook 6.5 will override the entry points. I guess this is what the end-user at the end is willing.

If the user then uninstalls notebook, the entrypoints will be removed, the remaining nbclassic will loose 3 of the 4 entrypoints.

Do you see other scenarios giving problem?

echarles avatar Aug 05 '22 07:08 echarles

For the jupyterlab case, this may be more tricky as it may depend on notebook and nbclassic. Should we revisit the jupyterlab dependency constraints?

echarles avatar Aug 05 '22 07:08 echarles

Be aware that this is not a discussion about your future plans but about an existing problem of released packages.

notebook 6.5 is beta pre-release to gather this kind of inputs.

I have installed in a fresh env latest official jupyterlab 3.4.4, and notebook 6.4.12 as nbclassic 0.4.3 are pulled. The colliding entrypoints are installed with the nclassic variant. nbclassic 0.4 goal is to have parity with notebook 6.4, so my best bet would be to remove the notebook dependency from jupyterlab cc/ @jtpio @fcollonval @RRosio @ericsnekbytes

echarles avatar Aug 05 '22 08:08 echarles

  • You don't have control over the install order. A user could have nbclassic already installed and then decide to upgrade notebook from 6.4 to 6.5.
  • Packaging systems like RPM, Debian, Arch Linux, prohibit packages with conflicting files. There is no overriding where one package damages the integrity of another. I worked around this restriction in openSUSE by using update-alternatives, but you are making life for distro packagers unnecessary hard.

If you already require nbclassic in notebook 6.5, why don't you just remove the duplicate entry-points from notebook 6.5?

bnavigator avatar Aug 05 '22 08:08 bnavigator

notebook 6.5 is beta pre-release to gather this kind of inputs.

It is already a problem with notebook 6.4

bnavigator avatar Aug 05 '22 08:08 bnavigator

but you are making life for distro packagers unnecessary hard.

Really sorry about that.

An really appreciated the time and feedback here.

If you already require nbclassic in notebook 6.5, why don't you just remove the duplicate entry-points from notebook 6.5?

That would work to remove the colliding issues. Underneath, the code path may be surprising, as the 3 colliding entrypoint are about activating extensions. I am pretty sure that would work as both code path (notebook-v6.5 and nbclassic) are compatible and delivering the same features. But they may read the configuration from different source (jupyter_***_config.py), so in some scenario the user may be surprised.

In case of upgrade from 6.4 to 6.5, I don't know what a user will see a remaining entrypoint.

We had discussed the option to release a npm module with the static assets, as finally this is what notebook-6.5 needs. We have finally gone to the road to use the full pypi package. I wonder if the npm approach would be finally safer to take. Just thinking loud.

echarles avatar Aug 05 '22 08:08 echarles

It is already a problem with notebook 6.4

You mean in the jupyterlab scenario?

@jtpio @fcollonval Would it make sense to pin nbclassic<0.4 on jupyterlab side for now to ease the packagers work?

echarles avatar Aug 05 '22 08:08 echarles

In any scenario where nbclassic and notebook is installed. The trigger could by jupyterlab or something else. There are people out there installing everything into system (distro packages) or user-sitelib.

bnavigator avatar Aug 05 '22 08:08 bnavigator

@jtpio @fcollonval Would it make sense to pin nbclassic<0.4 on jupyterlab side for now to ease the packagers work?

Isn't that already what was implemented in https://github.com/jupyterlab/jupyterlab/pull/12767?

Also JupyterLab 4.0 will not depend on nbclassic, only notebook_shim: https://github.com/jupyterlab/jupyterlab/pull/11894

jtpio avatar Aug 05 '22 08:08 jtpio

Isn't that already what was implemented in https://github.com/jupyterlab/jupyterlab/pull/12767?

The title is confusing and should be changed. What has finally been merged is https://github.com/jupyterlab/jupyterlab/pull/12767/files#diff-fa602a8a75dc9dcc92261bac5f533c2a85e34fcceaff63b3a3a81d9acde2fc52R40-R41

echarles avatar Aug 05 '22 08:08 echarles

Right this is because nbclassic itself used to have a pin on notebook<7.

jtpio avatar Aug 05 '22 08:08 jtpio

In any scenario where nbclassic and notebook is installed

A quick action would be to rename the nbclassic entrypoints to

           'jupyter-nbclassic-nbextension = nbclassic.nbextensions:main',
            'jupyter-nbclassic-serverextension = nbclassic.serverextensions:main',
            'jupyter-nbclassic-bundlerextension = nbclassic.bundler.bundlerextensions:main',

But we loose the target to ensure portability between notebook-6 and nbclassic.

As said, the safer and making the most sense in terms of architecture, is to publish a npm package with the static assets. This would not solve the issue in case of a user manually installing notebook and nbclassic. In that case, documentation and fast-fail is something we may consider

We are trying here to support as much cases as possible, with the most portability and backwards compatibility. At some point some tradeoffs need to be decided.

echarles avatar Aug 05 '22 08:08 echarles

A quick action would be to rename the nbclassic entrypoints ...

this maybe the easiest and most relevant option. We ensure portability of code (a notebook 6 extension will continue working on nbclassic without source code change), however, we already ask to launch with a different command : jupyter nbclassic instead of jupyter notebook. It would make sense to ask to change e.g. jupyter serverextension to jupyter nbclassic-serverextension.

echarles avatar Aug 05 '22 09:08 echarles

I commented in nbclassic's PR already, but...

What if notebook_shim owned these entry-points, and we removed the entrypoints from both nbclassic and notebook<7?

notebook_shim would attempt to load the underlying methods from nbclassic first, and fallback to notebook if not available.

Zsailer avatar Aug 08 '22 17:08 Zsailer

But they may read the configuration from different source (jupyter_***_config.py), so in some scenario the user may be surprised.

I don't think this is true, since we hardcoded the configuration name in nbclassic to jupyter_notebook_config in https://github.com/jupyter/nbclassic/pull/137

I agree with @bnavigator, we should just remove the entrypoint definitions from the notebook package, since we depend on nbclassic already. In environments where you already have the old entrypoints installed, they will be over-written with the new ones.

Packaging systems like RPM, Debian, Arch Linux, prohibit packages with conflicting files. There is no overriding where one package damages the integrity of another. I worked around this restriction in openSUSE by using update-alternatives, but you are making life for distro packagers unnecessary hard.

@bnavigator, can you clarify something for me (apologies for being naive here)? Can you replace an entrypoint in a distro when upgrading or installing a conflicting package at a later time? I would have assumed that "last write wins". In the point above, is this only happening when trying to install these two things together at the same time?

Zsailer avatar Aug 08 '22 18:08 Zsailer

I agree with @bnavigator, we should just remove the entrypoint definitions from the notebook package, since we depend on nbclassic already.

notebook 7 will depend on nbclassic, notebook 6 will not depend.

echarles avatar Aug 08 '22 18:08 echarles

notebook 6.5 branch already depends on nbclassic

bnavigator avatar Aug 08 '22 18:08 bnavigator

Can you replace an entrypoint in a distro when upgrading or installing a conflicting package at a later time? I would have assumed that "last write wins".

Depends on the distribution.

Dumb installers like pip do "last write wins". Proper packaging systems like rpm don't allow packages with conflicting files. If you have notebook 6.4 already installed and then want to install nbclassic which would install files that are already owned by the existing notebook package, the installer refuses to install nbclassic.

This is solvable by installing a new notebook package that does not own the new files anymore or have special mechanisms like "update-alternatives".

bnavigator avatar Aug 08 '22 18:08 bnavigator

New idea.

Here's a new library that would "own" the entrypoints from notebook: https://github.com/Zsailer/jupyter_notebook_entrypoint.

This would require us to change notebook v6, v7, and nbclassic to remove their entrypoints and depend on this (lightweight) package for their entrypoints. This package will resolve the entrypoints based on the packages they have installed.

Zsailer avatar Aug 12 '22 17:08 Zsailer