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

Dependabot fails to update same dependency in same pull request #6200

Closed
1 task done
glensc opened this issue Nov 23, 2022 · 18 comments
Closed
1 task done

Dependabot fails to update same dependency in same pull request #6200

glensc opened this issue Nov 23, 2022 · 18 comments
Labels
L: python:pipenv Python packages via pipenv T: bug 🐞 Something isn't working

Comments

@glensc
Copy link

glensc commented Nov 23, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Package ecosystem

pip, pipenv

Package manager version

No response

Language version

No response

Manifest location and content before the Dependabot update

No response

dependabot.yml content

No response

Updated dependency

No response

What you expected to see, versus what you actually saw

Dependabot created two PR's for same dependency update:

it used to manage this nicely and update both files in same pull request. sometimes it fails, i have not yet determined the pattern.

Native package manager behavior

No response

Images of the diff or a link to the PR, issue, or logs

No response

Smallest manifest that reproduces the issue

No response

@deivid-rodriguez
Copy link
Contributor

Looking at your dependency files, I think there's two issues here:

  • In presence of Pipfile and Pipfile.lock file, a requirements.txt file should be considered just as another lock file, and don't get updated directly, but just be updated to follow Pipfile.lock changes. So no PRs should be proposed for you unless you configure allow: [{ dependency-type: "indirect" }] in your dependabot.yml file.

  • If allow: [{ dependency-type: "indirect" }] was configured, you should indeed get just one PR with both updates included, not two.

@glensc
Copy link
Author

glensc commented Nov 23, 2022

@deivid-rodriguez so, if I got you correctly, do I need to do this?:

@deivid-rodriguez
Copy link
Contributor

I think that's how it should work, yes. By default you should only get updates for dependencies explicitly defined in your Pipfile.

But actually, you probably want dependency-type: all since otherwise you're opting out of direct dependencies.

But in any case, I don't think it's going to make a difference right now due to this bug, but feel free to leave feedback on whether the change had any effect on your repo.

@glensc
Copy link
Author

glensc commented Nov 23, 2022

i'll go with all then. thanks

@glensc
Copy link
Author

glensc commented Nov 27, 2022

I've merged that, but now it still updated only one file, requirements.txt this time:

@glensc
Copy link
Author

glensc commented Nov 27, 2022

oh, wow, it created the second PR already so soon:

@pavera
Copy link
Contributor

pavera commented Dec 5, 2022

I started looking at this in the debugger and it looks like it's possibly an issue in the resolver_type method which basically picks which package manager we think is being used. In my tests it keeps picking plain pip based on the fact that it is finding a possible update in requirements.txt before finding the Pipfile update.

I'll keep pulling on this thread and see if I can come up with a solution.

@jeffwidman
Copy link
Member

jeffwidman commented Dec 5, 2022

This is not the first time we've run into problems with trying to guess which package manager is being used within an ecosystem. From a product perspective IMO we should reconsider if it might be more straightforward for us to ask users to specify the specific manager for any ecosystem with more than one possible package manager. We probably could pick a default for every ecosystem. This would also pet users specify more than one package manager if they wanted to track updates multiple paths. Anyway, a discussion beyond the scope of this particular issue but wanted to mention as a recurring thread I've seen.

