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

Enable PNPM and rspack #385

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
77 changes: 21 additions & 56 deletions invenio_cli/commands/assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@

"""Invenio module to ease the creation and management of applications."""

import subprocess
from pathlib import Path

import click
from pynpm import NPMPackage

from ..helpers import env
from ..helpers.process import ProcessResponse, run_interactive
Expand All @@ -26,17 +24,16 @@ def __init__(self, cli_config):
"""Constructor."""
super().__init__(cli_config)

@staticmethod
def _module_pkg(path):
def _module_pkg(self, path):
"""NPM package for the given path."""
return NPMPackage(Path(path) / "package.json")
path = Path(path) / "package.json"
return self.cli_config.javascript_package_manager.create_pynpm_package(path)

def _assets_pkg(self):
"""NPM package for the instance's webpack project."""
return self._module_pkg(self.cli_config.get_instance_path() / "assets")

@staticmethod
def _watch_js_module(pkg):
def _watch_js_module(self, pkg):
"""Watch the JS module for changes."""
click.secho("Starting watching module...", fg="green")
status_code = pkg.run_script("watch")
Expand All @@ -48,24 +45,12 @@ def _watch_js_module(pkg):
status_code=status_code,
)

@staticmethod
def _run_script(module_pkg):
"""Run script and return a ProcessResponse."""
status_code = module_pkg.run_script("link-dist")
if status_code == 0:
return ProcessResponse(
output="Module linked correctly to global", status_code=0
)
else:
return ProcessResponse(
error=f"Unable to link-dist. Got error code {status_code}",
status_code=status_code,
)

@staticmethod
def _npm_install_command(path):
def _npm_install_command(self, path, module_pkg):
"""Run command and return a ProcessResponse."""
status_code = subprocess.call(["npm", "install", "--prefix", path])
install_args = self.cli_config.javascript_package_manager.install_local_package(
path
)
status_code = module_pkg.install(" ".join(install_args))
if status_code == 0:
return ProcessResponse(
output="Dependent packages installed correctly", status_code=0
Expand All @@ -89,28 +74,6 @@ def _build_script(module_pkg):
status_code=status_code,
)

@staticmethod
def _assets_link(assets_pkg, module_pkg):
try:
module_name = module_pkg.package_json["name"]
except FileNotFoundError as e:
return ProcessResponse(
error="No module found on the specified path. "
f"File not found {e.filename}",
status_code=1,
)

status_code = assets_pkg.link(module_name)
if status_code == 0:
return ProcessResponse(
output="Global module linked correctly to local folder", status_code=0
)
else:
return ProcessResponse(
error=f"Unable to link module. Got error code {status_code}",
status_code=status_code,
)

