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

feat: Add detached-environments to the config, so users can move the environments out of the project folder. #1381

Merged

Conversation

ruben-arts
Copy link
Contributor

@ruben-arts ruben-arts commented May 13, 2024

Fixes: #997

Todo:

  • Fix tests
  • Add warning on already local to project installed environments
  • Add documentation
  • Add symlink from target directory folder to local .pixi folder.
  • Find solution to the deny_unknown_fields
  • Also move solve-envs

@ruben-arts ruben-arts marked this pull request as draft May 13, 2024 16:09
tests/config/config_2.toml Outdated Show resolved Hide resolved
@pavelzw
Copy link
Contributor

pavelzw commented May 14, 2024

We should probably also print a warning when we already see a populated .pixi directory to remove it because this setting is set.
Also, we might want to think about printing a warning when we see symlinks?
For example, your mounted devcontainer might have this setting enabled but locally you don't want this behavior. (although there are better options in general for pixi in devcontainers as elaborated in #997 (comment))

@ruben-arts
Copy link
Contributor Author

@pavelzw What do you mean with warning when symlinking?

Copy link
Contributor Author

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

remove debug work

tests/install_tests.rs Outdated Show resolved Hide resolved
tests/install_tests.rs Outdated Show resolved Hide resolved
@ruben-arts ruben-arts marked this pull request as ready for review May 15, 2024 10:04
@pavelzw
Copy link
Contributor

pavelzw commented May 16, 2024

What do you mean with warning when symlinking?

When I have a filled .pixi directory instead of a symlink already present, we should warn the user instead of directly deleting it.

authentication-override-file = "/path/to/your/override.json"
```

### `target-environment-directory`
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this change also affect pixi global environments?

In my setup, all HOME is persisted on a network drive. When this affects the global installation tools as well, this could break some setups

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it doesn't indeed because of the PIXI_HOME setting

@ruben-arts
Copy link
Contributor Author

This PR move the complete .pixi dir to the specified target dir. But I see a situation where it makes sense to only move the "heavy" environments. So would it make sense to only move the .pixi/envs to the target? @pavelzw @baszalmstra?

Currently it is already weird as you can specify the configuration in the projects .pixi folder but then symlinking it is not possible.

@ruben-arts
Copy link
Contributor Author

@baszalmstra I looked at all your review points and updated the symlink function a little to also do something on windows. With adding a README.txt file to the .pixi folder to tell the user looking there that it was moved.

Copy link
Contributor

@pavelzw pavelzw left a comment

Choose a reason for hiding this comment

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

💜

@ruben-arts
Copy link
Contributor Author

Fixed the unknown field issue:
image

Not sure yet why we run the config parsing twice.

@@ -343,6 +378,19 @@ impl Config {
/// # Returns
///
/// The loaded global config
/// Validate the config file.
Copy link
Contributor

Choose a reason for hiding this comment

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

The doc is misaligned

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dammit thanks for noticing!
Was a pretty big conflict, and the magic wand didn't do its magic properly

@wolfv
Copy link
Member

wolfv commented May 22, 2024

A few thoughts that came up:

This configuration option is hard to "disable" locally since it mixes two things:

  • I want my .pixi folder external
  • Where to put the .pixi folders

My proposal would be to split the two options into something like the following:

external-pixi-folder = false  # or true (false is default)
external-pixi-folder-location = "/some/path/pixi-root"  # (path to pixi folder root thingy)

The external-pixi-folder-location would, by default, be inside the global pixi folder, e.g. under:

~/.pixi/pixi-envs-v1/<project-name>-<hash>/...

or we could also drop it next to the package cache (although I am unsure wether a "cache" location is the right place). On the other hand, it "is" a cache since the definition is in the lockfile).

The benefit of having the pixi envs next to the cache would be that hard linking should be possible under most configurations (while the home folder might be on a different partition, or NFS, etc.).

@ruben-arts
Copy link
Contributor Author

This configuration option is hard to "disable" locally since it mixes two things:

For documentation of thoughts, I believe these shouldn't be two things. You either want to move it or you don't. I can't see a situation where defining both has a beneficial experience as it is not explicit what it will be doing when you have those two defined but set to false.

What about this, we kinda keep the same for the global config, just a rename.
~/.config/pixi/config.toml:

pixi-folder-location = "/etc/pixi_environments"  

Allow for relative paths in this configuration.
~/myproject/.pixi/config.toml:

# This just overrides the global configuration.
pixi-folder-location = "."  

The only problem with that is that we move the complete .pixi folder to the new location. Thus a config.toml will also needs to move with the current design.

Sidenote: the name is not the best yet, as we pixi-folder might make users think its the ~/.pixi folder which listens to $PIXI_HOME.

@chawyehsu
Copy link
Contributor

chawyehsu commented May 22, 2024

I'm thinking of the same idea of:

My proposal would be to split the two options into something like the following:

external-pixi-folder = false # or true (false is default)
external-pixi-folder-location = "/some/path/pixi-root" # (path to pixi folder root thingy)

but instead using just one config

/// Option to toggle project environments to be detached from the project directory
#[serde(untagged)]
enum DetachedEnvironments {
    Boolean(bool),
    Path(PathBuf),
}

detached-environments = Option<DetachedEnvironments>
//
// - None/Some(false): the default behavior, envs stored within `<project_root>/.pixi`
// - Some(true): toggle detached environments on, envs stored within `<pixi_cache_dir>/envs/<hash(project-x)>`
//    * `<pixi_cache_dir>` would be `XDG_CACHE_HOME/pixi` or its synonym
//    * By storing envs in `pixi_cache_dir` it is possible to leverage command like `pixi clean`/`pixi cache clean`
//      or whatever command we will introduce to clean up caches with envs. It could be configurable, e.g.
//      `pixi clean --[no-]envs`
// - Some(my_path): toggle detached environments on, envs stored in `my_path/<hash(project-x)>`

This would be much more configurable yet concise and only one key is added.

And later you can use pixi config set detached-environments true --global to enable it globally, then use pixi config set detached-environments '/path/to/nfs/pixi-envs-location' --local to explicitly config it to somewhere else for specific projects. The project-level config.toml can even be committed to the project repo for sharing the configuration of env location, would be especially convenient for shared envs I believe - and this is why I think config.toml should not be moved out of .pixi togtther with envs.

@ruben-arts
Copy link
Contributor Author

@chawyehsu We all like your idea! I'm going to implement it like that including only moving the .pixi/envs folder and not the complete .pixi folder.

@ruben-arts ruben-arts changed the title feat: Add target-environments-directory to the global config feat: Add detached-environments to the config, so users can move the environments out of the project folder. May 24, 2024
@baszalmstra baszalmstra merged commit dc5822a into prefix-dev:main May 24, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make pixi create envs somewhere else than $PWD/.pixi
5 participants