Skip to content

Unify syntax to define environment declarations (--define) #44562

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
Mabsten opened this issue Dec 28, 2020 · 10 comments
Closed

Unify syntax to define environment declarations (--define) #44562

Mabsten opened this issue Dec 28, 2020 · 10 comments
Assignees
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool.

Comments

@Mabsten
Copy link

Mabsten commented Dec 28, 2020

The syntax to define environment declarations (to read with String.fromEnvironment() method and relateds) is very divergent between the various tools/dart sub-commands:
(I tested the flags with Dart Sdk 2.12 (2.12.0-133.2.beta, on Windows x64))

dart2native / dart compile exe
use the -D/--define flag
(cfr https://dart.dev/tools/dart2native );

dart / dart run
does not support the define flags
UPDATE:
dart
supports the -D flag (also dart -D run is ok)
although it is not mentioned in the help and in the doc
(cfr https://dart.dev/tools/dart-vm );

dart2js
use only the -D flag;
(cfr https://dart.dev/tools/dart2js#options );

dart compile js
does not even support the -D flag, despite it being the substitute of the dart2js command (I think);

dartdevc -using webdev-
require to use a build.yaml file or this command-line syntax (not tested):
webdev serve -- '--define=build_web_compilers:ddc=environment={"SOME_VAR":"some_value"}'
(cfr https://dart.dev/tools/webdev and https://pub.dev/packages/build_web_compilers );

flutter (from the Sdk 1.17)
use the --dart-define flag.
The --dart-define flag is very useful in Flutter, for example to set a log level or an api token.

Wouldn't it be useful if all the sub-commands of the new dart tool supported the define flag? Or maybe I'm missing an alternative that already exists?

@srawlins srawlins added the area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. label Dec 29, 2020
@srawlins
Copy link
Member

I'll initially tag this as dart-cli; @devoncarew is there a plan to unify this flag?

@bkonyi
Copy link
Contributor

bkonyi commented Dec 29, 2020

There should be an effort to make this more consistent, and it's definitely an oversight that dart run -D isn't valid. At the very least, we should make sure that -D is accepted by the relevant commands (e.g., run, compile, test).

A temporary workaround is to provide -D before the CLI command as the VM will interpret those flags itself. This will work for the dart run case, but I'm not sure about the other commands.

@bkonyi
Copy link
Contributor

bkonyi commented Dec 29, 2020

@sigmundch can you or someone else take a look at the dart compile js case?

@bkonyi bkonyi self-assigned this Dec 29, 2020
@Mabsten
Copy link
Author

Mabsten commented Jan 2, 2021

Thank you for the attention and for the tip about dart -D, I didn't know that (it was not mentioned in the help); then the -D flag is available for all the old tools, except dartdevc/webdev (which however supports the build.yaml approach).

This flag is very important in the build pipeline because it provides (in pair with fromEnvironment()) an equivalent of BuildConfig fields in the Android (Gradle) development and of environment variables in the Node/Javascript build pipeline (I think to Js package to inline env vars like envify).

That the need for such a mechanism is strongly felt in the Dart community is testified by the multitude of related issues opened (as Please document {String|int|bool}.fromEnvironment) and of Dart packages that try to emulate it in various ways (with code generation, multiple entry point files, etc.), but all with severe drawbacks (i.e. dotenv, flutter_dotenv, envify, dynamic_config_generator, flutter_config, env_vars, dotenv2dart, etc).
Dart's environment declarations, now that they are also supported in flutter, can replace all previous workarounds.
It would be great if with the introduction of the new general-purpouse dart tool the syntax were unified, in order to make -D similar to --dart-define in flutter run and flutter build.

@Mabsten
Copy link
Author

Mabsten commented Jan 2, 2021

Another syntax-related issue concerns comma support:

dart2js and dart (VM tool)
allow to insert the comma in an environment declaration value (also without quote, which are instead required for the space).

dart2native and dart compile exe (as flutter)
doesn't allow to insert the comma, not even using the quotes.
This has already been reported for the flutter tool.
dart2native and dart compile exe (as flutter) on the other hand allow the definition of many variables without repeating the flag, using the comma: example: dart compile exe --dart-define=MY_VAR1="first value",MY_VAR2="second value" (also without quotes, if the values don't contains spaces).
I guess the problem is that the parsing is stopped as soon as the comma is found even if the string is surrounded by the quotation marks (with the space instead the quotation marks work correctly ie they make the string be read completely).
Reproducible test:

//main.dart

void main() {
  const my_var = String.fromEnvironment("my_var", defaultValue: "not found");
  print("my_var: $my_var");
}

Compile the source with:
dart -Dmy_var="first value,second value" main.dart
dart2js -Dmy_var="first value,second value" main.dart
dart2native -Dmy_var="first value,second value" main.dart
dart compile exe -Dmy_var="first value,second value" main.dart

Output using dart and dartsjs:

my_var: first value,second value

Output using dart2native and dart compile exe:

my_var: first value

dart-bot pushed a commit that referenced this issue Jan 13, 2021
commands

- Added support for --define to the VM and dart2js
- Added support for -D and --define for `dart run` and `dart compile js`

Remaining improvements:
- Add support for providing multiple comma separated values for `dart
  run`, `dart`, and `dart2js`

Related issue: #44562

TEST=Updated CLI tests and added new dart2js tests.

Change-Id: I9379c7aee1eab377adb3438393d9ad79c4938cc4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/178262
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
dart-bot pushed a commit that referenced this issue Jan 14, 2021
…and"

This reverts commit e83e784.

Reason for revert: Failing on SIMARM and AOT bots

Original change's description:
> [ CLI ] Improved consistency of -D and --define across tools and
> commands
>
> - Added support for --define to the VM and dart2js
> - Added support for -D and --define for `dart run` and `dart compile js`
>
> Remaining improvements:
> - Add support for providing multiple comma separated values for `dart
>   run`, `dart`, and `dart2js`
>
> Related issue: #44562
>
> TEST=Updated CLI tests and added new dart2js tests.
>
> Change-Id: I9379c7aee1eab377adb3438393d9ad79c4938cc4
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/178262
> Commit-Queue: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>

TBR=bkonyi@google.com,asiva@google.com,sigmund@google.com

Change-Id: I1c594ae7db551619cc3191ff7f832c4fc61a4171
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/179081
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
dart-bot pushed a commit that referenced this issue Feb 5, 2021
…and commands"

- Added support for --define to the VM and dart2js
- Added support for -D and --define for `dart run` and `dart compile js`

Remaining improvements:
- Add support for providing multiple comma separated values for `dart
  run`, `dart`, and `dart2js`

Related issue: #44562

TEST=Updated CLI tests and added new dart2js tests.

This reverts commit e499377.

Change-Id: I5f9275b829665eb5e8695403d67f230e752ab0e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183180
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
@bkonyi
Copy link
Contributor

bkonyi commented Feb 12, 2021

Closing this issue as --define support has been improved across all tools. Filed #44995 for the issue described by @Mabsten above.

@jodinathan
Copy link

how to add environment variables to dart run to start a VM console app?

this works: dart -Dconsole=true bin/pagebuilder_api.dart

this doesn't: dart -Dconsole=true run -Dconsole=true

Roms1383 added a commit to Roms1383/flutter_rust_bridge that referenced this issue Oct 4, 2022
- env vars cannot be piped to wasm binary (AFAIU, see bevyengine/bevy#5737 (comment))
- args can be passed though, but dart2js has a specificity: it only works with '-D' syntax, not '--define' as it seems (see dart-lang/sdk#44562 huge thanks @Mabsten)
- logs from rust won't appear in the terminal, but can be seen in the browser's console spinned by puppeteer: this is why outputting to stdout is not particularly useful on web platform. since the browser gets spinned up and down very quickly, but you can peek at them either using --pause-isolates-on-exit on dart run command (more convenient), or by sleeping thread, etc
@pak3nuh
Copy link

pak3nuh commented Mar 19, 2023

I don't think the define flag is supported across all commands evenly. I'm using dart SDK 2.19 and the test command doesn't respect the flag. For it to work, I need to run each test file individually with the run command.

Is this happening to anyone else?

@bkonyi
Copy link
Contributor

bkonyi commented Mar 20, 2023

@pak3nuh would you mind filing a new issue? This one has been closed for quite awhile.

@pak3nuh
Copy link

pak3nuh commented Mar 20, 2023

Sure. Created issue #51791

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

5 participants