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

stop recommending --no-cache-dir be used with pip #653

Merged
merged 2 commits into from Oct 8, 2018

Conversation

jjhelmus
Copy link
Contributor

@jjhelmus jjhelmus commented Oct 8, 2018

The --no-cache-dir argument supressed wheel building when used with pip. This
results in egg-info rather than .dist-info metadata directories being created.
A local cache directory is provided by conda-build since 3.14.0 which avoid any
issues with the global cache directory.

The --no-cache-dir argument supressed wheel building when used with pip.  This
results in egg-info rather than .dist-info metadata directories being created.
A local cache directory is provided by conda-build since 3.14.0 which avoid any
issues with the global cache directory.
@jjhelmus
Copy link
Contributor Author

jjhelmus commented Oct 8, 2018

See conda/conda-build#3094 for additional discussion on why --no-cache-dir is not a good default.

@@ -356,7 +356,7 @@ Normally Python packages should use this line:
.. code-block:: yaml

build:
script: "{{ PYTHON }} -m pip install . --no-deps --ignore-installed --no-cache-dir -vvv"
script: "{{ PYTHON }} -m pip install . --no-deps --ignore-installed -vvv"
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove --no-deps and --ignore-installed as well? conda-build sets env variables that does the same thing right?

Copy link
Member

Choose a reason for hiding this comment

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

Even though I prefer the explicitness of the having the options in there, since they are already in conda-build (and my guess is that they won't remove/change them) I'm +0.5.

@scopatz
Copy link
Member

scopatz commented Oct 8, 2018

@jjhelmus - does there need to be something in to conda smithy rerender for this?

@ocefpaf
Copy link
Member

ocefpaf commented Oct 8, 2018

@jjhelmus - does there need to be something in to conda smithy rerender for this?

Looks like we could use it in an automated way. Not sure if conda-smithy rerender is the best place for it though. The re-rendering works mostly with the CI boilerplate. We can/should lint, and maybe create a bot to fix the current recipes.

@ocefpaf
Copy link
Member

ocefpaf commented Oct 8, 2018

Oh. And we should fix conda-skeleton ASAP. I'll look into it.

Done in conda/conda-build@85d3a97#diff-c61365ff02198f8397cfa6e399b6ef8b

@jjhelmus
Copy link
Contributor Author

jjhelmus commented Oct 8, 2018

Rerendering could try to replace this. The old install command is not "wrong" and there are edge cases where a non-standard install command is needed. I'm hesitant to recommend that the bot do this automatically or the linter flag other incantation.

@ocefpaf ocefpaf merged commit 7dcce85 into conda-forge:master Oct 8, 2018
@ocefpaf
Copy link
Member

ocefpaf commented Oct 8, 2018

Rerendering could try to replace this. The old install command is not "wrong" and there are edge cases where a non-standard install command is needed. I'm hesitant to recommend that the bot do this automatically or the linter flag other incantation.

The bot re-renders with conda-smithy so your hesitation applies there too 😄

Are you recommending to leave as-is and change it when a problem happen?

jakirkham pushed a commit to jakirkham/conda-forge.github.io that referenced this pull request Oct 21, 2018
Fixes up the docs to include an intended change from another PR that was
lost (as it changed autogenerated files).

xref: conda-forge#653
@@ -356,7 +356,7 @@ Normally Python packages should use this line:
.. code-block:: yaml

build:
script: "{{ PYTHON }} -m pip install . --no-deps --ignore-installed --no-cache-dir -vvv"
script: "{{ PYTHON }} -m pip install . -vvv"
Copy link
Member

Choose a reason for hiding this comment

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

This is unfortunately an autogenerated file and not the source itself. So this didn't show up in the docs. Fixing with PR ( #659 ).

@djsutherland
Copy link
Contributor

So, following the advice in this PR, I removed --no-deps in a recipe...and it went ahead and installed deps I didn't want. Example build, using pip 18.1-py27_1000, conda-build 3.16.2-py36_0. Any ideas?

@jakirkham
Copy link
Member

I don't think it is installing them. I think it is caching them.

If that is what it is doing, then that is expected as conda-build does enable caching to avoid some strange behavior by pip, but does disables dependencies. More details of the environment variables conda-build sets for pip in these lines.

Note: pip uses rather strange logic with its environment variables. So this confuses things unfortunately. ( pypa/pip#5735 )

@djsutherland
Copy link
Contributor

Oh, I see; I noticed the installation and the egg_info but not the Removed afterwards. But it seems to stop doing that after I added --no-deps, and downloading them is a waste of CI time, though probably usually not very long....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants