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

Update some libraries #44

Merged
merged 2 commits into from
Dec 1, 2015
Merged

Update some libraries #44

merged 2 commits into from
Dec 1, 2015

Conversation

SimonSapin
Copy link
Member

No description provided.

@vyp
Copy link

vyp commented Dec 1, 2015

Btw, I get:

    Updating registry `https://github.com/rust-lang/crates.io-index`
    Updating git repository `https://github.com/servo/servo`
    Updating git repository `https://github.com/ecoal95/rust-offscreen-rendering-context`
    Updating git repository `https://github.com/servo/rust-layers`
    Updating git repository `https://github.com/servo/ipc-channel`
    Updating git repository `https://github.com/pcwalton/gaol`
    Updating git repository `https://github.com/servo/glutin`
no matching package named `glutin` found (required by `offscreen_gl_context`)
location searched: https://github.com/servo/glutin?branch=servo
version required: *

after doing cargo build for this package. @SimonSapin Does this PR fix that?

@SimonSapin
Copy link
Member Author

Yes.

That error is a consequence of git commit hashes not having something equivalent to SemVer. Since Servo’s glutin fork diverges from upstream, I’ve had to rename it to servo-glutin to publish it on crates.io. This is a breaking change, but Cargo has no way of recognizing breaking commits in a git dependency. (Existing builds with a Cargo.lock file will be fine.)

Until this PR is merge, you can “downgrade” your servo/glutin repo dependency to before the rename:

cargo update -p glutin --precise f04bc869a37752d3d4a4258660428a360588e3a7

@vyp
Copy link

vyp commented Dec 1, 2015

@SimonSapin Ah I see, thanks for the explanation. Much appreciated!

emilio added a commit that referenced this pull request Dec 1, 2015
Update some libraries
@emilio emilio merged commit 483abed into servo:master Dec 1, 2015
@SimonSapin SimonSapin deleted the libs branch December 1, 2015 08:55
@vyp
Copy link

vyp commented Dec 1, 2015

Hmm, so now after this merge when I do a cargo build I get:

    Updating git repository `https://github.com/servo/rust-layers`
    Updating registry `https://github.com/rust-lang/crates.io-index`
    Updating git repository `https://github.com/servo/rust-azure`
    Updating git repository `https://github.com/servo/rust-freetype`
    Updating git repository `https://github.com/servo/core-text-rs`
   Compiling serde_codegen v0.6.5
   Compiling rustc-serialize v0.3.16
   Compiling libc v0.2.2
   Compiling pkg-config v0.3.6
   Compiling khronos_api v1.0.0
   Compiling winapi v0.2.5
   Compiling winapi-build v0.1.1
   Compiling quasi v0.3.8
   Compiling bitflags v0.3.3
   Compiling aster v0.8.0
   Compiling log v0.3.4
   Compiling x11 v2.3.0
   Compiling xml-rs v0.2.2
   Compiling advapi32-sys v0.1.2
   Compiling quasi_codegen v0.3.9
/home/u/.cargo/registry/src/git.luolix.top-0a35038f75765ae4/quasi_codegen-0.3.9/src/lib.rs:25:1: 25:27 error: can't find crate for `rustc_plugin` [E0463]
/home/u/.cargo/registry/src/git.luolix.top-0a35038f75765ae4/quasi_codegen-0.3.9/src/lib.rs:25 extern crate rustc_plugin;
                                                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error
   Compiling rand v0.3.12
Build failed, waiting for other jobs to finish...
Could not compile `quasi_codegen`.

To learn more, run the command again with --verbose.

However when I do:

  1. git clone https://github.com/serde-rs/quasi
  2. cd quasi/quasi_codegen
  3. cargo build

it builds fine. So I'm not sure what's going wrong... do you get the same error? N.B. serde-deprecated/quasi#22.

@SimonSapin
Copy link
Member Author

Try cargo update?

@SimonSapin
Copy link
Member Author

Sorry, misread your error message. Try updating your Rust nightly

@vyp
Copy link

vyp commented Dec 2, 2015

