Skip to content

Commit

Permalink
Add name key to sources
Browse files Browse the repository at this point in the history
Newer versions of `pipenv` require any specified `sources` to be
explicitly `name`'d.

More context here: pypa/pipenv#5370 (comment)

This has been true since at least Sept 2022. Our version of `pipenv` was
from April of 2022, so it didn't complain. But that means most of our
active users of `pipenv` are unlikely to see this error if they're
running a newer version of `pipenv`.

Also, for sources that :dependabot: dynamically injects into the
`Pipfile`, they need a name. These sources are stripped from the final
`Pipfile` / `Pipfile.lock` during the `FileUpdater#post_process_lockfile`
method, so all we need is a placeholder `name` to placate `pipenv`.

Long term, we may want to add custom error handling to flag this
missing key as a `Dependabot::DependencyFileNotResolvable` error.

But I decided that was out of scope for now as this PR does not generate
the error... that will not happen until we upgrade to newer `pipenv`.
And even at that point, our first priority will be upgrading, and then
from there handling any new errors that start popping up.
  • Loading branch information
jeffwidman committed Aug 10, 2023
1 parent 221897f commit 62f4d33
Show file tree
Hide file tree
Showing 34 changed files with 47 additions and 14 deletions.
12 changes: 9 additions & 3 deletions python/lib/dependabot/python/file_updater/pipfile_preparer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,15 @@ def sub_auth_url(source, credentials)

def config_variable_sources(credentials)
@config_variable_sources ||=
credentials.
select { |cred| cred["type"] == "python_index" }.
map { |c| { "url" => AuthedUrlBuilder.authed_url(credential: c) } }
credentials.select { |cred| cred["type"] == "python_index" }.
map do |c|
{
"url" => AuthedUrlBuilder.authed_url(credential: c),
# Random suffix ensure names are unique. These are stripped out of the final Pipfile.lock in the
# FileUpdater step so it's okay that they're non-deterministic.
"name" => "dependabot-inserted-index-#{SecureRandom.alphanumeric(5)}"
}
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@
let(:lockfile_fixture_name) { "version_not_specified.lock" }

it "adds the source" do
expect(updated_content).
to include("https://username:password@pypi.posrip.com/pypi/")
expect(updated_content).to include('name = "dependabot-inserted-index-')
expect(updated_content).to include("https://username:password@pypi.posrip.com/pypi/")
end

context "with auth details provided as a token" do
Expand All @@ -154,8 +154,8 @@
end

it "adds the source" do
expect(updated_content).
to include("https://username:password@pypi.posrip.com/pypi/")
expect(updated_content).to include('name = "dependabot-inserted-index-')
expect(updated_content).to include("https://username:password@pypi.posrip.com/pypi/")
end
end

Expand All @@ -178,7 +178,7 @@
it "keeps source config" do
expect(updated_content).to include(
"[[source]]\n" \
"name = \"pypi\"\n" \
"name = \"internal-pypi\"\n" \
"url = \"https://username:password@pypi.posrip.com/pypi/\"\n" \
"verify_ssl = true\n"
)
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/arbitrary_equality
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/conflict_at_current
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/conflict_at_latest
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/${ENV_VAR}"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/exact_version
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
2 changes: 1 addition & 1 deletion python/spec/fixtures/pipfile_files/extra_subdependency
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
flask = "==1.0.*"
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source_bad_ref
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source_no_ref
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/git_source_unreachable
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/hard_names
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/not_in_lockfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/only_dev
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/path_dependency
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/private_source
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "internal-pypi"
url = "https://some.internal.registry.com/pypi/"
verify_ssl = true

Expand Down
2 changes: 1 addition & 1 deletion python/spec/fixtures/pipfile_files/private_source_auth
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[[source]]
name = "internal-pypi"
url = "https://${ENV_USER}:${ENV_PASSWORD}@pypi.posrip.com/pypi/"
verify_ssl = true
name = "pypi"

[dev-packages]
pytest = "==3.4.0"
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/prod_and_dev
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/prod_and_dev_different
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/required_python
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/required_python_invalid
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
3 changes: 1 addition & 2 deletions python/spec/fixtures/pipfile_files/unparseable
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
[[source]]

name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[dev-packages]

Expand Down
2 changes: 1 addition & 1 deletion python/spec/fixtures/pipfile_files/unsupported_dep
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
requests = "==2.18.0"
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/version_hash
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/version_not_specified
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
{
"_meta": {
"hash": {
"sha256": "c402ea48092e9d467af51a483bb8dd8ad0620e11c94f009dcd433f97a99d45db"
"sha256": "e76ae491d793d659f05c7f7eab261fd6167dc062efcba08f17be68e73eb87665"
},
"pipfile-spec": 6,
"requires": {},
"sources": [
{
"name": "this-key-probably-will-show-up-if-regenerating-lockfiles",
"url": "https://pypi.org/simple",
"verify_ssl": true
}
Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/version_table
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/wildcard
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/with_quotes
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down
1 change: 1 addition & 0 deletions python/spec/fixtures/pipfile_files/yanked
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

Expand Down

0 comments on commit 62f4d33

Please sign in to comment.