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

Automatically create missing package repository dir #623

Conversation

mottosso
Copy link
Contributor

$ rez build --install
09:34:22 ERROR    PackageRepositoryError: Package repository path does not exist: C:\Users\marcus\packages
$ rez pip --install six
09:34:22 ERROR    PackageRepositoryError: Package repository path does not exist: C:\Users\marcus\packages

As choosing a destination directory is either default to users home directory, or explicitly chosen as a config option, installing to a non-existing directory shouldn't throw an error.

This should also help users new to Rez with the rez bind --quickstart option which currently throws an exception at the user right after install.

As choosing a destination directory is either default to users home directory, or explicitly chosen as a config option, installing to a non-existing directory shouldn't throw an error.
@mottosso mottosso force-pushed the feature/auto-create-package-repository-dir branch from f787c34 to 99f9413 Compare April 29, 2019 09:07
@mottosso
Copy link
Contributor Author

Could we merge this? This is a critical feature for any new user, who will not have this directory present on his machine. An error is currently the first encounter with anyone looking at Rez, and that's not good.

@nerdvegas
Copy link
Contributor

I'm considering it, but consider:

  • the existing error message is clear, and
  • attempting to create the dir could fail anyway (it may not be a local packages dir), and
  • if it isn't a local packages dir, then creating it is making an assumption about what the owner and permissions of the created directory should be. This isn't great, as someone may create a shared package repo dir, that another user may then not have write access to. Rez doesn't really have enough info to know what attributes the package repo dir should have.

If this functionality existed only for local package dirs, I think it would be more acceptable. What are your thoughts?

@mottosso
Copy link
Contributor Author

If I got to pick, I would let Rez try, and give the user an error only as a last-resort.

@instinct-vfx
Copy link
Contributor

For me i'd say restrict to local folders, then error on fail.

@nerdvegas
Copy link
Contributor

I'll merge as-is, I don't feel strongly enough about it and I don't think it's worth the extra complication of checking for local dir. Mind you, the error needs changing (it swallows the existing OSError, which could give useful info). I'll adjust and release.

@nerdvegas nerdvegas merged commit 99f9413 into AcademySoftwareFoundation:master Jun 18, 2019
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.

3 participants