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

Assume that vcs and direct URL installs need to be reinstalled #5936

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

matteius
Copy link
Member

Thank you for contributing to Pipenv!

The issue

Fixes issue #5791

The fix

Since skip-lock is still in play, and the existing method is operating off only the install req, we cannot with certainty know if the remote requirement has been updated, so queue it for install by pip.

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

@matteius matteius marked this pull request as ready for review September 17, 2023 10:10
@matteius matteius requested a review from oz123 September 17, 2023 10:10
@oz123
Copy link
Contributor

oz123 commented Sep 24, 2023

@matteius The change is OK. But does it really fix the issue? Reading the issue (briefly) it seems there is still work going on? Can this be merged?

@fmssn
Copy link
Contributor

fmssn commented Sep 26, 2023

Possibly

return(
    if req.link is not None and
    ...
)

Would also be ok. The change is pretty important, e.g. it atm crashes the current prefect installation at python 3.11.

@matteius
Copy link
Member Author

@fmssn Sorry but I don't understand what you mean by its crashing on a perfectly good install of 3.11 or how this change could affect that.

@oz123 Yeah I am not sure yet why this change wouldn't have solved their issue but probably needs more discovery and possibly this change isn't quite it yet.

@fmssn
Copy link
Contributor

fmssn commented Sep 27, 2023

@matteius

For reproduction, you can open a dev container with e.g. "prefecthq/prefect:2.13-python3.11".

With Pipfile

[[source]]
url = "https://pypi.python.org/simple"
verify_ssl = true
name = "pypi"

[packages]
prefect = "*"

[dev-packages]
pytest = "*"

Executing

pipenv lock 
pipenv install --system

yields
AttributeError: 'NoneType' object has no attribute 'scheme'

This is because req.link is optional and of Type InstallRequirement, with the default being None. Therefore accessing the attribute scheme if it is None raises an Attribute Error.

