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

Incorrect bundler updates after #5581 #5892

Closed
1 task done
CvX opened this issue Oct 16, 2022 · 7 comments · Fixed by #5901
Closed
1 task done

Incorrect bundler updates after #5581 #5892

CvX opened this issue Oct 16, 2022 · 7 comments · Fixed by #5901
Labels
T: bug 🐞 Something isn't working

Comments

@CvX
Copy link

CvX commented Oct 16, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Package ecosystem

bundler

Package manager version

2.3.5

Language version

2.7.6

Manifest location and content before the Dependabot update

/Gemfile

excerpt:

gem 'oj', '3.13.14'
gem 'sassc', '2.0.1', require: false
gem 'active_model_serializers', '~> 0.8.3'

dependabot.yml content

excerpt:

- package-ecosystem: bundler
  versioning-strategy: lockfile-only
  allow:
    - dependency-type: direct
    - dependency-type: indirect

Updated dependency

sassc updated from the locked version (2.0.1) to 2.4.0 discourse/discourse#18617
oj updated from the locked version (3.13.14) to 3.13.21 discourse/discourse#18614
active_model_serializers updated from the locked version (~> 0.8.3 that resolved to 0.8.4) to 0.10.13 discourse/discourse#18610

What you expected to see, versus what you actually saw

Before that PR (#5581) Dependabot was correctly ignoring those dependencies. The PRs it now creates update the lockfile versions agains what's in the Gemfile.

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

@CvX CvX added the T: bug 🐞 Something isn't working label Oct 16, 2022
@deivid-rodriguez
Copy link
Contributor

Hi @CvX!

I'm sorry that Dependabot started proposing updates that you didn't want 🙏.

Could you explain a little bit in more detail what you want Dependabot to do? From the PRs you linked to, Dependabot seems to be doing the right thing to me. It's updating only the Gemfile.lock file, as requested by the lockfile-only versioning strategy.

Since you are mentioning that the updated dependencies are explicit in the Gemfile, and you mention that you want these dependencies ignored, that suggests that you want Dependabot to ignore indirect dependencies. However, that's in contraction with your configuration of

allow:
  - dependency-type: indirect

Maybe the solution to get what you want is to remove that line from your configuration?

@deivid-rodriguez
Copy link
Contributor

Oh, actually, the one bumping active_model_serializers is definitely a bug, let me look into that!

@andreas-venturini
Copy link

@deivid-rodriguez all three are bugs

It's updating only the Gemfile.lock file, as requested by the lockfile-only versioning strategy.

It should not update the Gemfile.lock for oj and sassc because both are locked to a specific version - so there is nothing to update in the Gemfile.lock

@CvX
Copy link
Author

CvX commented Oct 17, 2022

From the PRs you linked to, Dependabot seems to be doing the right thing to me. It's updating only the Gemfile.lock file, as requested by the lockfile-only versioning strategy.

Dependabot documentation says about lockfile-only option: "Only create pull requests to update lockfiles. Ignore any new versions that would require package manifest changes."

As I understand it, and how it worked before, is that should update the lockfile only if the update does not contradict the requirements stated in the Gemfile. That's the behavior that regressed.

Since you are mentioning that the updated dependencies are explicit in the Gemfile, and you mention that you want these dependencies ignored, that suggests that you want Dependabot to ignore indirect dependencies.

Those are direct dependencies. And we don't want them completely ignored, like in the active_model_serializers case where the version is specified as ~> 0.8.3 which means we'd want to be informed if there are any new versions in 0.8.x line.

(GitHub should alert when you're about to post a comment and there are new replies 😅)

@deivid-rodriguez
Copy link
Contributor

@CvX I'm sorry, I completely missed the full pin on those requirements. You're 100% right, my bad!

I'm looking into this right now, and will fix this ASAP.

@deivid-rodriguez
Copy link
Contributor

This has been now reverted, I may ask you to do some testing before retying another fix, if you don't mind.

@CvX
Copy link
Author

CvX commented Oct 17, 2022

Sure thing!

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

Successfully merging a pull request may close this issue.

3 participants