@SimonSapin That worked, thanks! But doesn't this mean that https://github.com/servo/servo/blob/master/rust-snapshot-hash should be updated? Because when I tried the above, that's the version of rustc that I had installed (i.e. 1805bba399805d8fd8e85e23c31a0580f21533cb). (Now I have updated to latest master, 69b2fce7bb2283ceb950f786c15cbbcd50787c8e, which is the one that works.)

@vyp
Copy link

vyp commented Dec 2, 2015

Also, while I can build this package in particular with 69b2fce, I can't build servo with it (rust compilation errors), whereas I could build servo with 1805bba. So is there any solution to that? Am I doing things wrong?

By 'build servo', I mean:

  1. git clone https://github.com/servo/servo
  2. cd servo/components/servo
  3. cargo build --lib

Sorry to spam here, not sure where else to ask..

@jdm
Copy link
Member

jdm commented Dec 2, 2015

We use ./mach build, which sets up a bunch of environment variables and grabs supported snapshots of both cargo and rust. Any build that doesn't use that is basically unsupported (regardless of whether it works or not).

@vyp
Copy link

vyp commented Dec 2, 2015

@jdm oh right, even for the library only? Because then how would one use ./mach build for the servo library from an application?

@vyp
Copy link

vyp commented Dec 2, 2015

@jdm Because see here: https://github.com/meh/miserve/blob/18bf3ca945cffa8b6f97f088f04c918e741f7dd9/Cargo.toml#L20-L22

So I figured it would work if I install the appropriate rustc version from the rust-snapshot-hash file and then cargo build (from miserve)?

@jdm
Copy link
Member

jdm commented Dec 2, 2015

AFAIK @meh is the only one who has experimented with that, so they're in a better position to answer questions.

@vyp
Copy link

vyp commented Dec 2, 2015

@jdm Ah right I see, thanks. We are actually in contact, he hasn't had time to update miserve yet (no rush @meh). In the meantime, I'm trying to see what I can do to update miserve a bit. Of course I do understand to build/run the servo binary itself, ./mach build and ./mach run should be used.

@SimonSapin
Copy link
Member Author

Servo’s rust-snapshot-hash should works together with components/servo/Cargo.lock which lock all dependencies to versions that are known to work with that version of the compiler.

So far, I think we haven’t considered much the case of using Servo as a Rust library directly. Using many libraries that use unstable language features makes this kinda hard. I don’t know what we can do to support it better.

Perhaps CEF is the answer: compile Servo as one big library with a C-compatible API, and the Rust version it uses becomes irrelevant. If the program that uses it also uses Rust, it doesn’t have to be the same version. There could be a Rust wrapper around that C-compatible API on the "client" side to make it more convenient to use. (That last part doesn’t exist yet.)

@meh
Copy link

meh commented Dec 2, 2015

If I recall correctly the trick I used was copying the Cargo.lock from components/servo into the project's root and then running a cargo build.

@vyp
Copy link

vyp commented Dec 4, 2015

@SimonSapin That makes sense, thanks for the clarification. Last time I tried, the rust-snapshot-hash version 1805bba does work with components/servo/Cargo.lock, so my concern above about requiring two different versions of rustc can be dismissed.

The whole reason I've come here in the first place is because I couldn't cargo build or cargo update from miserve because it errored out while updating git repositories. For example, servo/rust-layers#217. I got that error first from miserve, so I tried just doing it from rust-layers, and got the same error so I opened that issue there. When it was fixed, cargo build from miserve didn't error out there anymore. But then it had the same no package named glutin found error that I posted above here. When this PR was merged, this error also went away.

Anyway, so currently it seems as though all such naming errors have been fixed because cargo build from miserve does get to the compilation stage, so it's all good.

Yes perhaps CEF is the more reliable solution, thanks for suggesting! But I guess just using the same version of rust that servo uses also works for now.

@SimonSapin
Copy link
Member Author

Yeah, these renames involved changes across repositories that work together, but didn’t all got merged at the same time.

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.

5 participants