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

run bare rust CLI binary, no node #57

Merged
merged 10 commits into from Jan 7, 2023
Merged

run bare rust CLI binary, no node #57

merged 10 commits into from Jan 7, 2023

Conversation

alexeagle
Copy link
Member

@alexeagle alexeagle commented Jun 21, 2022

BREAKING CHANGES:

  • the term "transpile" is not typically used in swc, so the underlying rule is renamed swc_compile matching the command we run swc compile ...
  • We force users to upgrade to newest SWC version that is compatible. We print a nice error if your swc is older than 1.3.25

@alexeagle
Copy link
Member Author

alexeagle commented Sep 2, 2022

Okay this has been hard and I haven't found much focus time for it.

As far as I know, swc-project/swc#5171 was the last bug preventing Bazel from using the pure-rust CLI.

We will have some rules_swc API changes:

  • just mirror the rust CLI, no npm packages
  • allow integrity_hashes={"darwin-arm64": "sha512-DuB... to be passed along with version number in WORKSPACE so users don't have to rely on our mirroring
  • look for .swcrc in the package and use it if present, like we do for tsconfig.json in ts_project
  • maybe the swcrc file becomes required to make the rust CLI work. Accept a dictionary to dynamically generate it, just like we do with tsconfig.json
  • get all the tests passing again, and document the breaking changes that were required

@alexeagle
Copy link
Member Author

Update: it's still necessary to provide a fake tty to make the swcx binary accept a folder, now trying to make a tty_binary rule in aspect-build/bazel-lib#251

This keeps getting stuck because it's a side-project for me. We need some sponsorship for this work. Filed #88 so that the PR has an associated issue.

@cgrindel cgrindel assigned cgrindel and alexeagle and unassigned cgrindel Sep 30, 2022
@realtimetodie
Copy link
Contributor

I'm working on this

@realtimetodie
Copy link
Contributor

Fixed an upstream bug, see #88

@alexeagle
Copy link
Member Author

awesome, LMK if you'd like some help getting this rebased or anything

@realtimetodie
Copy link
Contributor

realtimetodie commented Dec 24, 2022

Yes, I already made a rebase in https://github.com/realtimetodie/rules_swc/tree/swc_rust and got bazel build //... running with a custom swc cli and the changes from swc-project/swc#6708. In addition, I found new bugs in the swc cli and will create subsequent PRs upstream.

bazel test //... 13 tests pass and 3 fail locally. All failed tests are related to the --output-dir option of the swc cli compile command. It is due to the cwd. The cwd is set to the execroot directory and the swc cli can not find the auto-generated mock test files that are written into the bazel-out directory. But I think this might be related to my custom binary location, see

realtimetodie@ac516ac#diff-1444361579758fc6be4458b5105b2ee3ee623d72154094c3fa1fd9056845903eR154

Test summary

//docs:update_test                                              (cached) PASSED in 0.0s
//examples/custom_outs:tests_0_test                             (cached) PASSED in 0.1s
//examples/custom_outs:tests_1_test                             (cached) PASSED in 0.0s
//examples/macro:test_test                                      (cached) PASSED in 0.0s
//examples/opaque_src:test_test                                 (cached) PASSED in 0.1s
//examples/out_dir:check_outputs                                (cached) PASSED in 0.1s
//examples/paths:test_test                                      (cached) PASSED in 0.1s
//examples/rc/src:tests_0_test                                  (cached) PASSED in 0.1s
//examples/rc/src:tests_1_test                                  (cached) PASSED in 0.0s
//examples/root_dir:check_outputs                               (cached) PASSED in 0.1s
//examples/simple:test_test                                     (cached) PASSED in 0.1s
//examples/transitive:assertion_test                            (cached) PASSED in 0.0s
//swc/tests:versions_test_test_0                                (cached) PASSED in 0.1s
//examples/directory:minify_test                                         FAILED in 0.0s
//examples/directory:out_dir_test                                        FAILED in 0.0s
//examples/filegroup:check_outputs                                       FAILED in 0.0s

merry christmas

@realtimetodie
Copy link
Contributor

Fixed an upstream bug, see #88

@realtimetodie
Copy link
Contributor

The tests are now passing 🎉

@realtimetodie
Copy link
Contributor

#61

@alexeagle
Copy link
Member Author

hey @realtimetodie this is fantastic, I'm watching those upstream PRs.
In theory we could use rules_rust to allow a from-source swc toolchain here, then users could point to an unreleased branch of swc (or they could vendor swc sources into their monorepo so they can make local modifications, though that's essentially a fork)
That's probably not worth it though, the upstream maintainers are active and responsive so I bet you'll be able to get an official swc release that works 100% with Bazel.

@realtimetodie
Copy link
Contributor

Yes, the swc team is very responsive. The changes for symbolic links were merged upstream.

@realtimetodie
Copy link
Contributor

  • I refactored repositories.bzl. The binaries are now downloaded from GitHub releases instead from npm.

  • Node.js: All remaining rules_nodejs and npm artifacts were removed. Now, Node.js is only required to run some of the examples.

  • In addition, the rule accepts a dictionary to dynamically generate the swrc configuration file.

    I added tests to validate that the generated configuration works

    https://github.com/realtimetodie/rules_swc/tree/swc_rust/examples/rcdict

  • While doing all this, I oriented myself on the existing aspect-build rules and followed the general coding style.

  • Upgrade: The swc macro is unchanged 🎉 and users do need to upgrade their targets.

As this is a breaking change, I used the opportunity to do some house cleaning (please bear with me).
I renamed the underlying rule from swc_transpile -> swc_compile. This matches the wording of the SWC project (I think the word "transpile" was originally inspired by the Node-based TypeScript compiler, but this is different in the SWC world).

Checklist

  • just mirror the rust CLI, no npm packages
  • allow integrity_hashes={"darwin-arm64": "sha512-DuB... to be passed along with version number in WORKSPACE so users don't have to rely on our mirroring
  • look for .swcrc in the package and use it if present, like we do for tsconfig.json in ts_project
  • maybe the swcrc file becomes required to make the rust CLI work. Accept a dictionary to dynamically generate it, just like we do with tsconfig.json
  • get all the tests passing again, and document the breaking changes that were required

Breaking changes will be documented in the final PR.

Blocked by swc-project/swc#6714

@alexeagle
Copy link
Member Author

@realtimetodie SWC just cut a release that includes your last fix, so I think it's time to merge this!

@CLAassistant
Copy link

CLAassistant commented Jan 6, 2023

CLA assistant check
All committers have signed the CLA.

swc/private/swc.bzl Outdated Show resolved Hide resolved
@alexeagle alexeagle marked this pull request as ready for review January 6, 2023 18:51
@realtimetodie
Copy link
Contributor

The solution to obtain the bazel runfiles directory as an environment variable is very elegant, I like it

swc/defs.bzl Outdated Show resolved Hide resolved
Copy link
Member

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💚

@alexeagle
Copy link
Member Author

look for .swcrc in the package and use it if present, like we do for tsconfig.json in ts_project

This doesn't actually seem to work, if I drop the swcrc = ".swcrc" attribute from an swc_compile then it stops using the config file. I'll fix in a follow-up, doesn't really have to be in-scope here as it wasn't a feature before either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants