Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't build the extension in pre-commit environments #1226

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions hatch_build.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import logging
import os
import sys

from hatch_jupyter_builder.plugin import JupyterBuildHook

LOGGER = logging.getLogger(__name__)


class CustomBuildHook(JupyterBuildHook):
"""
We use a custom build hook to detect pre-commit environments. In those
environments there is no need to build the extension (and, in addition
the build will fail if npm is not available, which is often the case
of pre-commit environments).
"""

def initialize(self, version, _):
if "/.cache/pre-commit/".replace("/", os.path.sep) in sys.executable:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea. But I am not quite sure how are you detecting if the current installation is done by pre-commit and not a regular installation.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, so far I have not found a better way than checking whether the Python path contains /.cache/pre-commit/ (I still need to check how it looks like on Windows)

LOGGER.info(
"This environment looks like a pre-commit environment. "
"We will skip the build of the Jupytext extension for JupyterLab."
)
self._skipped = True
return

return super().initialize(version=version, _=_)


# hatch wants a single hook
del JupyterBuildHook
8 changes: 4 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ packages = ["src/jupytext", "src/jupytext_config", "jupyterlab/jupyterlab_jupyte
"jupyterlab/jupyter-config" = "etc/jupyter"
"jupyterlab/jupyterlab_jupytext/labextension" = "share/jupyter/labextensions/jupyterlab-jupytext"

[tool.hatch.build.hooks.jupyter-builder]
[tool.hatch.build.hooks.custom]
# Enable running hook by default.
# We disable this hook only by setting env var HATCH_BUILD_HOOKS_ENABLE=false
# So `HATCH_BUILD_HOOKS_ENABLE=false pip install .` will **not** build JupyterLab related
# extension. A simple `pip install .` will **always** build extension
enable-by-default = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not set enable-by-default = false, so that extension will not build by default. In the CI, we will need to add HATCH_BUILD_HOOKS_ENABLE=true to force the building of extension.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well sure that would solve this issue indeed! That's how we were doing it up to version 1.14.x I believe. Would that be ok with you? I'll be careful to make sure that we don't miss the extension in the conda package too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, most of the jupytext "contributors" are interested in developing core jupytext and do not touch its extension. If that is the case, I think it makes more sense to disable extension building by default as it completely avoids node dependency.

So, yes, I think it makes sense for me to set enable-by-default = false and more over, that is what I proposed originally when we moved to hatch build system.

# Runtime dependency for this build hook
# We need jupyterlab as build time depdendency to get jlpm (wrapper around yarn)
# We need jupyterlab as build time dependency to get jlpm (wrapper around yarn)
dependencies = [
"hatch-jupyter-builder>=0.5", "jupyterlab>=4"
]
Expand All @@ -154,15 +154,15 @@ ensured-targets = [
# the extension, build will be triggered even if the build assets exist
skip-if-exists = ["jupyterlab/jupyterlab_jupytext/labextension/static/style.js"]

[tool.hatch.build.hooks.jupyter-builder.build-kwargs]
[tool.hatch.build.hooks.custom.build-kwargs]
# Root directory where build should be done
path = "jupyterlab"
# Build command that is defined in package.json
build_cmd = "build:prod"
# We use jlpm, which is wrapper around yarn to build transpiled assets
npm = ["jlpm"]

[tool.hatch.build.hooks.jupyter-builder.editable-build-kwargs]
[tool.hatch.build.hooks.custom.editable-build-kwargs]
path = "jupyterlab"
build_cmd = "build"
npm = ["jlpm"]
Expand Down
Loading