def watch_assets(self):
"""High-level command to watch assets for changes."""
watch_cmd = self.cli_config.python_package_manager.run_command(
Expand All @@ -135,25 +98,27 @@ def link_js_module(self, path):
steps = [
FunctionStep( # Install dependent packages
func=self._npm_install_command,
args={"path": path},
args={"path": path, "module_pkg": module_pkg},
message="Installing dependent packages...",
),
FunctionStep( # Run build script
func=self._build_script,
args={"module_pkg": module_pkg},
message="Building...",
),
FunctionStep( # Create link to global folder
func=self._run_script,
args={"module_pkg": module_pkg},
message="Linking module to global dist...",
),
FunctionStep( # Link the global folder to the assets folder.
func=self._assets_link,
]

# The commands necessary for linking local JS packages vary by package manager
js_package_manager = self.cli_config.javascript_package_manager
link_steps = [
FunctionStep(
func=step.function,
args={"assets_pkg": assets_pkg, "module_pkg": module_pkg},
message="Linking module to assets...",
),
message=step.message,
)
for step in js_package_manager.package_linking_steps()
]
steps.extend(link_steps)

return steps

Expand Down
17 changes: 10 additions & 7 deletions invenio_cli/commands/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,17 @@ def update_statics_and_assets(self, force, debug=False, log_file=None):
Needed here (parent) because is used by Assets and Install commands.
"""
# Commands
pkg_man = self.cli_config.python_package_manager
ops = [pkg_man.run_command("invenio", "collect", "--verbose")]
py_pkg_man = self.cli_config.python_package_manager
js_pkg_man = self.cli_config.javascript_package_manager
ops = [py_pkg_man.run_command("invenio", "collect", "--verbose")]

if force:
ops.append(pkg_man.run_command("invenio", "webpack", "clean", "create"))
ops.append(pkg_man.run_command("invenio", "webpack", "install"))
ops.append(py_pkg_man.run_command("invenio", "webpack", "clean", "create"))
ops.append(py_pkg_man.run_command("invenio", "webpack", "install"))
else:
ops.append(pkg_man.run_command("invenio", "webpack", "create"))
ops.append(py_pkg_man.run_command("invenio", "webpack", "create"))
ops.append(self._statics)
ops.append(pkg_man.run_command("invenio", "webpack", "build"))
ops.append(py_pkg_man.run_command("invenio", "webpack", "build"))
Comment on lines +89 to +97
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is still a good amount of optimization potential by calling the invenio command only once and thus saving all the redundant application initializations which eat up a lot of time.
one way to move forward is to implement a single larger command (e.g. in invenio-app-rdm) that performs all the steps currently implemented here, based on the provided CLI args.
@ntarocco has pointed out to me that something very much like that has already been done in Invenio-App-ILS

Copy link
Contributor

Choose a reason for hiding this comment

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

an option would be to use a rpc-server implementation. a WIP would be here for invenio-cli combined with PR from invenio-app

# Keep the same messages for some of the operations for backward compatibility
messages = {
"build": "Building assets...",
Expand All @@ -108,7 +109,9 @@ def update_statics_and_assets(self, force, debug=False, log_file=None):
if op[-1] in messages:
click.secho(messages[op[-1]], fg="green")
response = run_interactive(
op, env={"PIPENV_VERBOSITY": "-1"}, log_file=log_file
op,
env={"PIPENV_VERBOSITY": "-1", **js_pkg_man.env_overrides()},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here we tell invenio about the JS package manager that we want to use, via environment variables

log_file=log_file,
)
if response.status_code != 0:
break
Expand Down
29 changes: 24 additions & 5 deletions invenio_cli/helpers/cli_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,19 @@
"""Invenio-cli configuration file."""

from configparser import ConfigParser
from functools import cached_property
from pathlib import Path

from ..errors import InvenioCLIConfigError
from .filesystem import get_created_files
from .package_managers import UV, Pipenv, PythonPackageManager
from .package_managers import (
NPM,
PNPM,
UV,
JavascriptPackageManager,
Pipenv,
PythonPackageManager,
)
from .process import ProcessResponse


Expand Down Expand Up @@ -62,12 +70,10 @@ def __init__(self, project_dir="./"):
with open(self.private_config_path) as cfg_file:
self.private_config.read_file(cfg_file)

@property
@cached_property
def python_package_manager(self) -> PythonPackageManager:
"""Get python packages manager."""
manager_name = self.config[CLIConfig.CLI_SECTION].get(
"python_package_manager", None
)
manager_name = self.config[CLIConfig.CLI_SECTION].get("python_package_manager")
if manager_name == Pipenv.name:
return Pipenv()
elif manager_name == UV.name:
Expand All @@ -82,6 +88,19 @@ def python_package_manager(self) -> PythonPackageManager:
"Could not determine the Python package manager, please configure it."
)

@cached_property
def javascript_package_manager(self) -> JavascriptPackageManager:
"""Get javascript packages manager."""
manager_name = self.config[CLIConfig.CLI_SECTION].get(
"javascript_package_manager"
)
if manager_name == NPM.name:
return NPM()
elif manager_name == PNPM.name:
return PNPM()

return NPM()

def get_project_dir(self):
"""Returns path to project directory."""
return self.config_path.parent.resolve()
Expand Down
Loading