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

Set default storage folder to $CONAN_USER_HOME/.conan/data #7910

Merged
merged 2 commits into from Oct 29, 2020

Conversation

fpoliakov
Copy link
Contributor

@fpoliakov fpoliakov commented Oct 20, 2020

Changelog: Bugfix: Set default storage_folder to .conan/data in case if storage_path entry fails to be defined by conan.conf.
Docs: Omit

Set default storage_folder to .conan/data in case if storage/path entry is missing from the conan.conf. Usage of .conan for storage root seems to be a mistake and leads to failures in case of various operations, like conan config install <some_relatively_deep_git_repo>

We've noticed that sometimes when doing conan install from a git repo, we're getting an error like:

conan config install <sample-repo>
Trying to clone repo: <sample-repo>
Repo cloned!
Defining remotes from remotes.txt
ERROR: Failed conan config install: Value provided for package version, '.git' (type Version), is an invalid name. Valid names MUST begin with a letter, number or underscore, have between 2-51 chars, including letters, numbers, underscore, dot and dash

It turned out that, if [storage] / path entry in conan.conf is missing, the default storage path would be set to $CONAN_USER_HOME/.conan ; which looks to be incorrect. Default per here seems to be $CONAN_USER_HOME/.conan/data .
Later during conan config install command execution, conan tries to traverse all packages to fix the remotes, but given that it clones the config repo to the temporary directory within .conan dir, it tries to read packages in the temporarily cloned repo and fails with the error above.
Adding "data" at the end of the path fixes the error.

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@CLAassistant
Copy link

CLAassistant commented Oct 20, 2020

CLA assistant check
All committers have signed the CLA.

@fpoliakov fpoliakov changed the title Set default cache_folder to $CONAN_USER_HOME/.conan/data Set default storage folder to $CONAN_USER_HOME/.conan/data Oct 20, 2020
@fpoliakov fpoliakov marked this pull request as draft Oct 20, 2020
@memsharded memsharded self-assigned this Oct 20, 2020
@memsharded
Copy link
Member

Hi @Sfairat

Thanks for contributing this. I think it would be necessary to have a red/green test for this fix, so we make sure that this is both fixed correctly and we don't break it in the future. Please have a look to the test suite, if the bug is fired by conan config install then config_install_test.py could be a good starting point. Don't hesitate to ask for guidance or help, we could contribute the tests ourselves too.

@fpoliakov
Copy link
Contributor Author

@memsharded Thanks for the feedback - I was convinced that tests are necessary here. Will add later this week.

@memsharded
Copy link
Member

@memsharded Thanks for the feedback - I was convinced that tests are necessary here. Will add later this week.

Excellent. Conan 1.31 is planned for next week, if we could have the tests this week, this could be included. Adding it to the milestone. Thanks again!

@memsharded memsharded added this to the 1.31 milestone Oct 20, 2020
@memsharded
Copy link
Member

Hi @Sfairat

We are doing 1.31 this week, could you please add a test for this fix? If not possible, we might try to contribute the test, please tell. Thanks!

@memsharded memsharded marked this pull request as ready for review Oct 29, 2020
@memsharded memsharded merged commit fabef25 into conan-io:develop Oct 29, 2020
2 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

3 participants