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

[Feature] allow packageLocations in .pnp.cjs to be absolute paths #4030

Open
1 of 2 tasks
adrian-gierakowski opened this issue Jan 28, 2022 · 15 comments
Open
1 of 2 tasks
Labels
enhancement New feature or request

Comments

@adrian-gierakowski
Copy link

  • I'd be willing to implement this feature (contributing guide)
  • This feature is important to have in this repository; a contrib plugin wouldn't do

Describe the user story

As a NixOS package maintainer or a developer wanting to package node applications using yarn berry with nix, I'd like to be able to reference packages stored in the /nix/store using absolute paths, instead of relative.
Currently paths starting with / are resolved as relative to the project (not sure if this is intended, it could be a bug?)

Describe the solution you'd like

Detect when path start with / and treat the path as absolute, instead of relative.

Describe the drawbacks of your solution

existing code/projects might depend on current behaviour

Describe alternatives you've considered

I can resolve the paths to relative ones, and as long as the .pnp.cjs remains at it's original location in /nix/store things will work just fine. However this adds an extra step to the build process and makes it impossible to simply copy .pnp.cjs to a different location (relatively to /nix/store) and use it from there.

@adrian-gierakowski adrian-gierakowski added the enhancement New feature or request label Jan 28, 2022
@arcanis
Copy link
Member

arcanis commented Jan 28, 2022

Currently paths starting with / are resolved as relative to the project (not sure if this is intended, it could be a bug?)

It's intended; otherwise the path wouldn't be portable across systems. Instead, you should ensure the relative path between the cache and the project is always the same.

@adrian-gierakowski
Copy link
Author

@arcanis thank you for clarifying. Do you think this behaviour could be customised via a plug-in? Given that I’m packaging software for a specific Linux distro, I don’t care about compatibility with other system. Btw, the absolute paths to /nix/store would work across all systems on which nix can be installed.

@arcanis
Copy link
Member

arcanis commented Jan 28, 2022

If /nix/store is some kind of Nix global we can rely on, then I think it'd make sense to generate absolute paths when we detect it. Do you want to try to PR it? It should mostly be about adding the right check here, plus adding a test here.

@adrian-gierakowski
Copy link
Author

adrian-gierakowski commented Feb 4, 2022

@arcanis thank you, that's very kind! I took a bit longer to reply since I wanted to tests things out first.

Yes, /nix/store paths are portable and immutable across instances of the NixOs system (or any other systems on which nix package manager can be installed). It's a bit like yarn cache, as it's used for storing content addressed sources, but it is also used to store build results, which are addressed by the hash of all their build inputs (with those inputs being restricted to other /nix/store paths and configuration variables). So, for example, the individual "unplugged" packages, and the final packaged application, will also end up in the nix store.

I modified the function you pointed to (see here), but then ran into issues at runtime, since it seems like there are other places where relative absolute paths are assumed. Here are the other changes I had to make. Finally I also had to convert the packageLocations of null package and the top level workspace:.in .pnp.js to absolute paths. I am kind of bothered by the fact that I am not able to mix absolute paths with relative ones though. I have a feeling I am missing something. Would you have any ideas? Thanks!

@adrian-gierakowski
Copy link
Author

adrian-gierakowski commented Feb 4, 2022

btw. I also made a modification which allows the yarn cache directory to contain symlinks to packages. See here and a fixup commit here. The symlinks are resolved to the destination path at .pnp.cjs creation time. I believe it might address this issues. Not sure if this breaks any existing functionality, but I needed it since at build time, I populate the cache directory with symlinks to packages in /nix/store (which yarn seems to be happy with), but I want them resolved to the actual paths when producing .pnp.cjs, since at runtime the symlinks don't seem to work. Would you be open to accepting a PR with this change? Thanks!

@rbf
Copy link

rbf commented Feb 12, 2022

I landed here after googling for "yarn pnp.cjs absolute path". This is just a quick +1 for this OP's feature and another use case for it to help you better understand a potential solution.

TL;DR

In our case the issue is that we use a container with all cached dependencies for both dev and CI and the point where the app folder is mounted on both cases is different (potentially unpredictable) so that relative paths in .pnp.clj can only be resolved for one of both cases.

For reference, my first reaction was to go to the .yarnrc.yml documentation and search for a configuration key such as pnpUseAbsolutePaths, which of course, I didn't find.

Short story long

We are capturing all dependencies of the project (including tools, languages, dependencies and other libraries) in a docker image that is then used for both dev and CI. In order to capture all this context, we have to store it in a hardcoded directory in the image, which has to be different from the app code directory because that directory is mounted on the container. That is in fact the main reason why we use yarn since it allows to store all cached and installed dependencies in a custom directory: in our case, a hardcoded directory of the docker image different from the directory where we mount the code directory for dev and CI.

Workarounds

In our use case we have two workarounds:

  1. Running yarn install when dependencies cannot be resolved (i.e. in each new CI job and when a new dev environment is created). Since the dependencies are properly cached in the docker image, yarn install effectively takes less than a second or two because it is only adding the necessary ../ at the beginning of each packageLocation value.

  2. Another workaround, although more fragile, is to ensure that the app mount point is the same (or relatively the same) so that the relative paths in .pnp.cjs successfully resolve in both cases.

@adrian-gierakowski
Copy link
Author

@arcanis any chance you'd be able to provide some feedback on my messages above? Thanks!

@stevenyxu
Copy link