@matteius matteius changed the title Assume the vcs and direct URL installs need to be reinstalled Assume that vcs and direct URL installs need to be reinstalled Nov 14, 2023
@matteius matteius merged commit 55258f7 into main Nov 14, 2023
1 check passed
@matteius matteius deleted the issue-5791 branch November 14, 2023 23:01
achim-k pushed a commit to foxglove/ws-protocol that referenced this pull request Nov 20, 2023
Bumps [pipenv](https://github.com/pypa/pipenv) from 2023.10.24 to
2023.11.15.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/pypa/pipenv/releases">pipenv's
releases</a>.</em></p>
<blockquote>
<h2>Release v2023.11.15</h2>
<h2>What's Changed</h2>
<ul>
<li>Treat all return paths of this method as strings by <a
href="https://github.com/matteius"><code>@​matteius</code></a> in <a
href="https://redirect.github.com/pypa/pipenv/pull/6017">pypa/pipenv#6017</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/pypa/pipenv/compare/v2023.11.14...v2023.11.15">https://github.com/pypa/pipenv/compare/v2023.11.14...v2023.11.15</a></p>
<h2>Release v2023.11.14</h2>
<h2>What's Changed</h2>
<ul>
<li>Restore this code that should prevent the string has no attribute
update bug. by <a
href="https://github.com/matteius"><code>@​matteius</code></a> in <a
href="https://redirect.github.com/pypa/pipenv/pull/6007">pypa/pipenv#6007</a></li>
<li>Pass through pipfile index urls when creating https session so that
keyring fully works by <a
href="https://github.com/mungojam"><code>@​mungojam</code></a> in <a
href="https://redirect.github.com/pypa/pipenv/pull/5994">pypa/pipenv#5994</a></li>
<li>Fix issue-6011 direct file url path by <a
href="https://github.com/matteius"><code>@​matteius</code></a> in <a
href="https://redirect.github.com/pypa/pipenv/pull/6012">pypa/pipenv#6012</a></li>
<li>Ignore existing venv dir when PIPENV_VENV_IN_PROJECT is false by <a
href="https://github.com/arnaud-dezandee"><code>@​arnaud-dezandee</code></a>
in <a
href="https://redirect.github.com/pypa/pipenv/pull/6009">pypa/pipenv#6009</a></li>
<li>Assume that vcs and direct URL installs need to be reinstalled by <a
href="https://github.com/matteius"><code>@​matteius</code></a> in <a
href="https://redirect.github.com/pypa/pipenv/pull/5936">pypa/pipenv#5936</a></li>
</ul>
<h2>New Contributors</h2>
<ul>
<li><a
href="https://github.com/arnaud-dezandee"><code>@​arnaud-dezandee</code></a>
made their first contribution in <a
href="https://redirect.github.com/pypa/pipenv/pull/6009">pypa/pipenv#6009</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a
href="https://github.com/pypa/pipenv/compare/v2023.10.24...v2023.11.14">https://github.com/pypa/pipenv/compare/v2023.10.24...v2023.11.14</a></p>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/pypa/pipenv/blob/main/CHANGELOG.md">pipenv's
changelog</a>.</em></p>
<blockquote>
<h1>2023.11.15 (2023-11-15)</h1>
<h1>Pipenv 2023.11.15 (2023-11-15)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Fix regression with path installs on most recent release
<code>2023.11.14</code>
<code>[#6017](pypa/pipenv#6017)
&lt;https://github.com/pypa/pipenv/issues/6017&gt;</code>_</li>
</ul>
<h1>2023.11.14 (2023-11-14)</h1>
<h1>Pipenv 2023.11.14 (2023-11-14)</h1>
<h2>Behavior Changes</h2>
<ul>
<li>pipenv now ignores existing venv dir when
<code>PIPENV_VENV_IN_PROJECT</code> is false.
<code>[#6009](pypa/pipenv#6009)
&lt;https://github.com/pypa/pipenv/issues/6009&gt;</code>_</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Assume the vcs and direct URL installs need to be reinstalled.
<code>[#5936](pypa/pipenv#5936)
&lt;https://github.com/pypa/pipenv/issues/5936&gt;</code>_</li>
<li>Pass through pipfile index urls when creating https session so that
keyring fully works
<code>[#5994](pypa/pipenv#5994)
&lt;https://github.com/pypa/pipenv/issues/5994&gt;</code>_</li>
<li>Fix Using dependencies from a URL fails on Windows.
<code>[#6011](pypa/pipenv#6011)
&lt;https://github.com/pypa/pipenv/issues/6011&gt;</code>_</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/pypa/pipenv/commit/7ec8fee4f261bf27a655e926df34ea63c2c3ea0d"><code>7ec8fee</code></a>
Release v2023.11.15</li>
<li><a
href="https://github.com/pypa/pipenv/commit/85ba107e5b63a2543ed3a406dff2f31bdeb32e30"><code>85ba107</code></a>
Treat all return paths of this method as strings (<a
href="https://redirect.github.com/pypa/pipenv/issues/6017">#6017</a>)</li>
<li><a
href="https://github.com/pypa/pipenv/commit/acbcdcc14273ad55f31e0cd14309f0a8d3f5470b"><code>acbcdcc</code></a>
Bumped version.</li>
<li><a
href="https://github.com/pypa/pipenv/commit/ff5cbbb3484de90744be2bee0bc8fc4af836d95c"><code>ff5cbbb</code></a>
Release v2023.11.14</li>
<li><a
href="https://github.com/pypa/pipenv/commit/55258f71c78026a0bd02c0bf389f0b4d7d9728f1"><code>55258f7</code></a>
Assume that vcs and direct URL installs need to be reinstalled (<a
href="https://redirect.github.com/pypa/pipenv/issues/5936">#5936</a>)</li>
<li><a
href="https://github.com/pypa/pipenv/commit/9eb07b443944b64c83c3f0a1dbbdcfecadea4979"><code>9eb07b4</code></a>
add news fragment on behalf of contributor</li>
<li><a
href="https://github.com/pypa/pipenv/commit/bb0ca90fecf11e1189d6e20c3276f4526c4331e6"><code>bb0ca90</code></a>
Ignore existing venv dir when PIPENV_VENV_IN_PROJECT is false (<a
href="https://redirect.github.com/pypa/pipenv/issues/6009">#6009</a>)</li>
<li><a
href="https://github.com/pypa/pipenv/commit/cd961a41b87f00d1b0923bc8ee87861adb88b468"><code>cd961a4</code></a>
Fix issue-6011 direct file url path (<a
href="https://redirect.github.com/pypa/pipenv/issues/6012">#6012</a>)</li>
<li><a
href="https://github.com/pypa/pipenv/commit/bc668e64755079abd9e154cacace98bce88a59c3"><code>bc668e6</code></a>
Merge pull request <a
href="https://redirect.github.com/pypa/pipenv/issues/5994">#5994</a>
from mungojam/fix-keyring-warnings</li>
<li><a
href="https://github.com/pypa/pipenv/commit/023b6bccf7b71b13449ef9420255d1bf81bc710a"><code>023b6bc</code></a>
Add news entry</li>
<li>Additional commits viewable in <a
href="https://github.com/pypa/pipenv/compare/v2023.10.24...v2023.11.15">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pipenv&package-manager=pip&previous-version=2023.10.24&new-version=2023.11.15)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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