Skip to content

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

Merged
memsharded merged 2 commits into
conan-io:developfrom
fplk0:patch-1
Oct 29, 2020
Merged

memsharded merged 2 commits into
conan-io:developfrom
fplk0:patch-1

Conversation

@fplk0

@fplk0 fplk0 commented Oct 20, 2020

Copy link
Copy Markdown
Contributor

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

CLAassistant commented Oct 20, 2020

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@fplk0 fplk0 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
@fplk0
fplk0 marked this pull request as draft October 20, 2020 06:08
@memsharded memsharded self-assigned this Oct 20, 2020
@memsharded

Copy link
Copy Markdown
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.

@fplk0

fplk0 commented Oct 20, 2020

Copy link
Copy Markdown
Contributor Author

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

@memsharded

Copy link
Copy Markdown
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
Copy Markdown
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 October 29, 2020 16:31
@memsharded
memsharded merged commit fabef25 into conan-io:develop Oct 29, 2020
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