Skip to content

dart pub --disable-dartdev-analytics works but dart --disable-dartdev-analytics pub doesn't #44135

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

Closed
sigurdm opened this issue Nov 10, 2020 · 3 comments
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool.

Comments

@sigurdm
Copy link
Contributor

sigurdm commented Nov 10, 2020

This is because the vm doesn't recognize the flag, but this flag belongs to the main dart command, not the subcommand.

This might not be urgent, as this flag is not exposed by default.

But it makes embedding the dart tool into eg. flutter awkward.

See dart-lang/pub#2736

@sigurdm sigurdm added the area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. label Nov 10, 2020
@sigurdm
Copy link
Contributor Author

sigurdm commented Nov 12, 2020

dart-bot pushed a commit that referenced this issue Nov 13, 2020
Some other refactorings are piggy-backed along.

TestProject.runSync no longer takes a 'command' argument. It was anyway
often not an argument.

Also stop the messy handling of pub arguments. It is no longer needed.

BUG: #44135
TEST=The VM change is tested via all the pkg/dartdev/test/command/* tests that invoke dart with the --no-analytics flag.

Change-Id: Ib5a1a29841a5fdb28663b7f60c5d6fc31ba252d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171284
Commit-Queue: Sigurd Meldgaard <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
Reviewed-by: Jonas Jensen <[email protected]>
dart-bot pushed a commit that referenced this issue Nov 13, 2020
This reverts commit cda994f.

Reason for revert: This stopped all sending of analytics from dartdev.

The added flag in dartdev had an implicit default value of 'false' -> no analytics would be sent.

Also the main_options.cc change was broken and did not pass the argument on to dartdev.

This failed to be caught by tests because all test are run with analytics off, and that happened implicitly.

Original change's description:
> Improve handling of disable-dartdev-analytics
>
> Some other refactorings are piggy-backed along.
>
> TestProject.runSync no longer takes a 'command' argument. It was anyway
> often not an argument.
>
> Also stop the messy handling of pub arguments. It is no longer needed.
>
> BUG: #44135
> TEST=The VM change is tested via all the pkg/dartdev/test/command/* tests that invoke dart with the --no-analytics flag.
>
> Change-Id: Ib5a1a29841a5fdb28663b7f60c5d6fc31ba252d0
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171284
> Commit-Queue: Sigurd Meldgaard <[email protected]>
> Reviewed-by: Martin Kustermann <[email protected]>
> Reviewed-by: Jonas Jensen <[email protected]>

[email protected],[email protected],[email protected],[email protected]

Change-Id: I92ef65b16cdb75fb2475faf9f522fda62e181bab
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171941
Reviewed-by: Sigurd Meldgaard <[email protected]>
Commit-Queue: Sigurd Meldgaard <[email protected]>
dart-bot pushed a commit that referenced this issue Dec 1, 2020
This is second try of https://dart-review.googlesource.com/c/sdk/+/171284
that was reverted to to faulty logic in main_options.

Some other refactorings are piggy-backed along.

TestProject.runSync no longer takes a 'command' argument. It was anyway
often not an argument.

Also stop the messy handling of pub arguments. It is no longer needed.

BUG: #44135
Change-Id: I49abf5810d9ea262409ba9d93f0471037cb8a753
TEST=The VM change is tested via all the pkg/dartdev/test/command/* tests that invoke dart with the --no-analytics flag.
TEST=Furthermore manual test that the --no-analytics flag is passed to dartdev.
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174261
Reviewed-by: Jonas Jensen <[email protected]>
Commit-Queue: Sigurd Meldgaard <[email protected]>
dart-bot pushed a commit that referenced this issue Dec 1, 2020
This reverts commit 58860f4.

Reason for revert: Broke bots. Will investigate.

Original change's description:
> Improve handling of disable-dartdev-analytics
>
> This is second try of https://dart-review.googlesource.com/c/sdk/+/171284
> that was reverted to to faulty logic in main_options.
>
> Some other refactorings are piggy-backed along.
>
> TestProject.runSync no longer takes a 'command' argument. It was anyway
> often not an argument.
>
> Also stop the messy handling of pub arguments. It is no longer needed.
>
> BUG: #44135
> Change-Id: I49abf5810d9ea262409ba9d93f0471037cb8a753
> TEST=The VM change is tested via all the pkg/dartdev/test/command/* tests that invoke dart with the --no-analytics flag.
> TEST=Furthermore manual test that the --no-analytics flag is passed to dartdev.
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174261
> Reviewed-by: Jonas Jensen <[email protected]>
> Commit-Queue: Sigurd Meldgaard <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: I754bcebdcfc595158b04d431662b65bf25f5b89d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174466
Reviewed-by: Sigurd Meldgaard <[email protected]>
Commit-Queue: Sigurd Meldgaard <[email protected]>
dart-bot pushed a commit that referenced this issue Dec 4, 2020
This is a reland of 58860f4

Original change's description:
> Improve handling of disable-dartdev-analytics
>
> This is second try of https://dart-review.googlesource.com/c/sdk/+/171284
> that was reverted to to faulty logic in main_options.
>
> Some other refactorings are piggy-backed along.
>
> TestProject.runSync no longer takes a 'command' argument. It was anyway
> often not an argument.
>
> Also stop the messy handling of pub arguments. It is no longer needed.
>
> BUG: #44135
> Change-Id: I49abf5810d9ea262409ba9d93f0471037cb8a753
> TEST=The VM change is tested via all the pkg/dartdev/test/command/* tests that invoke dart with the --no-analytics flag.
> TEST=Furthermore manual test that the --no-analytics flag is passed to dartdev.
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174261
> Reviewed-by: Jonas Jensen <[email protected]>
> Commit-Queue: Sigurd Meldgaard <[email protected]>

Change-Id: I725662f578d061f87171ceffe9aff3de83688f58
TEST=Furthermore run the vm-kernel-precomp-obfuscate-linux-release-x64-try trybot
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174473
Commit-Queue: Sigurd Meldgaard <[email protected]>
Reviewed-by: Jonas Jensen <[email protected]>
@bkonyi
Copy link
Contributor

bkonyi commented Mar 18, 2021

@sigurdm is this still relevant?

@sigurdm
Copy link
Contributor Author

sigurdm commented Mar 18, 2021

No! It was fixed with: https://dart-review.googlesource.com/c/sdk/+/171284 !

@sigurdm sigurdm closed this as completed Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool.
Projects
None yet
Development

No branches or pull requests

2 participants