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

Have a "facade" top-level crate with Cargo features for CEF and Gonk #5973

Closed
SimonSapin opened this issue May 7, 2015 · 11 comments
Closed

Have a "facade" top-level crate with Cargo features for CEF and Gonk #5973

SimonSapin opened this issue May 7, 2015 · 11 comments

Comments

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented May 7, 2015

Currently we have three "top-level" crates, each with their (largely duplicated) Cargo.lock file and target directory: components/servo, ports/cef, and ports/gonk.

Could we take advantage of Cargo features instead, to switch between these kinds of builds? That way we could have a single top-level crate (and so a single, shared Cargo.lock file and target directory). Maybe it would be a "facade" crate that mostly contains pub use re-exports conditioned by #[cfg] attributes.

CC @larsbergstrom

@jdm jdm added the A-infrastructure label May 7, 2015
@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented May 7, 2015

Even better, we could remove gonk and servo in favor of the CEF port, and use the CEF port as the facade.

@SimonSapin
Copy link
Member Author

@SimonSapin SimonSapin commented May 7, 2015

That would require writing two CEF “clients” that don’t exist yet, right?

@pcwalton
Copy link
Contributor

@pcwalton pcwalton commented May 7, 2015

Yes, but hopefully they'd be pretty similar if not identical. CEF has a simple API for the common case of "open a full screen or windowed chromeless window displaying a URI", which I believe is all that components/servo and ports/gonk do.

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Jul 21, 2015

I suspect we'll want to let clients choose whether to use CEF or the raw Rust embedding API directly, if their final binary is in Rust and they'd prefer not to do a bunch of C shimming. Certainly, gonk prefers that - https://github.com/servo/servo/wiki/Mozlandia-B2S#use-cef

@SimonSapin
Copy link
Member Author

@SimonSapin SimonSapin commented Jan 5, 2016

@bholley, you asked in https://github.com/servo/servo/wiki/Meeting-2016-01-04 about dealing with the build system to add a new build target. The precedent we have is CEF and Gonk, that each have their own top-level Cargo.toml file and therefore their own Cargo.lock file. I do not think you should repeat this. Instead, I’d rather stay with a single top-level crate (with it’s Cargo.toml and Cargo.lock files), and use Cargo’s "features" mechanism to make it do different things. Maybe leave components/servo mostly as it is (or rename it) and have a new top-level crate that only does conditional compilation.

(This is only my opinion, maybe other people have other ideas.)

@alexcrichton Can we change crate-type (executable vs dynamic lib vs static lib) based on Cargo features?

@SimonSapin
Copy link
Member Author

@SimonSapin SimonSapin commented Jan 5, 2016

CC @metajack (see previous comment)

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Jan 5, 2016

Note that cargo-feature-specific deps don't get entries in the lockfile. We can circumvent that with version pinning in the toml.

@SimonSapin
Copy link
Member Author

@SimonSapin SimonSapin commented Jan 5, 2016

@Manishearth it looks like they do:

/tmp$ cargo new aa
/tmp$ cd aa
/tmp/aa$ $EDITOR Cargo.toml
[package]
name = "aa"
version = "0.1.0"
authors = []

[dependencies]
matches = {version = "*", optional = true}

[features]
foo = ["matches"]
/tmp/aa$ cargo build
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling aa v0.1.0 (file:///tmp/aa)
/tmp/aa$ cat Cargo.lock 
[root]
name = "aa"
version = "0.1.0"
dependencies = [
 "matches 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "matches"
version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"

/tmp/aa$ cargo build --features foo
   Compiling matches v0.1.2
   Compiling aa v0.1.0 (file:///tmp/aa)
@Manishearth
Copy link
Member

@Manishearth Manishearth commented Jan 5, 2016

Huh. Perhaps that got fixed then

@alexcrichton
Copy link
Contributor

@alexcrichton alexcrichton commented Jan 11, 2016

@SimonSapin unfortunately, no, the crate-type cannot change based on features

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Jul 14, 2017

Now that cargo workspaces exist and we've moved to them, this is no longer relevant.

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

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.