svanderburg
node2nix ignores package-lock.json on collections of packages when fetching from remote
I have a concrete problem with a package in nixpkgs: https://github.com/NixOS/nixpkgs/tree/master/pkgs/servers/matrix-synapse/matrix-appservice-slack
The package currently fetches the sources from github, compiles the tsc sources and installs the package.
However, node2nix ignores the remote package-lock.json for packages that are defined as a collection in the nixpkgs package.json as per https://github.com/svanderburg/node2nix#deploying-a-collection-of-npm-packages-from-the-npm-registry
This poses a problem when deploying sources that require a correct package-lock.json, as in this case, the dependencies of upstream packages are not compatible (typescript compile) with the dependencies required to build the package.
Thus, bumping the package to version 1.6.0 (latest stable) will fail during npm build.
@kampka Yesterday, I was trying to develop some test cases so that this kind of change is guarded from potential future breakage.
I wrote two testcases: one that uses a dependency from a local directory (with lock file), and one referring to a git repository (with lock file).
However, what I noticed is that the dependencies in the lock file are ignored. The project simply reevaluates the dependencies provided by the dependency's package.json file and then "freezes" the closure in the project's lock file. In one of my test cases, multiple libraries seem to resolve to their latest versions, instead of older versions that the lock file specifies. So it seems that your theory is not valid.
After studying some NPM docs, I learned that package-lock.json files are not supposed to be used for public libraries (but npm shrinkwrap can). This also explains that if you run npm publish that the package-lock.json file is automatically ignored.
About the matrix-appservice-slack package: I think your modification does seem to fix that package. I believe the incompatibility has a different root cause. Not so long ago, I discovered a subtle flaw in my replicated NPM dependency resolution algorithm -- it tries to mimic NPM's behaviour for reusing dependencies provided by parent node_modules/ folders.
However, my implementation only looks at the name and version attributes. It turns out that more recent versions of NPM also take the origins of the dependency into account, which my implementation ignores.
I believe one of the major reasons that a package may refer to a Git repository for a library dependency (as opposed to the NPM registry) is because that dependency requires some kind of fix/modification. With my current implementation, it may happen that instead of a dependency from Git, a version of a dependency from the NPM registry is used instead (because their name and version are identical).
I'm bit thorn on how to proceed -- I can of course roll this change back, but this will break the matrix-appservice-slack package again.
Implementing a revised dependency resolution algorithm (that also takes the origins of the dependencies into account), is not a trivial modification and will take a bit of time.
I can also for the time being keep your fix included. It will improve compatibility with some packages, but in other cases it might also break things.
@kampka I took another look at the matrix-appservice-slack package -- so the summarize: what we want is to deploy a remote development project (development projects are package-lock.json driven), rather than something that is treated as a dependency (for which package-lock.json files are ignored)
Currently, when you write a collections JSON file (a JSON array) then every entry is basically treated the same way as an NPM-dependency and npm install -g: it ignores the package-lock.json file, because this is also what NPM does.
So this explains why after your fix work the package seems to deploy -- because for every local and git dependency the package-lock.json is used all the dependencies and transitive dependencies in the matrix-appservice-slack package are used.
However, the undesired side effect of this change is that also packages with Git dependencies that have a lock file, will have all their "locked dependencies" included. This is not what NPM does, causing differences in the generation process.
I have to look into how I can change this -- what matters for the matrix-appservice-slack package is that only the package-lock.json file of the root project is used. Lock files of all transitive dependencies must be ignored.
I'm thinking about introducing a replacement feature -- giving node2nix the ability to deploy remote development projects. With this feature, we can guarantee the correct generation process.
Moreover, it will also simplify the codebase.
@kampka I have created a new experimental branch: https://github.com/svanderburg/node2nix/tree/remote-projects with a proof-of-concept implementation that does what I just explained.
I have removed your changes to the Source prototypes that fetch the lock files (because dependencies and transitive dependencies should ignore them). Instead, I made a fetchgit function that can be used to fetch remote development projects.
The idea of this new feature is that you can deploy remote development projects, and for these development projects you can deploy from lock files as well (and in a future revision: also use supplement JSON files etc. if desired).
Normally with a (local) development project, you can generate Nix expressions from a package.json (and optionally a package-lock.json) as follows:
$ node2nix -i package.json -l package-lock.json
The above command expects that the package.json and package-lock.json files are already stored on your local filesystem.
I have added a new option: --git-repository to the CLI that allows you to first fetch the corresponding Git repository and then generate Nix expressions from it. For example, the following command deploys on of my own projects:
$ node2nix -i package.json --git-repository --git-repository git+https://github.com/svanderburg/nijs
After generating the Nix expressions, I can deploy the package as follows:
$ nix-env -f default.nix -iA package
and even spawn a development shell, if desired:
$ nix-shell -A shell
Let me know if this works for you. You should do something similar for the matrix-appservice-slack package (probably you need to use both the --git-repository and -l` options)
The implementation still requires more work before it can be merged into master again.
@kampka let me know if the feature works for you.
In the meantime I have reverted the lock file inclusion because I ran into a problem with one of my non-public projects.
I intend to release a new version of node2nix very soon. Maybe this feature will go into the next version.
@svanderburg I tried to play around with your branch, and it seems to generate correct dependencies.
What strikes me as odd is that I need to provide both the packages package.json and package-lock.json from the remote repository. This seems unnecessary, given that the repo is pulled via --git-repository contains both those files.
@svanderburg Any way we can make progress on the remote-projects branch?
The fact that package-lock.json currently has to be a file that exists on the local file system is a nuisance, but it can certainly be worked around. Otherwise, the branch works fine for me.