dart-lang / setup-dart

A GitHub Action to install and setup a Dart SDK.
BSD 3-Clause "New" or "Revised" License
192 stars 38 forks source link

Support renaming the be channel to main. #102

Closed sortie closed 1 year ago

sortie commented 1 year ago

This change automatically detects when the main channel pops into existence and starts using the latest version of it instead. The forward compatibility will be removed once the channel has been renamed.

Bug: b/270022609

--

I couldn't figure out how to test this change. Could you add some instructions to DEVELOPING about how I can run and test my changes locally? Maybe I could catch a more specific exception or http status code to perform the fallback more cleanly so a transient failure doesn't downgrade from main to be.

No particular timetable on when we'll rename the be channel to main. I actually believe this is the final piece of forward compatibility needed but I'll need to confirm that.

sortie commented 1 year ago

The tests appear to fail like my attempts to run it locally.

I literally just ran:

sudo apt-get install nodejs
sudo npm i -g @vercel
dart pub get # on 3.2.0-134.0.dev
npm run all

Please advise and update DEVELOPING if I'm doing something wrong?

devoncarew commented 1 year ago

I'll review and see if I can spot where this is having problems. In the meantime, is there a github issue you can reference (from the PR and the code) instead of a buganizer issue? Even if the two issues are cross-referenced, it would help w/ visibility.

devoncarew commented 1 year ago
sudo apt-get install nodejs
sudo npm i -g @vercel
dart pub get # on 3.2.0-134.0.dev
npm run all

Please advise and update DEVELOPING if I'm doing something wrong?

Those steps look correct (the setup and re-compiling the javascript code), and matches whats in https://github.com/dart-lang/setup-dart/blob/main/DEVELOPING.md. From the failures it does look like there's some issue with the javascript produced. Perhaps due to differing versions of node or of dart2js?

devoncarew commented 1 year ago

The bulk of this action is tested via the various matrixs in this file: https://github.com/dart-lang/setup-dart/blob/main/.github/workflows/dart.yml

I did make a small change to a file and verified that my version of node and dart sdk produces working javascript: https://github.com/dart-lang/setup-dart/pull/103.

Dart SDK version: 3.2.0-134.0.dev ncc: 0.36.1

sortie commented 1 year ago

I addressed the code review but unfortunately the tests are still broken when I use the same versions as you.

sortie@sortie ~/src/setup-dart $ npm --version
9.2.0
sortie@sortie ~/src/setup-dart $ sudo npm i -g @vercel/ncc@0.36.1

changed 1 package in 2s
sortie@sortie ~/src/setup-dart $ ncc --version
0.36.1
sortie@sortie ~/src/setup-dart $ dart --version
Dart SDK version: 3.2.0-134.0.dev (dev) (Mon Sep 4 13:06:39 2023 -0700) on "linux_x64"
sortie@sortie ~/src/setup-dart $ dart pub get
Resolving dependencies... 
Got dependencies!
Resolving dependencies in ./example... (1.4s)
Got dependencies in ./example.
sortie@sortie ~/src/setup-dart $ npm run all

> setup-dart@0.0.0 all
> npm run build && npm run dist

> setup-dart@0.0.0 build
> dart compile js --enable-experiment=inline-class -olib/main.js lib/main.dart && dart tool/sig.dart --generate

Hint: When run on the command-line, the compiled output might require a preamble file located in:
  <sdk>/lib/_internal/js_runtime/lib/preambles.
Compiled 9,932,454 input bytes (5,130,131 characters source) to 357,512 characters JavaScript in 1.92 seconds using 236.555 MB of memory

> setup-dart@0.0.0 dist
> ncc build lib/main.mjs && cp lib/main.js dist/main.cjs && cp lib/sig.txt dist/sig.txt

ncc: Version 0.36.1
ncc: Compiling file index.mjs into ESM
12kB  dist/index.mjs
12kB  [662ms] - ncc 0.36.1
sortie commented 1 year ago

Ah good point about force pushes. A habit from my work in other code hosting solutions hehe. Thanks for recompiling for actions for me. Although I really would love if we could figure out why I can't compile them myself, or if there's something special about your setup. I added a CHANGELOG entry :)

devoncarew commented 1 year ago

Although I really would love if we could figure out why I can't compile them myself, or if there's something special about your setup.

Yeah, I definitely agree here - I want to make sure everyone can contribute; probably worth tracking in an issue to see if other people hit it / have ideas.

Thanks for updating the action!