-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Use Cargo.toml to fetch cargo crates #370
Changes from all commits
1b69edd
c64fb9b
165f19a
37c4080
5831e01
e232319
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
[package] | ||
name = "deno" | ||
version = "0.1.0" | ||
authors = "Ryan Dahl <[email protected]>" | ||
|
||
[dependencies] | ||
libc = "0.2.42" | ||
url = "1.7.1" | ||
matches = "0.1.6" | ||
unicode-bidi = "0.3.4" | ||
unicode-normalization = "0.1.7" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only libc and url are necessary - the others are transitive deps. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,30 +36,7 @@ | |
'flatbuffers' | ||
}, { | ||
'url': | ||
'https://github.com/rust-lang/libc.git@8a85d662b90c14d458bc4ae9521a05564e20d7ae', | ||
'https://github.com/avakar/pytoml.git@cb92445ca769b3966b3976cd69f5140761f15843', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not very keen on adding pytoml as a dependency. |
||
'name': | ||
'rust_crates/libc' | ||
}, { | ||
'url': | ||
'https://github.com/servo/rust-url.git@fbe5e50316105482dcd53d2dabb148c445a5f4cd', | ||
'name': | ||
'rust_crates/url' | ||
}, { | ||
# Needed for url. | ||
'url': | ||
'https://github.com/SimonSapin/rust-std-candidates.git@88a017b79ea146d6fde389c96982fc7518ba98bf', | ||
'name': | ||
'rust_crates/rust-std-candidates' | ||
}, { | ||
# Needed for url. | ||
'url': | ||
'https://github.com/servo/unicode-bidi.git@32c81729db0ac90289ebeca9e0d4886f264e724d', | ||
'name': | ||
'rust_crates/unicode-bidi' | ||
}, { | ||
# Needed for url. | ||
'url': | ||
'https://github.com/behnam/rust-unicode-normalization.git@3898e77b110246cb7243bf29b896c58d8975304a', | ||
'name': | ||
'rust_crates/unicode-normalization' | ||
'pytoml' | ||
}] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
#!/usr/bin/env python | ||
# Inspired by | ||
# https://fuchsia.googlesource.com/build/+/master/rust/list_3p_crates.py | ||
# https://fuchsia.googlesource.com/build/+/master/rust/compile_3p_crates.py | ||
# Copyright 2018 The Fuchsia Authors. All rights reserved. | ||
# Use of this source code is governed by a BSD-style license that can be | ||
# found in the LICENSE file. | ||
|
||
import os | ||
from os.path import join, dirname, realpath | ||
import sys | ||
import subprocess | ||
|
||
root_path = dirname(dirname(realpath(__file__))) | ||
third_party_path = join(root_path, "third_party") | ||
|
||
sys.path += [join(third_party_path, "pytoml")] | ||
import pytoml | ||
|
||
def run_command(args, env, cwd): | ||
job = subprocess.Popen(args, env=env, cwd=cwd, stdout=subprocess.PIPE, | ||
stderr=subprocess.PIPE) | ||
stdout, stderr = job.communicate() | ||
return (job.returncode, stdout, stderr) | ||
|
||
def clone_crate(name, version): | ||
rust_crates_path = join(third_party_path, "rust_crates") | ||
if not os.path.exists(rust_crates_path): | ||
os.makedirs(rust_crates_path) | ||
call_args = [ | ||
"cargo", "clone", | ||
"%s:%s" % (name, version) | ||
] | ||
env = os.environ.copy() | ||
_, _, stderr = run_command( | ||
call_args, | ||
env, | ||
join(third_party_path, "rust_crates") | ||
) | ||
print(stderr) | ||
|
||
def main(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is all of this really necessary? I haven't looked into it - but it would be very much preferable to not introduce an alternate custom Cargo.toml parser. The idea of using Cargo is to simply the updating of packages - it will only be used when someone adds a dependency. I was hoping we could just do something like (i have not researched this - this certainly does not work)
I'm not sure it's worth the complexity of maintaining this program. I would rather just manually maintain the dependencies. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I laid out the issues here: https://github.com/ry/deno/issues/366 They are trying to add it to vanilla cargo here: rust-lang/cargo#1861 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TBH, everything would be much easier if we separated the Rust build from the libdeno build. Cargo is very opinionated in regards to project structure, and most of the Rust tooling expects a certain structure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like they want to improve Cargo in that regard: rust-lang/rfcs#2136 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah thanks, I missed those comments.
We should do that but not yet...
For these reasons it makes more sense to treat the rust code as a native code library that we link to, rather than the system that drives compilation. As things solidify and we have more rust code, it will make sense to separate things out more... but I'm happy with the rust code being in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ry I can take a stab at writing the Cargo subcommand as you suggested, I'll update this PR when I have it ready... but my Rust-fu is rusty! (ba dum tss) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @f-a-a I threw something together that basically does what we want: https://github.com/robbym/cargo-download-deps I'm probably missing a bunch of edge cases, but its mostly proof of concept. The toml needs some dummy stuff: [package]
name = "dummy"
version = "0.1.0" And also an empty ./src/lib.rs Dependencies can be added as usual: [dependencies]
url = {git = "https://github.com/servo/rust-url.git", rev = "2efa106"} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @robbym i'd be happy to contribute/maintain to your repo, also would you consider supporting the semantics @ry suggest? (i.e. p/s: let me see if i can amend this PR with your cargo subcommand There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @f-a-a Yeah I'm fine with any modifications. I don't really want to own or maintain this. Was just an example. Edit: Changed to use named args. |
||
cargo_toml_path = join(third_party_path, "Cargo.toml") | ||
with open(cargo_toml_path, "r") as file: | ||
cargo_toml = pytoml.load(file) | ||
for key, value in cargo_toml["dependencies"].items(): | ||
if type(value) is dict: | ||
git = value.get("git") | ||
clone_crate(key, git) | ||
else: | ||
clone_crate(key, value) | ||
|
||
if __name__ == '__main__': | ||
sys.exit(main()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be part of build_third_party.py. It's the analog of yarn for rust - and yarn is called there.