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

cargo should be the main build frontend #2608

Closed
ry opened this issue Jul 2, 2019 · 0 comments · Fixed by #2865

Comments

@ry
Copy link
Collaborator

commented Jul 2, 2019

Background: Deno has a relatively complex build system. It involves building various programs, running them, and using their output in further steps (V8 snapshots). We use the GN/Ninja build system to define this both because it solves these tasks well and because V8 uses it. Over time we have integrated more tightly with the Rust ecosystem by making "cargo check", "cargo build", and "cargo publish" work - but only superficially. When you run "cargo build", it calls into GN/Ninja for the bulk of the work, and only Rust compilation and final linking is done by cargo. The default build frontend is tools/build.py, which calls GN/Ninja directly. build.py is what we develop with and what we use to produce the release. When you call build.py, the rust is compiled and linked by our code.

We could simplify our code base and better integrate with the Rust ecosystem, if we could remove the GN/Ninja/build.py frontend and instead only use "cargo build". This is easier said than done. I'm starting this issue to track progress towards this eventual goal.

Problems:

  • "cargo build" does not properly do incremental builds when a file such as //js/main.ts is modified. #2609
  • nowhere in the CI do we run the tests after "cargo build"
  • hyper_hello is not built by cargo #2641
  • "cargo test" does not work without having tools/http_server.py running in the background.
  • "cargo build" (debug mode) does not work on windows due to this
  • the snapshot creator program should probably be in rust #2617
  • Use proc macros for the snapshot #2711

I've started a project called cargo_gn to better abstract this integration... But it might make sense to first iterate on the more deno-specific tools/gn.rs first.

When complete:

@ry ry added the build label Jul 2, 2019

ry added a commit to ry/deno that referenced this issue Jul 12, 2019
Remove Deno.build.args feature
This is a minor feature which complicates the build signifigantly.
Removing to ease refactoring the build system:
denoland#2608
ry added a commit to ry/deno that referenced this issue Jul 17, 2019
Remove Deno.build.args feature
This is a minor feature which complicates the build signifigantly.
Removing to ease refactoring the build system:
denoland#2608
ry added a commit that referenced this issue Jul 22, 2019
Remove Deno.build.args feature
This is a minor feature which complicates the build signifigantly.
Removing to ease refactoring the build system:
#2608
ry added a commit to ry/deno that referenced this issue Aug 5, 2019
Remove Deno.build.args feature
This is a minor feature which complicates the build signifigantly.
Removing to ease refactoring the build system:
denoland#2608
ry added a commit that referenced this issue Aug 5, 2019
Remove Deno.build.args feature (#2728)
This is a minor feature which complicates the build signifigantly.
Removing to ease refactoring the build system:
#2608
ry added a commit to ry/deno that referenced this issue Aug 20, 2019
Remove flatbuffers compile support
This commit checks into the repo the two generated files
  cli/msg_generated.rs
  js/msg_generated.js
And removes support for automatically building them from the build.

This is further simplifications in order to allow the cli crate to build
without gn/ninja. See denoland#2608.
ry added a commit to ry/deno that referenced this issue Aug 20, 2019
Remove flatbuffers compile support
This commit checks into the repo the two generated files
  cli/msg_generated.rs
  js/msg_generated.js
And removes support for automatically building them from the build.

This is further simplifications in order to allow the cli crate to build
without gn/ninja. See denoland#2608.
ry added a commit to ry/deno that referenced this issue Aug 28, 2019
ry added a commit to ry/deno that referenced this issue Aug 28, 2019
Remove @stardazed/streams
This is a regression on several some features in the fetch API. To bring
these back @stardazed/streams simply needs to be ported to TS and
included in the //js directory.

Towards denoland#2608
ry added a commit to ry/deno that referenced this issue Aug 28, 2019
Remove @stardazed/streams
This is a regression on several some features in the fetch API. To bring
these back @stardazed/streams simply needs to be ported to TS and
included in the //js directory.

Towards denoland#2608
ry added a commit to ry/deno that referenced this issue Aug 28, 2019
Remove @stardazed/streams
This is a regression on several some features in the fetch API. To bring
these back @stardazed/streams simply needs to be ported to TS and
included in the //js directory.

Towards denoland#2608
ry added a commit to ry/deno that referenced this issue Aug 28, 2019
Remove @stardazed/streams
This is a regression on several some features in the fetch API. To bring
these back @stardazed/streams simply needs to be ported to TS and
included in the //js directory.

Towards denoland#2608
ry added a commit that referenced this issue Aug 28, 2019
Remove @stardazed/streams
This is a regression on several some features in the fetch API. To bring
these back @stardazed/streams simply needs to be ported to TS and
included in the //js directory.

Towards #2608

@ry ry closed this in #2865 Sep 7, 2019

Tzikas added a commit to Tzikas/squares that referenced this issue Sep 10, 2019
Remove Deno.build.args feature (#2728)
This is a minor feature which complicates the build signifigantly.
Removing to ease refactoring the build system:
denoland/deno#2608
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
You can’t perform that action at this time.