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

Update dependabot to use flutter pub get properly #1643

Closed
literalEval opened this issue Mar 9, 2023 · 31 comments
Closed

Update dependabot to use flutter pub get properly #1643

literalEval opened this issue Mar 9, 2023 · 31 comments
Assignees
Labels
bug Something isn't working no-issue-activity No issue activity

Comments

@literalEval
Copy link
Member

literalEval commented Mar 9, 2023

Describe the bug
After our migration to Flutter 3.7.3, the new pub get command adds sha256 hash of each plugin to pubspec.lock. The problem is that dependabot somehow does not generate these hashes (this might mean that it is using an old version of flutter sdk ?), changing pubspec.lock entirely.

This has some issues as

  1. Contributors will have conflicts with pubspec.lock as their version of the file has sha256 hashes while the dependabot version has not.
  2. Next PR will need to fix pubspec.lock again, which is not good because they will need to work with files they are not supposed to work with.
  3. If the next PR doesn't fix pubspec.lock, then that's again an issue because then the flutter pub get in our workflow (which uses latest version of flutter) will complain about pubspec.lock being not in the format it expects.

To fix

Find out the internals of dependabot and which version of Dart SDK it uses and upgrade it to use the latest version.

Important Links

@literalEval literalEval added the bug Something isn't working label Mar 9, 2023
@github-actions github-actions bot added the unapproved Unapproved, needs to be triaged label Mar 9, 2023
@xoldd
Copy link

xoldd commented Mar 13, 2023

@literalEval Btw if dependabot is too much of a problem, there's an alternative too, it's called renovate. It's has much more features I've heard. But dependabot integrated into github so using it is easy.

@literalEval
Copy link
Member Author

@xoldyckk yeah that is the point. Dependabot comes prebuilt so our initial idea is to work it out. If it can not be made to work anyhow, then maybe we will think of other solutions.

@xoldd
Copy link

xoldd commented Mar 13, 2023

@xoldyckk yeah that is the point. Dependabot comes prebuilt so our initial idea is to work it out. If it can not be made to work anyhow, then maybe we will think of other solutions.

Renovate is more advanced I've heard, provides much more insights into changes, and it's more failproof. Check out this video to know more https://youtu.be/kW2yY4kSZhQ

@literalEval
Copy link
Member Author

@xoldyckk I discussed with @palisadoes to remove dependabot and use some alternative. But he insisted on making dependabot work somehow. Maybe he had something in mind.

@xoldd
Copy link

xoldd commented Mar 13, 2023

@xoldyckk I discussed with @palisadoes to remove dependabot and use some alternative. But he insisted on making dependabot work somehow. Maybe he had something in mind.

Probably the ease of integration with github.

@palisadoes
Copy link
Contributor

  1. This issue was created to see whether we could get dependabot to work as it is the GitHub default.
  2. We need a solution to keep the repository dependencies up to date.
  3. If dependabot cannot work, we can certainly find something else.

@literalEval
Copy link
Member Author

Point 3 is great. Sure we can look into renovate.

@SiddheshKukade
Copy link
Member

@palisadoes Sir, I'd like to get assigned to this issue.

@palisadoes palisadoes removed the unapproved Unapproved, needs to be triaged label Mar 16, 2023
@xoldd
Copy link

xoldd commented Mar 16, 2023

@literalEval Just wanted to let you know that the configuration for dependabot.yml for this repo was set up by me. I followed the official docs on it, but more atomic improvements could be made to the configuration according to your requirements. I just followed the most generic guide, because I'm not at all familiar with dart and flutter.

@SiddheshKukade
Copy link
Member

@xoldyckk Dependabot was not working even after enforcing the flutter version.
I've also tried to get feedback about this from the community and have posted a question on StackOverflow about dependabot.
But I haven't received any answers on that.
I got a reply from a person here dev-yakuza/dev-yakuza.github.io#79 (comment) who said that:

The Dart package manager uses https://pub.dev by default. However, GitHub Dependabot uses https://pub.dartlang.org by default.

So, I think there is no way to fit it, currently. If I find the solution, I will share it.

I tried renovate bot on one of my old flutter repository https://github.com/SiddheshKukade/sample-flutter/ to test it.
Seems like it only changes the yaml file and not the lock file.
Is this an expected behaviour according to you ?

image

I think we can implement this for now until the dependabot issues is fixed. As it only updates the pubspec.yaml file then pubspec.lock file might be update in the future PRs by any contributors by simply doing flutter pub get.

@literalEval
Copy link
Member Author

literalEval commented Mar 19, 2023

@SiddheshKukade this is not a complete solution as the lock file must be updated with each update in pubpsec.yaml.
Also, if we want to depend on the future PRs by contributors, then that's already happening anyway. The purpose of this issue is to fix the problem completely.

@SiddheshKukade
Copy link
Member

SiddheshKukade commented Mar 19, 2023

Yes, it's not but currently the dependabot is also not working properly.
As stated here It should work on the stable channel