(the other recurring thread I've seen is the confusion resulting from python treating requirements.txt as both a manifest and a lockfile. Life would be simpler if they had an officially-blessed lockfile that all package managers generate towards... I wonder if anyone has ever submitted a PEP for that?)

@glensc
Copy link
Author

glensc commented Dec 7, 2022

@pavera
Copy link
Contributor

pavera commented Dec 8, 2022

Yeah, our logic here is a little fuzzy and doesn't really work for transitive dependencies. Basically since prompt-toolkit is a transitive dependency, we don't see that it needs to be updated in the pipenv environment because it is not in the Pipfile. When we're detecting which package manager we're operating as we only consider the "manifest" files currently, which we classify requirements.txt as a manifest file. Since the dependency doesn't occur in Pipfile we choose pip instead of pipenv and perform the update only on requirements.txt instead of choosing pipenv which would then update both.

I'm attempting to improve this logic to prefer pipenv over pip in the presence of a Pipfile even if the dependency doesn't occur in said Pipfile. Hopefully will have a PR in the next day or so. Basically the current assumption is if there is a Pipfile and a requirements.txt the requirements.txt is probably managed directly with pip and not generated from the Pipfile. My approach is going to be to change this assumption to be if there are both files in the repository, assume the requirements.txt is generated from pipenv until shown otherwise.

@glensc
Copy link
Author

glensc commented Dec 8, 2022

ok, so for me workaround is to just add all dependencies to both files. the requrements.txt is just a dump of pipfile.lock anyway in that project.

EDIT: Added:

pipenv install prompt-toolkit==3.0.33

glensc added a commit to Taxel/PlexTraktSync that referenced this issue Dec 8, 2022
@glensc
Copy link
Author

glensc commented Dec 8, 2022

And now there's another problem: It updates other unrelated dependencies as well:

The PR is about "Bump certifi from 2022.9.24 to 2022.12.7", which it happens to update Pipfile, Pipfile.lock, requirements.txt. so far so good.

but it also updates prompt-toolkit==3.0.36; python_full_version >= '3.6.2', which there was prior PR already:

@pavera
Copy link
Contributor

pavera commented Dec 8, 2022

I'm not seeing the prompt-toolkit update in that PR? In requirements.txt it just seems to be removing the python_full_version from the prompt-toolkit line not changing the version? So it seems this is just another example of your issue #6091? Or am I missing another update to prompt-toolkit in that PR?

@glensc
Copy link
Author

glensc commented Dec 8, 2022

it was in the previous push:

@dependabot dependabot bot force-pushed the dependabot/pip/certifi-2022.12.7 branch from 36243f3 to 8e5462c 1 hour ago

So see this commit:

@glensc
Copy link
Author

glensc commented Dec 21, 2022

How do I add items with markers to Pipfile?

i.e requirements has:

attrs==22.1.0; python_version >= '3.5'

I'm trying to work around that dependabot updates Pipenv files as well files in this PR:

@glensc
Copy link
Author

glensc commented Dec 21, 2022

Since there was already marker in Pipfile.lock:

I only added line to Pipfile without markers

glensc added a commit to Taxel/PlexTraktSync that referenced this issue Jan 2, 2023
glensc added a commit to Taxel/PlexTraktSync that referenced this issue Jan 12, 2023
glensc added a commit to Taxel/PlexTraktSync that referenced this issue Jan 13, 2023
glensc added a commit to Taxel/PlexTraktSync that referenced this issue Jan 20, 2023
halostatue added a commit to KineticCafe/docker-ansible that referenced this issue Jul 15, 2023
This needed to be created because Dependabot only updates
`requirements.txt` because of dependabot/dependabot-core#6200.

Closes #22
github-merge-queue bot pushed a commit to KineticCafe/docker-ansible that referenced this issue Jul 17, 2023
This needed to be created because Dependabot only updates
`requirements.txt` because of dependabot/dependabot-core#6200.

Closes #22
halostatue added a commit to KineticCafe/docker-ansible that referenced this issue Aug 4, 2023
- Upgraded to Ansible 8.2.0

- Changed from [pipenv][] to [pdm][] and `pyproject.toml` because of
  ongoing issues with Dependabot not detecting dependencies in
  `Pipfile.lock` as opposed to only in `requirements.txt` (the inverse
  of dependabot/dependabot-core#6200). After trying Poetry (predates the
  latest Python packaging PEPs), rye (experimental), and hatch (no lock
  file), [pdm][] seems to fit the bill best for the limited needs that
  we have with this project.

- Experimentally removed the generated `requirements.txt` file. It isn't
  used, but it has been valuable in getting update notifications from
  Dependabot, although the update PRs are less useful. If required, we
  can add it back with `pdm export -f requirements > requirements.txt`.
halostatue added a commit to KineticCafe/docker-ansible that referenced this issue Aug 4, 2023
- Upgraded to Ansible 8.2.0

- Changed from [pipenv][] to [pdm][] and `pyproject.toml` because of
  ongoing issues with Dependabot not detecting dependencies in
  `Pipfile.lock` as opposed to only in `requirements.txt` (the inverse
  of dependabot/dependabot-core#6200). After trying Poetry (predates the
  latest Python packaging PEPs), rye (experimental), and hatch (no lock
  file), [pdm][] seems to fit the bill best for the limited needs that
  we have with this project.

- Experimentally removed the generated `requirements.txt` file. It isn't
  used, but it has been valuable in getting update notifications from
  Dependabot, although the update PRs are less useful. If required, we
  can add it back with `pdm export -f requirements > requirements.txt`.
github-merge-queue bot pushed a commit to KineticCafe/docker-ansible that referenced this issue Aug 4, 2023
- Upgraded to Ansible 8.2.0

- Changed from [pipenv][] to [pdm][] and `pyproject.toml` because of
  ongoing issues with Dependabot not detecting dependencies in
  `Pipfile.lock` as opposed to only in `requirements.txt` (the inverse
  of dependabot/dependabot-core#6200). After trying Poetry (predates the
  latest Python packaging PEPs), rye (experimental), and hatch (no lock
  file), [pdm][] seems to fit the bill best for the limited needs that
  we have with this project.

- Experimentally removed the generated `requirements.txt` file. It isn't
  used, but it has been valuable in getting update notifications from
  Dependabot, although the update PRs are less useful. If required, we
  can add it back with `pdm export -f requirements > requirements.txt`.
@abdulapopoola
Copy link
Member

Closing out as stale

@glensc
Copy link
Author

glensc commented Jun 11, 2024

Some kind of problem still there. Need to add all packages to Pipenv to avoid some buggy behavior:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: python:pipenv Python packages via pipenv T: bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants