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

build needs a --clean flag #1170

Closed
kevmoo opened this issue Mar 15, 2018 · 17 comments
Closed

build needs a --clean flag #1170

kevmoo opened this issue Mar 15, 2018 · 17 comments
Labels
closed-duplicate Closed in favor of an existing report package:build_runner type-enhancement A request for a change that isn't a bug

Comments

@kevmoo
Copy link
Member

kevmoo commented Mar 15, 2018

Related to #1104

I'm doing a lot of rm -rf .dart_tool/build; pbr build to reproduce an issue, etc

@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug package:build_runner labels Mar 15, 2018
@matanlurey
Copy link
Contributor

Duplicate of #1104.

@matanlurey matanlurey added the closed-duplicate Closed in favor of an existing report label Mar 15, 2018
@kevmoo
Copy link
Member Author

kevmoo commented Mar 15, 2018

Not quite.

A stand-alone clean command. Great.

This is asking for a --clean flag to be added to the build command.

@kevmoo kevmoo reopened this Mar 15, 2018
@matanlurey
Copy link
Contributor

I'm not sure of the value here. It's 1 command versus 2?

We don't want this to be a common flag/command, for example in #1104 I outline that we should prompt the user to file a bug everytime they have to use this, because it means we are probably doing something wrong.

@kevmoo
Copy link
Member Author

kevmoo commented Mar 15, 2018

It's nice when debugging builders, etc – to run everything every time...

I guess one could just do pbr clean; pbr build 🤷‍♂️

@matanlurey
Copy link
Contributor

Right, but the point is you shouldn't need it - builders should rerun from scratch if changed.

@kevmoo
Copy link
Member Author

kevmoo commented Mar 15, 2018

Right, but the point is you shouldn't need it - builders should rerun from scratch if changed.

Right, but often you want to rerun several times from scratch as you're trying to step through and debug...

@natebosch
Copy link
Member

Today we typically touch a line of code in our builder which invalidates the entire build. I personally haven't had a need to rerun with a builder in exactly the same code state...

@natebosch
Copy link
Member

@DineshBhosale - If things are working correctly you should not need to delete this directory. If you update your configuration it will automatically clean whatever is necessary. The proposed --clean command is only for when things are going wrong, or here as a substitute for --rerun-failing-actions.
#910 (comment)

What issues are you seeing when you don't delete it?

@matanlurey
Copy link
Contributor

Closing. There is no reason to be "extra sure". If we need things cleaned up, its a bug.

@MikeMitterer
Copy link

I have another use case:
My builder reads a version info from git and generates some files.
Builder isn't aware of git changes so it doesn't rebuild the files.
--clean option would solve this problem.

My workaround is:

rm -rf .dart_tool/build/generated && pub run build_runner build

git_version: https://github.com/MikeMitterer/dart-git_version
usage-sample: https://github.com/MikeMitterer/mini_web_sample

pre condition:
e.g.: git tag v1.0

@matanlurey
Copy link
Contributor

matanlurey commented Jun 4, 2018

@MikeMitterer:

My builder reads a version info from git and generates some files.
Builder isn't aware of git changes so it doesn't rebuild the files.

That sounds like the whitelisting feature of the build system:

const List<String> defaultRootPackageWhitelist = const [
'benchmark/**',
'bin/**',
'example/**',
'lib/**',
'test/**',
'tool/**',
'web/**',
'pubspec.yaml',
'pubspec.lock',
];

I'd open up another bug asking how to customize this whitelist, not for --clean.

/cc @natebosch

@natebosch
Copy link
Member

natebosch commented Jun 4, 2018

My builder reads a version info from git and generates some files.

I guess you are using dart:io directly? This is not supported. If you need to be able to read files from .git/ you should add them to a target. Perhaps:

targets:
  $default:
    sources:
      exclude: [".git/**"]
   _git:
    sources:
      include: [".git/**"]

After that you should be able to construct an asset ID like new AssetId(<your package name>, '.git/path/to/file') and read these files through the build step.

@MikeMitterer
Copy link

MikeMitterer commented Jun 4, 2018

How would a whitelisting help if files don't change? OK - there are some changes but they are deep inside of .git

mini_web_sample $ git tag -l --sort=v:refname
v1.0
> mini_web_sample $ git describe --tags --match v1.0
v1.0-4-gb61c783

Thats what the package does (@natebosch: https://github.com/MikeMitterer/dart-git_version/blob/master/lib/git_version.dart) , then it cuts off everything after the last dash.
Result v1.0-4

Every commit increases the last value v1.0-4 -> v1.0-5
The build system doesn't know anything about those commits.

The builder replaces %version% in index.tmpl.html and changes the extension to index.html
It works the same way with version.tmpl.json. The last thing the builder does is - it generates 'git.version.dart'

Why is a --clean option so bad? It's easy to understand and easy to use.
It would also be quite useful for CI system.

pub run build_runner build --clean - looks clean and everyone know what it does

@matanlurey
Copy link
Contributor

@MikeMitterer As mentioned, using dart:io in the builder is not supported.

Take a look at @natebosch's comment for an example of what to do.

@jakemac53
Copy link
Contributor

Another option would be to run the git command and output it to a file in your package, then read that file in the builder.

Then, you just re-run the git command and run a build and only that one builder will rerun.

@jakemac53
Copy link
Contributor

You could even wrap up the build command in a little shell script that does that automatically for you before each build?

@natebosch
Copy link
Member

Why is a --clean option so bad?

It's a pattern we want to discourage so we don't want to build in explicit support and give the impression that we think it's something users should ever have to do.

It's easy to understand and easy to use.

It's also rarely something the users wants to happen. In this case it's definitely not what you want to happen - you wouldn't want to rerun DDC or any other builders that aren't impacted by the version change coming from git. You'd really like to rerun only that builder and any other builders that depend on it, not the entire build graph. I filed #1512 to track the feature that would solve this problem.

It would also be quite useful for CI system.

pub run build_runner clean && pub run build_runner build is pretty easy for a CI system, and we can always fall back on general "don't share build artifacts" practices which are already common practice for hermetic CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-duplicate Closed in favor of an existing report package:build_runner type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants