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

jlpm run remove:package deletes your source code! #5803

Open
yuvipanda opened this Issue Dec 23, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@yuvipanda
Copy link
Contributor

yuvipanda commented Dec 23, 2018

While debugging #5802, I wanted to run a dev install of JupyterLab with a dev install of my extension. I followed the steps outlined in https://jupyterlab.readthedocs.io/en/stable/developer/extension_dev.html#extension-authoring.

Specifically, I ran:

jlpm run add:sibling ../jupyterlab-nbmetadata/

From the root of my jupyterlab master checkout. However, I couldn't find a command combination that'll load both my jupyterlab master and my local extension's master. jupyter lab --dev --watch ran just the jupyterlab master but not my extension's, and jupyter lab --watch ran my extension's but not master.

I decided to try the 'turn it off and on again' strategy, and ran

jlpm run remove:package /home/yuvipanda/code/jupyterlab-nbmetadata/

This actually deleted my source directory completely without warning! The documentation specifically says it won't do this. I lost all my source code since last git push, since this took the .git directory with it too.

@yuvipanda

This comment has been minimized.

Copy link
Contributor

yuvipanda commented Dec 23, 2018

@afshin pointed out

fs.removeSync(path.dirname(packagePath));
which does seem to delete the files.

@afshin afshin changed the title jlpm run remove:package deletes you source code! jlpm run remove:package deletes your source code! Dec 24, 2018

@ian-r-rose

This comment has been minimized.

Copy link
Member

ian-r-rose commented Jan 7, 2019

Thanks for catching this @yuvipanda, and sorry for the close call with your code 😬

The intention for remove:package was to remove packages from the JupyterLab source tree in the packages directory. We should probably check that the packagePath given to remove:package is actually in the place we expect, and error out if that is not the case.

The documentation is out-of-date (it used to be remove:sibling, and did not actually remove files so much as not-include them in the build).

I propose we do the following:

  1. Fix the documentation to say that it actually does delete the package files here
  2. Don't allow absolute paths for packages. This would mean removing this line:
    packagePath = require.resolve(path.join(target, 'package.json'));
    and instead erroring out if the package does not exist.

Does that seem reasonable to you @yuvipanda?

@ian-r-rose

This comment has been minimized.

Copy link
Member

ian-r-rose commented Jan 7, 2019

Marking as a good first issue.

@yuvipanda

This comment has been minimized.

Copy link
Contributor

yuvipanda commented Jan 8, 2019

@ian-r-rose yeah, sounds good to me. I'd say (1) is probably higher priority than (2), since other people might also lose their code :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment