-
Notifications
You must be signed in to change notification settings - Fork 213
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
Add support for multiple full paths on macos #2276
Conversation
We cannot look up multiple basename commands in the system path and the current `macOsExecutable` configuration may have existing uses in `dart_test.yaml` files so it isn't safe to require full paths. Add a separate `macOsAbsolutePaths` configuration to enable internal definitions that check multiple full paths and execute the first one that exists. There is no support here for `dart_test.yaml`. Users that are configuring an executable for macOS will continue to have support for only a single value, even if they are specifying an absolute path.
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. Package publish validation ✔️
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
This is the least invasive way I could see to make this change. I'm not sure if it's worth landing, but opening the PR for discussion. |
I think if we wanted we could support this in |
fixes merge behavior to correctly override.
Co-authored-by: Jacob MacDonald <jakemac@google.com>
Co-authored-by: Jacob MacDonald <jakemac@google.com>
This will exercise things like launching browsers (where they are currently working on CI). Add `osx` to the sharded `package:test` tests alongside `windows`. These exercise the majority of OS specific behavior. Skip existing safari tests due to a bug running tests in non-interactive environments. (#1253) Skip existing firefox tests due to a changed executable path. (Planned fix in #2276) Refactor a ternary chain to an if/else chain to add the extra conditions for firefox and safari.
This fixes the problem for me on a local mac. It looks like maybe firefox doesn't come by default on github actions macos runs? I don't think mono package supports adding a firefox setup step. Any suggestions on a workaround @kevmoo ? |
No, it still fails even if I manually add the setup-firefox steps. |
Does Github Actions have it on the path?
Cannot currently add setup firefox action Tweak failure message.
I cannot figure out the mac problem. Landing this is still worth it because it does fix things for me locally. @jakemac53 any concerns about landing this without unskipping the tests on CI, or with using the basename as a fallback? |
I am fine with it, we don't have a lot of people asking about this, so it isn't worth dumping a ton of time into. If people do get broken again they will file issues (plus, there is a workaround to set your own path). |
Revisions updated by `dart tools/rev_sdk_deps.dart`. dartdoc (https://github.com/dart-lang/dartdoc/compare/8100ccf..526dbd5): 526dbd55 2024-09-15 Sam Rawlins Strip the enclosing package's lower-case name from a library's dir name (dart-lang/dartdoc#3883) 98dd9dad 2024-09-13 Sam Rawlins Add output-size info to the '--stats' flag on various tasks. (dart-lang/dartdoc#3879) 090b05ba 2024-09-13 Sam Rawlins Bump to 8.2.0 (dart-lang/dartdoc#3880) 0a529762 2024-09-12 Sam Rawlins Fix search which is not using canonical libraries as enclosing elements (dart-lang/dartdoc#3877) d4dff9a4 2024-09-12 Konstantin Scheglov Stop using 'augmented' from the element model. (dart-lang/dartdoc#3878) d6b865f9 2024-09-11 Sam Rawlins Support multiple partial calls from a single template (dart-lang/dartdoc#3873) 25c9edf3 2024-09-10 Parker Lougheed Standardize to newer SDK APIs (dart-lang/dartdoc#3872) b61de2f2 2024-09-10 Sam Rawlins Clean up some sorting, naming, and doc nits (dart-lang/dartdoc#3876) 55ce1164 2024-09-10 Sam Rawlins Use latest language version for DartFormatter (dart-lang/dartdoc#3875) 2a262818 2024-09-10 Sam Rawlins Comply with analyzer 6.9.0 APIs (dart-lang/dartdoc#3874) ca98b898 2024-09-09 Sam Rawlins Include extension members on the extended type's page (dart-lang/dartdoc#3863) mockito (https://github.com/dart-lang/mockito/compare/d0fda0c..b66be81): b66be81 2024-09-13 Googler Migration for analyzer APIs. stack_trace (https://github.com/dart-lang/stack_trace/compare/090d3d1..5b82965): 5b82965 2024-09-19 Ömer Sinan Ağacan Relax URI matching in V8 Wasm frame regex (dart-lang/stack_trace#161) 9b1ed4f 2024-09-19 Ömer Sinan Ağacan Add support for parsing Wasm stack frames of Chrome (V8), Firefox, Safari (dart-lang/stack_trace#159) d38eee8 2024-09-17 Ömer Sinan Ağacan Fix analysis issues (dart-lang/stack_trace#160) test (https://github.com/dart-lang/test/compare/9a2d155..22835e2): 22835e2e 2024-09-13 Nate Bosch Add support for multiple full paths on macos (dart-lang/test#2276) web (https://github.com/dart-lang/web/compare/933a37d..d8549a3): d8549a3 2024-09-13 Srujan Gaddam Add a generate-all flag to emit all bindings (dart-lang/web#302) Change-Id: Ib07e4c0eda9c8ea180ef989c7d9cf20c390457e8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386260 Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Auto-Submit: Devon Carew <devoncarew@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Revisions updated by `dart tools/rev_sdk_deps.dart`. dartdoc (https://github.com/dart-lang/dartdoc/compare/8100ccf..526dbd5): 526dbd55 2024-09-15 Sam Rawlins Strip the enclosing package's lower-case name from a library's dir name (dart-lang/dartdoc#3883) 98dd9dad 2024-09-13 Sam Rawlins Add output-size info to the '--stats' flag on various tasks. (dart-lang/dartdoc#3879) 090b05ba 2024-09-13 Sam Rawlins Bump to 8.2.0 (dart-lang/dartdoc#3880) 0a529762 2024-09-12 Sam Rawlins Fix search which is not using canonical libraries as enclosing elements (dart-lang/dartdoc#3877) d4dff9a4 2024-09-12 Konstantin Scheglov Stop using 'augmented' from the element model. (dart-lang/dartdoc#3878) d6b865f9 2024-09-11 Sam Rawlins Support multiple partial calls from a single template (dart-lang/dartdoc#3873) 25c9edf3 2024-09-10 Parker Lougheed Standardize to newer SDK APIs (dart-lang/dartdoc#3872) b61de2f2 2024-09-10 Sam Rawlins Clean up some sorting, naming, and doc nits (dart-lang/dartdoc#3876) 55ce1164 2024-09-10 Sam Rawlins Use latest language version for DartFormatter (dart-lang/dartdoc#3875) 2a262818 2024-09-10 Sam Rawlins Comply with analyzer 6.9.0 APIs (dart-lang/dartdoc#3874) ca98b898 2024-09-09 Sam Rawlins Include extension members on the extended type's page (dart-lang/dartdoc#3863) mockito (https://github.com/dart-lang/mockito/compare/d0fda0c..b66be81): b66be81 2024-09-13 Googler Migration for analyzer APIs. stack_trace (https://github.com/dart-lang/stack_trace/compare/090d3d1..5b82965): 5b82965 2024-09-19 Ömer Sinan Ağacan Relax URI matching in V8 Wasm frame regex (dart-lang/stack_trace#161) 9b1ed4f 2024-09-19 Ömer Sinan Ağacan Add support for parsing Wasm stack frames of Chrome (V8), Firefox, Safari (dart-lang/stack_trace#159) d38eee8 2024-09-17 Ömer Sinan Ağacan Fix analysis issues (dart-lang/stack_trace#160) test (https://github.com/dart-lang/test/compare/9a2d155..22835e2): 22835e2e 2024-09-13 Nate Bosch Add support for multiple full paths on macos (dart-lang/test#2276) web (https://github.com/dart-lang/web/compare/933a37d..d8549a3): d8549a3 2024-09-13 Srujan Gaddam Add a generate-all flag to emit all bindings (dart-lang/web#302) Change-Id: Ib07e4c0eda9c8ea180ef989c7d9cf20c390457e8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386260 Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Auto-Submit: Devon Carew <devoncarew@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
We cannot look up multiple basename commands in the system path and the
current
macOsExecutable
configuration may have existing uses indart_test.yaml
files so it isn't safe to require full paths. Add aseparate
macOsAbsolutePaths
configuration to enable internaldefinitions that check multiple full paths and execute the first one
that exists.
Add the basename
firefox
as a fallback. Adding that command to thepath is one workaround for users with firefox installed in an unexpected
location. This is also the only approach that is compatible with mac on
GitHub actions using the
setup-firefox
action.There is no support here for
dart_test.yaml
. Users that areconfiguring an executable for macOS will continue to have support for
only a single value, even if they are specifying an absolute path.
Tests remain skipped because our mono repo setup does not have a way to
include the
setup-firefox
action.