And also from the code of dependabot repo it should check the versions from the environment property of the yaml file. But it's not and giving the wrong implementation in PRs: (you can check here)
https://github.com/dependabot/dependabot-core/blob/c72a2b6d06c653e19ce1c9ca29b9a32265024b3f/pub/helpers/bin/infer_sdk_versions.dart#L48-L58
image

using Renovate will at least help to keep up to date with versions.
I would also like to get @xoldyckk's opinion on this.

@literalEval
Copy link
Member Author

I think we should go with Renovate.

@xoldd
Copy link

xoldd commented Mar 19, 2023

I don't have any knowledge about these automatic dependency managers, most of the times they'd work without needing much manual configuration. Mostly it needs to abide by the lock file so that it doesn't accidently update something which can break the system(though that itself depends on whether the packages used in the project themselves are following semantic versioning and stuff). I can't answer technical questions related to pub package manager ,its lock file and how dependabot works with it. I've never used dart/flutter.

Though I've read the dart/flutter dependency management was recently made available with dependabot. So, there are bound to be problems with it I guess cuz it's just released a few months back.

But yeah I've heard great things about renovate and most big organizations and projects on github I've seen use renovate instead of dependabot for this stuff.

@palisadoes
Copy link
Contributor

palisadoes commented Mar 19, 2023

@SiddheshKukade We need a workable solution and the GitHub default isn't it. Please investigate the alternative.

I don't want the mobile app to fall behind in updates

@SiddheshKukade
Copy link
Member

Though I've read the dart/flutter dependency management was recently made available with dependabot. So, there are
bound to be problems with it I guess cuz it was just released a few months back.

@xoldyckk I was also thinking the same. 👍🏻


I don't want the mobile app to fall behind in updates

@palisadoes, Sure I'll let you know.

@SiddheshKukade
Copy link
Member

Some updates from the dependabot/dependabot-core maintainers.

  1. They've added my issue as a bug
  2. They suggested some workaround to try to check if it works but it wasn't working in dependabot.
  3. According to them merging the below PR in dependabot will resolve this issue (by upgrading the dart version of dependabot)
  4. They've also said a PR is recently added in dartlang repo for adding support for sha hash. Here's the code that does that https://github.com/dart-lang/pub/blob/29a7c0990cf070aa118570554f89402c37b54cf6/lib/src/command/dependency_services.dart#L550-L566

I think we should now wait to see @depedabot's further PRs in talawa to see if the issue got fixed or not.

@literalEval
Copy link
Member Author

Sounds good to me.

@palisadoes
Copy link
Contributor

palisadoes commented Mar 25, 2023

Will this be backward compatible with older versions of Dart? For example, the user could have the option of specifying the preferred version in YAML. A possibly better approach would be to auto detect the version of code.

Is my question relevant in this context?

@literalEval
Copy link
Member Author

@palisadoes everyone has the latest migrated version of Talawa codebase, so backward compatibility should not be a problem. Am I correct @SiddheshKukade ?

@SiddheshKukade
Copy link
Member

SiddheshKukade commented Mar 25, 2023

Will this be backwards compatible with older versions of Dart? For example, the user could have the option of specifying the preferred version in YAML. A possibly better approach would be to auto detect the version of code.

Yes they've added backward compatibility in here with an if condition:
Backword compatibility added in general context here.

Is my question relevant in this context?

Yes

@SiddheshKukade
Copy link
Member

@palisadoes everyone has the latest migrated version of Talawa codebase, so backward compatibility should not be a problem. Am I correct @SiddheshKukade ?

@literalEval Yes, In the context of talawa app everyone uses the common version. Otherwise it won't work.

@SiddheshKukade
Copy link
Member

@palisadoes Sir, can I work on other issues in the meantime until dependabot/dependabot-core#6454 gets merged?

@palisadoes
Copy link
Contributor

Will this be backward compatible with older versions of Dart? For example, the user could have the option of specifying the preferred version in YAML. A possibly better approach would be to auto detect the version of code.

OK

@palisadoes
Copy link
Contributor

palisadoes commented Mar 25, 2023

@palisadoes Sir, can I work on other issues in the meantime until dependabot/dependabot-core#6454 gets merged?

OK. Reference this issue in case you don't get assigned

@github-actions
Copy link

github-actions bot commented Apr 5, 2023

This issue did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

@github-actions github-actions bot added the no-issue-activity No issue activity label Apr 5, 2023
@literalEval
Copy link
Member Author

@SiddheshKukade what is the progress on this ? Why do we still need to manually bump package versions ?

@SiddheshKukade
Copy link
Member

@literalEval the PR that fixes this is still not merged from Jan on the main dependabot repo.

@literalEval
Copy link
Member Author

Hmm. That's sad. Can we do anything to get it merged ?

@palisadoes
Copy link
Contributor

dependabot/dependabot-core#6929 (comment)

They merged this. It looks like it's fixed and just waiting to be pushed to the GitHub workflow

@palisadoes
Copy link
Contributor

dependabot/dependabot-core#6929 (comment)

It's live. We can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working no-issue-activity No issue activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants