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

feat: Avoid ignoring generated files in libraries #233

Closed
wants to merge 1 commit into from

Conversation

Gustl22
Copy link
Contributor

@Gustl22 Gustl22 commented Nov 12, 2024

Description

In contrast to apps, it is usually mandatory to include used .g.dart files in git versioning as they are part of the library (and they are in fact included in the package in pub.dev).
Right now any fork cannot be used remotely (via overriding pubspec dependencies) unless they include the generated files themselves into git.
The files are not generated by the build_runner and also shouldn't, as the app developer may doesn't even use build_runner.

Refs:
https://dart.dev/guides/libraries/private-files#details
https://stackoverflow.com/a/55177035/5164462

Build_runner / Dart is not proposing to ignore these:
https://github.com/dart-lang/build/blob/master/.gitignore

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Code Style Guide and followed the process outlined there for submitting PRs.
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze or dart analyze) does not report any problems on my PR.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I am authorized to release this code under the MIT License and agree to do so

UI Change

Does your PR affect any UI screens?

  • Yes, the relevant screens are included below.
  • No, there are no UI impacts with this PR.

@jpeiffer
Copy link
Contributor

Sorry, but no. The generated files are compile time artifacts and I have no intention of including them. The pipeline that builds and publishes ensures they are built fresh and published to pub.dev. I do wish to have a conflict between generated files that made it to GitHub and the ones generated at publish time.

@jpeiffer jpeiffer closed this Nov 30, 2024
@Gustl22
Copy link
Contributor Author

Gustl22 commented Dec 3, 2024

The generated files are compile time artifacts

No they are not build at compile time (they are not used for compilation only). They are also necessary for code analysis in the IDE. They are just generated code which is part of the source code, and should be treated like that in my opinion.
I know that it sounds confusing, but it is done by purpose from flutters design. I see your argument in other programming languages and other use cases, but not in libraries.

For that reason Flutter also checks in all the generated dart files:
https://github.com/search?q=repo%3Aflutter/packages%20path%3A.g.dart&type=code

I do wish to have a conflict between generated files that made it to GitHub and the ones generated at publish time.

You DO wish or you DON'T?
If you DO, then you should check-in them, to be able to check the differences.


Currently it is not possible to use betas like 3.27.x with your package:

dependencies:
  json_theme:
    git:
      url: "https://github.com/barbl058/json_theme.git"
      path: "json_theme"
      ref: "fsupport-flutter-3.27"

So what I have to do in order for this to work is to check in the source files, like i did in this PR + the regular changes:

main...Gustl22:flutter_json_theme:flutter-3.27.x

I ask you to reconsider your descision and to might conform with the way how Flutter / Dart is proposing it to do. Thank you.

@jpeiffer
Copy link
Contributor

jpeiffer commented Dec 20, 2024

Still not committing generated files. They are published, but not committed.

That said, your request can be met by creating a beta version with both a pubspec version and a changelog that explains the beta. Hopefully you can find a solution to your problem via that, and if not, TBH, I'd like to hear why not and maybe that might get me to reconsider.

Even if I checked in the generated files, you'd have the same exact problem. They'd be generated against a flutter version that differs (and doesn't allow) what was generated. Checking in the generated files solves nothing. However, creating a beta with an updated workflow that builds, publishes, and generates against said beta, would.

@Gustl22
Copy link
Contributor Author

Gustl22 commented Dec 20, 2024

Thank you for your repsonse.

Still not committing generated files. They are published, but not committed.

Would be nice to also object my other arguments, I made before.

That said, your request can be met by creating a beta version with both a pubspec version and a changelog

No my request can't be met like that. Imagine you have the main branch and two feature branches and need a feature or fix from the one branch, but not the other. One can't simply use your package (or more precise the patch of your package from a contributor) unless they are checking them out locally (which is unnecessarily hard in an app pipeline).

Also in rapid edge development one can't wait two months until they can continue like it was the case in #229. One needs to be able to use the package immediately via a git dependency, also a pre-release doesn't help there, as the maintainer might not be available in that urgency or refuses to apply a change (also for good reasons).

Your package should work regardless!! of which dependeny method you choose, either locally, via pub or via git.
So if one provides a fix the other developers are able to choose whether they want to use the fix via git or not.

They'd be generated against a flutter version that differs (and doesn't allow) what was generated.

You are right, that solves nothing if one doesn't update the generated files also. Otherwise it wouldn't make sense: checking in files, but not updating them -> not working. If someone provides a fix, they must regenerate the files of course in order to make them work, and these are checked in, so it is guaranteed to work regardless of the build_runner version etc. which can also differ locally. One can double check in the publish pipeline of course, if they were generated correctly.

From your other post:

That's a lost cause, and the truth is, it wouldn't help you in any way even if I did. The generated files are published, but not committed. That means every release has them. Beta, stable, whatever. So if a published release doesn't work for you, no git version would either.

But that's an architectural problem of your build pipeline(?). And it's also not directly interferring with my problem. You can still generate these additionally in your publish pipeline and e.g. check potential differences to the checked in files. A published package on pub.dev currently contains the .g.dart files due to the .pubignore file overriding your .gitignore, in contrast to a git dependency, which does not include the generated files and therefore cannot be used, because build_runner does not generate files within thrid party packages (regardless if git or pub), even if you execute it in your app.

It will publish the generated files so you can use them. It keeps the Git history clean.

That's understanable to keep the git as small as possible, I see that. I also like that idea. But Flutter build_runner wasn't meant for this as I showed in the proposed .gitignore before. This changes with macros, where these files can be generated at build time within the dart language (without addional files).
A git history in that regards also ensures, that a build runner version doesn't mess up with the generated files, so one can check the differences, if something doesn't work any more.

And you can ref them from pub and not need a git ref, which is strongly discouraged anyway.

That's not true per se. Referencing a git package is pretty common, e.g. for private packages, where it's not worth and also overcomplicating to set up its own pub.dev server.
It's only not allowed to have git dependencies within other published packages, which makes total sense. But not for the app repo.

It is also "strongly" discouraged to publish package resources (on pub.dev), which aren't checked into git:
https://dart.dev/tools/pub/publishing#what-files-are-published
https://stackoverflow.com/questions/69760021/what-should-be-ignored-in-darts-pubignore/69767697#69767697

pubignore was not meant to include additional files, which were ignored by gitignore, but to remove files, which are not needed during publishing. So removing *.g.dart files in .gitignore and removing pubignore completely would be the proposed decision (if you are speaking of discouraged and encouraged).

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.

2 participants