I'm sharing some details about my use case (supporting Yarn PnP in Bazel) in case it would be informative to the course of this fix. Thanks to all the contributors here.

In Bazel's case, sandboxing correctly is hard in the JS ecosystem. We are currently choosing to copy files instead of set up symlink farms for 2 reasons: (1) Node's default symlink following breaks out of the sandbox if we have a symlink farm and (2) Yarn does not support symlink farms for cache zips #3514. Copying the zips into a consistent relative location inside the sandbox is correct and to @arcanis's point the most portable option, but the copy (6k zips in our case) is expensive in practice. We can explore solutions like hard links, but I'm concerned about the complexity. Ultimately, the ability for all sandboxes to refer to a single global cache would be quite helpful. Do the other use cases here also plan to use a global cache? Perhaps we could condition on that instead of having a magic nix exception, for example?

We'll continue to explore options here, and I'll write back if we are successful with a local resolution.

@madjam002
Copy link

I'm working on a way of packaging Yarn PnP applications here - https://github.com/madjam002/yarnpnp2nix

Is there a reason why the paths need to be absolute when building with Nix? The .pnp.cjs file will be stored in a fixed location, so the relative path to the /nix/store can be worked out from there? That's what I've done with my approach and it seems to work fine.

I tried going down the route of absolute paths but it ended up not working as there are other tools now that understand the PnP format which also expect relative paths, so introducing support for absolute paths will require changing those tools as well.

Overall, Yarn PnP is really great here because it allows each package to be stored in the /nix/store individually, that way caching can be done at a more granular level rather than "refetch everything when a single dependency changes".

@adrian-gierakowski
Copy link
Author

Hi @madjam002, exciting to see someone working on such a project!

The .pnp.cjs file will be stored in a fixed location, so the relative path to the /nix/store can be worked out from there?

yes, however I wanted for the generated .pnp.js to be “portable”; for example I’d like to be able to just copy it to the root of a project outside the nix store (for example while developing) and have it just work. This way you can achieve “zero install” the nix way: with deps transparently pushed\pulled to\from binary cache instead of being checked into the repo.

@adrian-gierakowski
Copy link
Author

Btw I’ve got forks of yarn 3 and https://github.com/stephank/yarn-plugin-nixify which work with absolute paths in .pnp.js but maintaining a fork of yarn is not something I’d like to do long term.

@adrian-gierakowski
Copy link
Author

If pnp is becoming a standard supported by multiple tools, I think it would be worth advocating for support for absolute paths. I really don’t see any reason why they should not be supported

@madjam002
Copy link

Hi @madjam002, exciting to see someone working on such a project!

The .pnp.cjs file will be stored in a fixed location, so the relative path to the /nix/store can be worked out from there?

yes, however I wanted for the generated .pnp.js to be “portable”; for example I’d like to be able to just copy it to the root of a project outside the nix store (for example while developing) and have it just work. This way you can achieve “zero install” the nix way: with deps transparently pushed\pulled to\from binary cache instead of being checked into the repo.

Ah okay, with my approach I'm not really treating .pnp.cjs as something that is portable, it gets generated and then should stay in place where it is in the /nix/store. I've recently added a shellHook which does drop you into a shell that has all of the packages installed and a .pnp.cjs created in a temp directory, but I mainly use this for just running unit tests or database migrations during CI/CD rather than full blown development. My main project is a large monorepo with lots of packages as Yarn Workspaces, so for now I'm happy with the normal Yarn workflow and a bit of nix shell magic, although this does mean that packages are duplicated as they are also installed in the Yarn global cache.

If pnp is becoming a standard supported by multiple tools, I think it would be worth advocating for support for absolute paths. I really don’t see any reason why they should not be supported

Yes I agree I don't see in any harm in adding support for absolute paths. It will then be possible for people to shoot themselves in the foot, but this only really affects tooling developers as most people I imagine won't be modifying their .pnp.cjs files, and absolute paths would then provide a bit more flexibility to tooling developers.

The main issue I ran into was with esbuild which has implemented the PnP spec in Go, so patches to .pnp.cjs didn't help as it just looks at the RAW_RUNTIME_STATE JSON contained within and has a separate implementation for the actual module resolution.

@adrian-gierakowski
Copy link
Author

If /nix/store is some kind of Nix global we can rely on, then I think it'd make sense to generate absolute paths when we detect it. Do you want to try to PR it? It should mostly be about adding the right check here, plus adding a test here.

@arcanis I'd like to work on a PR to make this happen. Have there been any changes between v3 and v4 which need to be considered?

Given that it's now recommended to use global cache, the path in .pnp.cjs are not portable anymore: if I move the project directory relative to the global cache dir, it will break. So it would actually make sense to use absolute paths in .pnp.cjs when global cache is used, or make them relative to global cache dir, and then allow the cache dir path to be set separately.

Thanks!

@szamanr
Copy link

szamanr commented Mar 26, 2024

i second this. AFAIK yarn recommends to commit the .pnp.cjs file, but how can you do that when it contains relative references to the global cache and you have more than 1 collaborator or machine? you would have to keep overwriting the file.

i just run into this after i had moved my project to a different directory, and it wouldn't work until i run yarn install. afterwards, i have unstaged changed, despite nothing changing in the code.

i think the paths in .pnp.cjs should wither be relative to the global cache, or be absolute (e.g. ~/.yarn/berry/cache/...). however, the former would be preferred, because it would work on non-unix systems as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants
@stevenyxu @adrian-gierakowski @madjam002 @rbf @arcanis @szamanr and others