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

Add options to Python language module #993

Merged
merged 9 commits into from
Mar 22, 2024

Conversation

totoroot
Copy link
Collaborator

@totoroot totoroot commented Mar 1, 2024

Recently I continued using devenv for various different projects at work, so the need to have it also manage multi-language development environments got bigger.

For a lot of projects, the structure looks similar to the following.

project
├── backend
├── devenv.lock
├── devenv.nix
├── devenv.yaml
└── frontend

In this example backend would for example be the subdirectory of some Python/Django component and then there would be the frontend subdirectory e.g. some JavaScript/Node.js component.

For that reason I would like to specify a subdirectory for the individual components.

In this PR I made the necessary changes to be able to specify a working directory via languages.python.directory.
While it is already possible to pass paths of requirements files to install dependencies from, it is currently not possible to specify the location of the virtual environment or to change the working directory of the poetry command.

This makes it impossible to have the pyproject.toml file anywhere other than at the devenv root.

I also added some more missing options to languages.python.poetry.install.

@totoroot totoroot self-assigned this Mar 1, 2024
@totoroot
Copy link
Collaborator Author

totoroot commented Mar 1, 2024

I'm planning on adding support for pdm soon, and for that, these changes would facilitate its use as well.

Also, I don't think we don't want to use lib.types.path for the directory, as that would mean copying the subdirectory's content to the Nix store. Using lib.types.str and the root of the top-level devenv project as default seems fine, but please let me know in case you think otherwise.

@totoroot
Copy link
Collaborator Author

totoroot commented Mar 1, 2024

If nothing speaks against it, I would go ahead and try adding a similar option for the JavaScript module, since one might also want to have the package.json in a different directory than the devenv root

@domenkozar
Copy link
Member

See also #899, I'd really like to have a test for this before it's merged.

@totoroot totoroot force-pushed the pyproject-location branch 3 times, most recently from f1b2130 to f1f49e7 Compare March 4, 2024 10:30
@totoroot
Copy link
Collaborator Author

totoroot commented Mar 4, 2024

@domenkozar I have modified the example for python-poetry to include the newly added options.

I also added an example and tests for the introduced directory option, as you requested. In this example I have also tested the python-poetry module with the Python directory option set.

@@ -135,6 +135,14 @@ in
description = "Whether `pip install` should avoid outputting messages during devenv initialisation.";
};

directory = lib.mkOption {
Copy link
Member

Choose a reason for hiding this comment

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

does it work if you do ./. with types.path and then do config.devenv.root + cfg.directory?

Copy link
Collaborator Author

@totoroot totoroot Mar 5, 2024

Choose a reason for hiding this comment

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

I don't think I understand what you mean.

In #993 (comment) I mentioned that using types.str means not copying the specified directory to the Nix store, so poetry is still able to write the lock file.

It also gives users more flexibility when setting the option as they can set it to either an absolute path or a path relative to the devenv root with something like ./directory. I've added an example to the option.

Copy link
Member

Choose a reason for hiding this comment

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

I think you should be still able to use types.path to validate it's a path and then do mystringpath + cfg.directory

@domenkozar
Copy link
Member

Great work, only two comments and it's ready to go :)

@totoroot totoroot force-pushed the pyproject-location branch 4 times, most recently from 0ddeaae to 5b5e8e0 Compare March 5, 2024 08:05
@totoroot totoroot requested a review from domenkozar March 6, 2024 14:29
@domenkozar
Copy link
Member

@totoroot ping :)

@domenkozar
Copy link
Member

Can you rebase on main and I'll take over this one :)

@totoroot
Copy link
Collaborator Author

Can you rebase on main and I'll take over this one :)

@domenkozar Sorry, I did not have time to finish this up. Had a lot to do at work since last week, so I'm happy to hand it over to you :)
I have rebased onto main as requested. I hope that all conflicts were properly resolved.

@domenkozar domenkozar merged commit 8c97d19 into cachix:main Mar 22, 2024
213 of 239 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.

None yet

2 participants