Mix feature flags into fingerprint/metadata shorthash #3102

nipunn1313 · 2016-09-19T08:05:42Z

Since building dependencies results in different libraries
depending on the feature flags, I added the feature flags into
the short_hash.

This solves an issue when multiple crates share a target directory
or multiple targets share a common library with divergent feature
flag choice.

I'm not sure if this architecturally the best way to solve this problem, but I did confirm that this fixes the issue I was seeing. I can also add a test for this case if this code is taking the right approach (or if it would help illustrate the issue).

alexcrichton · 2016-09-29T17:01:01Z

Thanks for the PR! Wanted to let you know that I hadn't forgotten about this and I've just been thinking about it recently. It looks like a test is currently failing but I think it's just because it's expecting a rebuild when one isn't actually happening, so not a hard failure. That to me though is proof enough that this works :)

Could you add a test though which actually runs the output to ensure everything gets properly linked? (just to make sure we don't accidentally use stale artifacts).

I think that this may start inflating the size of build directories over time a bit, but otherwise this seems pretty reasonable to me. I admit that I've run into this problem multiple times before as well and it seems nice to solve it once and for all.

nipunn1313 · 2016-10-03T20:11:22Z

I also wanted to add a couple of tests for this case as well, so I'll take care of that and holler back when updated.

nipunn1313 · 2016-10-05T04:34:53Z

Ok. I'm not confident this is quite doing the right thing for binaries. See my comments on the diff. Looking for some assistance.

nipunn1313

I think I'm misunderstanding something about the tests and the suffix system (and perhaps the compilation process).

nipunn1313 · 2016-10-05T04:38:25Z

src/cargo/ops/cargo_rustc/context.rs

@@ -358,9 +366,18 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                })
            } else {
                None
+                /*


I left this code commented out to highlight a point.
I believe that when building a path dependency, this function returns None in order to not "pollute the suffix" of the executable or the library.

However, this means that when building across features, it appears to build to the same executable/library name.

It's correctly separating the .fingerprint, but not the actual built executable.

Hm yes I can see how this would indeed be a problem! For executables we could generate ones with long filenames and them link them into a location without the metadata, but that does weird things with dynamic libraries unfortunately, so we can't do that everywhere.

Short of building everything in an entirely new directory, we may have to just stomach recompiling these top level dependencies whenever various flags change.

nipunn1313 · 2016-10-05T04:39:22Z

src/cargo/ops/cargo_rustc/fingerprint.rs

+        dir.join(format!("feat-{}", feat_vec.join("-")))
+    } else {
+        dir
+    }


This is correctly separating out the fingerprints, but not the executables.

The executables/libraries are correctly separated as long as they are a full dep, but when they are a path dep, I think this is not working right.

I think this is related to the commented out piece above. One option we could do is to build everything into target/debug/$hash/... where $hash represents feature flags etc. When a build finishes we'd then link everything into target/debug/.... That'd allow multiple top-level executables with the same filename to exist in various locations in this cache at least.

alexcrichton · 2016-10-07T22:29:00Z

tests/freshness.rs

+[RUNNING] `[..]target/debug/b`
+"));
+
+    /* Build a/ package again. If we cache different feature dep builds correctly,


Hm so this test is passing?

If so, I think that may be due to how Cargo tracks where if one target is rebuilt it will unconditionally rebuild all transitive dependants. You can perhaps try to expose this through:

cd a cargo build -p dep_crate cargo run

That'll force Cargo to try to understand that the dependency was rebuilt, maybe? (not sure if that'd fail)

nipunn1313 · 2016-10-18T17:05:57Z

Didn't forget about this! Just got a bit busy.
Would it be viable to also mix in the panic flag (eg panic=unwind vs
panic=abort)?

On Fri, Oct 7, 2016 at 3:29 PM Alex Crichton notifications@github.com
wrote:

@alexcrichton commented on this pull request.

In tests/freshness.rs #3102:
+[..]Compiling dep_crate v0.0.1 ([..])
+[..]Compiling b v0.0.1 ([..])
+[FINISHED] debug [unoptimized + debuginfo] target(s) in [..]
+[RUNNING] [..]target/debug/b
+"));
+
/* Rebuild b/ package. Should not recompile since we just built b */

assert_that(p.cargo("run").cwd(p.root().join("b")),
           execs().with_status(0)
                  .with_stdout("ftest on")
                  .with_stderr("\
+[FINISHED] debug [unoptimized + debuginfo] target(s) in [..]
+[RUNNING] [..]target/debug/b
+"));
+
/* Build a/ package again. If we cache different feature dep builds correctly,
Hm so this test is passing?

If so, I think that may be due to how Cargo tracks where if one target is
rebuilt it will unconditionally rebuild all transitive dependants. You can
perhaps try to expose this through:

cd a
cargo build -p dep_crate
cargo run

That'll force Cargo to try to understand that the dependency was rebuilt,
maybe? (not sure if that'd fail)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#3102, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPXo5tZwxqd6nrU2sylgcTlxZQaSmPIks5qxseugaJpZM4KAOeN
.

alexcrichton · 2016-10-19T17:06:42Z

Yeah if we do take this route I think we'll want to hash in those options as well. I actually just had a chat with @brson as well and we're feeling like we may as well go whole hog on this and hash everything to ensure that bins are distinct. For now that basically means that we'd hash basically an entire Profile in Cargo which has all sorts of compiler configuration. Eventually we could also hash in information like the compiler we're compiling with.

nipunn1313 · 2016-10-22T20:09:56Z

I went through and completed this PR.

Fixed the test issue by cleaning just the parent package (to ensure the dep package rebuilt)
Moved bin targets into the target/debug/deps/ directory with hash suffix.
Hard link bin targets without hash suffix to both target/debug and target/debug/deps
Hard link test targets with a "-test" suffix rather than a hash suffix.
Fingerprint is based on pkg hash, but subdirectory based on target_fingerprint (same logic used for bin suffix generation).
Fix existing tests which expect a non-suffix compile

Stuff I did not do, but could add in

Mix in the entire unit.profile to the hash
Build bin targets into some kind of target/debug/bin/ directory instead of deps?

Stuff I would appreciate some additional scrutiny

Sufficient test coverage? How can we simplify the tests while maximizing test coverage?
Complexity. I fear this diff has gotten a bit complex and some architectural things could make this a lot simpler, but I couldn't wrap my head around the whole codebase.

nipunn1313 · 2016-10-23T04:47:10Z

src/cargo/ops/cargo_rustc/mod.rs

+                     .chain_error(|| {
+                         human(format!("failed to link or copy `{}` to `{}`",
+                                       src.display(), dst.display()))
+                }));


Very interesting. This code is non-deterministically failing due to parallelism + the non-atomicity of the remove_file, hard_link, and fs_copy on the test which is compiling two versions of a crate (use_a_spec_to_select).
Here is the ordering that causes a failure.

thread 1) foo-abcdef -> foo
thread 2) foo-123456 -> foo

Checks if foo exists [no]

Checks if foo exists [no]

hard links foo(inode x) -> foo-abcdef(inode x)

Attempts to hard link foo -> foo-123456 (fails due to foo existing)

Attempts to copy foo-123456 -> foo (succeeds, overwriting inode x and both versions of foo).

Now foo, foo-123456, and foo-abcdef all have the same contents.
We need to somehow prevent either

Multiple versions of the same crate being compiled concurrently

Multiple versions attempting hard link concurrently.

Perhaps linking within the deps/ directory is not necessary and we could try to eliminate this behavior of having a deps/libfoo.rlib. I will try that out.

nipunn1313 · 2016-10-23T04:52:09Z

src/cargo/ops/cargo_rustc/mod.rs

@@ -335,8 +365,7 @@ fn rustc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
                if !src_dir.ends_with("deps") {
                    continue
                }
-                let dst = src_dir.parent().unwrap()
-                                 .join(src.file_name().unwrap());
+                let dst = src_dir.parent().unwrap().join(bin_name);


Might this code below (hard link when doing move_outputs_up) also be susceptible to the same bug? Perhaps not today, since we should only be moving up a top level package, but it seems like a fragility that might cause us to inadvertently add a bug in the future.

One possibility (striving for simplicity over performance) is to only copy and never link.

nipunn1313 · 2016-10-24T01:25:16Z

Ok. I sucked it up and got my windows VM up and running with a developer environment. I narrowed the test issue down to this.

On windows, when producing a dylib with rustc, it produces four files in
target/debug/deps:

builder.dll
builder.dll.exp
builder.dll.lib
builder.pdb

My code modifies this to have
target/debug/deps:

builder-123456.dll
builder-123456.dll.exp
builder-123456.dll.lib
builder-123456.pdb
target/debug
builder.dll
My code only links the "target_filenames" to target/debug/ as specified from the context, which only includes "builder.dll" right now.

When setting as a dep for foo, it tries to link against builder.dll.lib (as seen here https://ci.appveyor.com/project/rust-lang-libs/cargo/build/1.0.44/job/8q6mclw54x5snn9j)

Thus, it fails to link since the target/debug/ directory only has the .dll and not the .dll.lib

Some possibilities, which I don't have enough context to select between =\

Link all four from deps/ up to debug/ by adding to "target_filenames"
Run rustc more than once for the ones which would link upward
Don't put suffixes on dylibs because windows

alexcrichton · 2016-10-26T04:50:43Z

Fingerprint is based on pkg hash, but subdirectory based on target_fingerprint (same logic used for bin suffix generation).

Perhaps we could just base the whole fingerprint on the target fingerprint? E.g. include the whole hash here.

Mix in the entire unit.profile to the hash

Yeah I think this'll give us the most bang for our buck, if it's easy to do let's go ahead and do it.

Build bin targets into some kind of target/debug/bin/ directory instead of deps?

Nah the strategy you outlined above seems fine to me. I'm a little worried about dynamic libraries but I'll read over this and see what's what.

Don't put suffixes on dylibs because windows

Oh geez that's a bummer. I guess this confirms my dylib worries above as well!

So OSX is also weird here where the dylib itself encodes the name it was compiled with. That way if you rename the dynamic library and link to the renamed copy, that binary won't work at runtime because it'll try to find the old name.

In that sense it's probably easiest to just not suffix dylibs with anything and completely recompile them more often (due to these issues).

alexcrichton

Ok, looking good! I've left some comments here or there, but I think I'll need to really sit down to digest this soon (currently sitting in an airport).

If this is getting too complicated I also don't mind taking this over!

alexcrichton · 2016-10-26T04:56:06Z

src/cargo/ops/cargo_rustc/context.rs

@@ -309,49 +309,46 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

    /// Get the metadata for a target in a specific profile
    pub fn target_metadata(&self, unit: &Unit) -> Option<Metadata> {


I think this function should always return Some now, right? It may be useful to just return Metadata here and then deal with the consequences elsewhere as they arise.

target_metadata returns Option. The only way to get None is if target is a dylib or perhaps if unit.target.metadata() returns None. I did not try to track down the latter case.

I think a lot of this code will simplify a lot if metadata was universally applicable. For example, a lot of the special casing of "unit.profile.test" would go away since that is part of the metadata hash.

alexcrichton · 2016-10-26T04:56:59Z

src/cargo/ops/cargo_rustc/context.rs

+            unit.target.crate_name()
+        };
+        if unit.profile.test {
+            bin + "-test"


Hm I'm trying to recall why something like this would be required but I'm drawing a blank. Could you remind me why this is needed here?

Yeah. This was needed to separate test binaries from true binaries. Previously they were separated by metadata hash. The move-targets-up code was linking them on top of each other in this code. One option was to replicate the -{metadata} behavior, but I thought it simpler to just do "-test" instead now that there was a clear way of separating them. I can maintain the existing behavior and separate out this behavioral change into another request.

alexcrichton · 2016-10-26T04:58:02Z

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -82,8 +83,13 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
        missing_outputs = !root.join(unit.target.crate_name())
                               .join("index.html").exists();
    } else {
-        for (filename, _) in try!(cx.target_filenames(unit)) {
-            missing_outputs |= fs::metadata(root.join(filename)).is_err();
+        let move_outputs_up = unit.pkg.package_id() == &cx.current_package || unit.target.is_bin();


This looks like it may want to be extracted to a method on Context?

alexcrichton · 2016-10-26T05:01:12Z

tests/build.rs

@@ -1790,13 +1790,15 @@ fn example_bin_same_name() {
        .unwrap();

    assert_that(&p.bin("foo"), is_not(existing_file()));
+    assert_that(&p.bin("foo-test"), existing_file());


Hm ok so this may be where the -test is showing up.

This looks like a somewhat orthogonal change, however (having predictably named test artifacts). Perhaps we could leave that to a future PR?

nipunn1313 · 2016-10-28T18:09:36Z

Sure I can take another look at it. Comments seem to make sense.

nipunn1313

Did a quick self-review. Let me know what you think.

nipunn1313 · 2016-11-03T02:18:13Z

src/cargo/ops/cargo_rustc/context.rs

+        // No metadata for dylibs because of a couple issues
+        // - OSX encodes the dylib name in the executable
+        // - Windows rustc multiple files of which we can't easily link all of them
+        if !unit.profile.test && unit.target.is_dylib() {


This is a finicky line. I would have thought dylib alone would be sufficient, but then "cargo build" and "cargo test" build to the same target which is not acceptable.

Ah yeah there's tons of special cases here for unit.profile.test, that's expected and ok

Ah so one thing I just remembered though is that we don't want to unconditionally return None here in the case of an upstream dylib. That is, if a transitive dependency is a dylib, we'll want metadata mixed in there to resolve name conflicts.

Additionally, the __CARGO_DEFAULT_LIB_METADATA below I think still needs to be factored in here as well. That's used when building the standard library itself to ensure that we throw a hash in there even though it's a dylib and even though it's a local path dependency.

What are the conditions we want? We want to check if the target is the current_package AND is a dylib?

How to handle the upstream dylib case

I think the conditions we're going to want here are:

if !unit.profile.test && unit.target.is_dylib() && unit.pkg.package_id().source_id().is_path() && magical_env_var_not_set() { return None }

That is, we want metadata on all tests, we want metadata on everything that's not a dylib, we want metdata on all upstream deps (non-path deps), and finally we have this magical env var thing.

Note that "upstream" == "not a path dep" because the current package changes when you move around a workspace. A path dep is basically a heuristic, but it works fine for us.

So for the magical env var, the problem is that if we didn't have that clause in the condition then when we cargo build the standard library we get a bunch of files that are literally libstd.so, librustc.so, etc. Now we don't want this because it means when you install Rust on your system you'd get /usr/lib/libstd.so. That's a little too "official" for us to take! To solve this problem we've always added some form of hash to the libraries, so you'd instead install /usr/lib/libstd-xxxxxxxx.so.

So basically when we use Cargo to build the standard library we want to unconditionally insert all these hashes, so the env var just says "please insert hashes everywhere".

Now if you're curious why we don't always install the hash, that's also interesting. The main idea is that if I have a project called "foo" and I built a dylib, we want to generate the predictable filename target/debug/libfoo.so. (no hash). Now for this to actually work correctly, however, we can't take the same trick we're doing with staticlibs and executables where we just hard link the file to the correct name. Platforms like OSX use the generate file name (target/debug/deps/libfoo-xxxxxx.so) at runtime for various reasons, so we need to generate the file as literally target/debug/deps/libfoo.so for now. We can fix this with linker options but rustc doesn't expose that yet (and is a problem for another day!)

Does that all make sense?

sounds good. Based on that explanation, seems like we should also have a test for this env-var situation. I added a test for this.

nipunn1313 · 2016-11-03T02:18:54Z

src/cargo/ops/cargo_rustc/context.rs

@@ -309,49 +309,46 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

    /// Get the metadata for a target in a specific profile
    pub fn target_metadata(&self, unit: &Unit) -> Option<Metadata> {


target_metadata returns Option. The only way to get None is if target is a dylib or perhaps if unit.target.metadata() returns None. I did not try to track down the latter case.

I think a lot of this code will simplify a lot if metadata was universally applicable. For example, a lot of the special casing of "unit.profile.test" would go away since that is part of the metadata hash.

nipunn1313 · 2016-11-03T02:26:02Z

src/cargo/ops/cargo_rustc/context.rs

-            let mut metadata = unit.pkg.generate_metadata();
-            metadata.mix(&format!("bin-{}", unit.target.name()));
-            Some(metadata)
+            pkg_metadata.mix(&format!("bin-{}", unit.target.name()));


The choice between pkg_metadata and metadata seems quite arbitrary to me and I'm not sure why there are two different constructs. This diff maintains the existing behavior, but I can't figure out why it is the way it is.

nipunn1313 · 2016-11-03T02:26:50Z

src/cargo/ops/cargo_rustc/context.rs

-            // just here for rustbuild. We need a more principled method of
-            // doing this eventually.
-            if unit.target.is_lib() {
-                env::var("__CARGO_DEFAULT_LIB_METADATA").ok().map(|meta| {


I think this env var is no longer necessary since we're always producing metadata on this path now. Where is it used from?

nipunn1313 · 2016-11-03T02:27:57Z

src/cargo/ops/cargo_rustc/context.rs

+        // it was compiled into something like `example/` or `doc/` then
+        // we don't want to link it up.
+        if src_dir.ends_with("deps") {
+            // Don't lift up library dependencies


I noticed that this if/else is untested (if we hit the else every time, all the tests pass). Should we add a test for this condition?

I think that if all tests pass this is fine to just have the else part. I think this came from way down below in cargo_rustc/mod.rs but I can't quite recall exactly what this is doing. Although the doc block on this function may help me refresh my memory :)

I think this if else prevents us from linking up dependent libs.

Eg if crate foo depends on something else (say petgraph)
This if else prevents us from linking target/debug/deps/petgraph-123456.rlib to target/debug/petgraph.rlib. There's no point in linking up deps, though it doesn't hurt.

Oh ok, so the test would be asserting that target/debug/libpetgraph.rlib doesn't exist. In that sense this behavior definitely looks correct to me, and if you want to add a test for that, sounds good to me!

Added a couple lines to an existing test. No prob!

nipunn1313 · 2016-11-03T02:30:27Z

src/cargo/ops/cargo_rustc/context.rs

@@ -429,6 +476,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                   support any of the output crate types",
                  unit.pkg, self.target_triple());
        }
+        info!("Target filenames: {:?}", ret);


I found these info's to be helpful. Let me know if you'd rather leave it in or dump them.

nipunn1313 · 2016-11-03T02:34:47Z

tests/build.rs

@@ -1790,13 +1790,15 @@ fn example_bin_same_name() {
        .unwrap();

    assert_that(&p.bin("foo"), is_not(existing_file()));
+    // We expect a file of the form bin/foo-{metadata_hash}


Currently, it's hard for us to confirm that such a file exists from our tests. We just confirm that the original "foo" doesn't exist. This was the original motivation for the "foo-test" change, but I left this comment here instead.

We could also provide a way to programatically report the metadata or the test binary name to the caller.

Yeah eventually we'll definitely publish the names of these files via --message-format=json (ala #3212)

nipunn1313 · 2016-11-03T04:20:46Z

Hey @alexcrichton. I noticed that the appveyor is getting backed up for hours. Enabling this feature may help reduce the size of the queue when something is under rapid development. Alternately, could upgrade the appveyor account for additional parallelism, but honestly, rolling builds will probably be good enough.

https://www.appveyor.com/docs/build-configuration/#rolling-builds

nipunn1313 · 2016-11-03T06:16:31Z

src/cargo/ops/cargo_rustc/mod.rs

                }));
            }
+            try!(fs::hard_link(&src, &dst)


Do we have to pop this linking step out into a separate task? I'm worried that the task won't re-run under all conditions. For example, if there are two binaries built with different features, it would compile to
deps/bin-abc123
deps/bin-456789
If you then switched back to using the first binary, the fingerprint would indicate that you're all set. Since the linking is happening as part of the rustc stage, there's no way to re-link without recompiling. Any suggestion on how to get around that?

For now, I will put together a test that fails this condition and see if I can get some code to trigger a recompile of the top level package.

Hm, a very good point! Definitely something we'll want to handle. This makes me vaguely wonder how this works today...

IIRC, each unit of work always has something run. There's a unit of work to run if it's dirty and a different one if it's "fresh" (doesn't need to be rerun). The thinking is that "fresh" work is way faster than dirty work.

So I think we can shove this into also happening during the fresh stages to cover this use case?

This works today because the fingerprint filename is the same for both deps. Thus if you change the features and try to recompile, it will trigger as dirty. I changed it to have different fingerprint filename for each of these variants so when switching back and forth we keep it working right.

bors · 2016-11-07T06:35:45Z

☔ The latest upstream changes (presumably #3257) made this pull request unmergeable. Please resolve the merge conflicts.

Since building dependencies results in different libraries depending on the feature flags, I added the feature flags into the short_hash. This solves an issue when multiple crates share a target directory or multiple targets share a common library with divergent feature flag choice. - Only link binaries, build scripts, and top level deps - Handle dylibs differently (no metadata filename / linking) - Fingerprint based on link_dst rather than file_stem. This (sadly) limits the effects of dep caching to things which don't produce a hard-link. Currently, this is only dependent library crates. Stil a big win.

nipunn1313 · 2016-11-09T09:05:02Z

FWIW this has ended up becoming quite the large diff. It could feasibly be broken into two parts

Refactor target_filenames into returning the 3-tuple with link_dst without any semantic changes
Make semantic changes to target_dst, link_dst, fingerprint, and dylib.

If you think it is worthwhile, I'm happy to do it, since it will definitely shorten the diffs. Just want to make sure part 1 is in line with your view before I go for it.

bors · 2016-11-15T23:28:07Z

📌 Commit d7977f8 has been approved by alexcrichton

bors · 2016-11-15T23:28:08Z

⌛ Testing commit d7977f8 with merge 1cf3cba...

bors · 2016-11-16T00:28:47Z

💔 Test failed - cargo-win-gnu-32

alexcrichton · 2016-11-16T02:19:30Z

tests/concurrent.rs

 #[test]
+#[cfg_attr(target_env = "msvc", ignore)]


Ah I guess this needs to be windows, not just MSVC

nipunn1313 · 2016-11-16T06:49:05Z

That seems like it'll cause you to lose test coverage on windows for some code that's specific to windows. But here you go!

alexcrichton · 2016-11-16T07:09:44Z

Ah that's ok, as soon as we ditch buildbot I'll re-enable the test

alexcrichton · 2016-11-16T07:09:50Z

@bors: r+

bors · 2016-11-16T07:09:50Z

📌 Commit 2af9e1f has been approved by alexcrichton

bors · 2016-11-16T07:09:51Z

⌛ Testing commit 2af9e1f with merge e82ad8a...

bors · 2016-11-16T07:19:19Z

💔 Test failed - cargo-win-msvc-32

nipunn1313 · 2016-11-16T07:46:22Z

Whoops. Said target_env = windows instead of target_os = windows. Fixing.

alexcrichton · 2016-11-16T15:02:14Z

@bors: r+

bors · 2016-11-16T15:02:15Z

📌 Commit 8e22eca has been approved by alexcrichton

bors · 2016-11-16T15:02:23Z

⌛ Testing commit 8e22eca with merge 1877f59...

Mix feature flags into fingerprint/metadata shorthash Since building dependencies results in different libraries depending on the feature flags, I added the feature flags into the short_hash. This solves an issue when multiple crates share a target directory or multiple targets share a common library with divergent feature flag choice. I'm not sure if this architecturally the best way to solve this problem, but I did confirm that this fixes the issue I was seeing. I can also add a test for this case if this code is taking the right approach (or if it would help illustrate the issue).

bors · 2016-11-16T16:52:19Z

☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64
Approved by: alexcrichton
Pushing 1877f59 to master...

AFAICT, we do not test on these older platforms anymore. Regardless, the test seems to work fine on 32-bit windows-gnu on Windows 10. See rust-lang#3102 (comment) where it was originally disabled.

alexcrichton mentioned this pull request Oct 3, 2016

Workspace dependencies getting rebuilt when child dependency versions differ #3151

Closed

nipunn1313 force-pushed the attempt branch from 31fa76f to 488e743 Compare October 5, 2016 04:29

nipunn1313 commented Oct 5, 2016

View reviewed changes

alexcrichton reviewed Oct 7, 2016

View reviewed changes

jsgf mentioned this pull request Oct 19, 2016

Export more build metadata from cargo build #3212

Closed

nipunn1313 force-pushed the attempt branch from 488e743 to 1afc35e Compare October 22, 2016 20:01

nipunn1313 commented Oct 23, 2016

View reviewed changes

nipunn1313 force-pushed the attempt branch from 11a5e46 to fd6a84c Compare October 24, 2016 01:18

alexcrichton reviewed Oct 26, 2016

View reviewed changes

alexcrichton mentioned this pull request Nov 2, 2016

Cache build results per feature matrix for dependencies #3245

Closed

nipunn1313 force-pushed the attempt branch from fd6a84c to 668aeee Compare November 3, 2016 01:28

nipunn1313 commented Nov 3, 2016

View reviewed changes

nipunn1313 force-pushed the attempt branch from f6794b2 to 625ae73 Compare November 4, 2016 21:13

nipunn1313 added 2 commits November 7, 2016 17:56

Add tests for feature freshness

14a28af

nipunn1313 force-pushed the attempt branch from 625ae73 to 530f2dd Compare November 7, 2016 09:18

alexcrichton reviewed Nov 16, 2016

View reviewed changes

tests/concurrent.rs

#[test]

#[cfg_attr(target_env = "msvc", ignore)]

Copy link

Member

alexcrichton Nov 16, 2016

Choose a reason for hiding this comment

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

Ah I guess this needs to be windows, not just MSVC

Ignore killing_cargo_releases_the_lock on windows

2af9e1f

Target_env -> Target_os

8e22eca

bors merged commit 8e22eca into rust-lang:master Nov 16, 2016

alexcrichton mentioned this pull request Dec 18, 2016

emscripten support broken by cargo file layout changes rust-lang/rust#38454

Closed

This was referenced Dec 29, 2016

itm fails its tests in Rust 1.15 rust-lang/rust#38684

Closed

Regression in wait-timeout-0.1.13 test suite in Rust 1.15 rust-lang/rust#38721

Closed

Regression in cargo-check 0.2.2 test suite in Rust 1.15 rust-lang/rust#38722

Closed

nipunn1313 deleted the attempt branch August 6, 2021 18:01

ehuss added this to the 1.15.0 milestone Feb 6, 2022

		@@ -309,49 +309,46 @@ impl<'a, 'cfg> Context<'a, 'cfg> {

		/// Get the metadata for a target in a specific profile
		pub fn target_metadata(&self, unit: &Unit) -> Option<Metadata> {

Mix feature flags into fingerprint/metadata shorthash #3102

Mix feature flags into fingerprint/metadata shorthash #3102

Conversation

nipunn1313 commented Sep 19, 2016

alexcrichton commented Sep 29, 2016

nipunn1313 commented Oct 3, 2016

nipunn1313 commented Oct 5, 2016 • edited Loading

nipunn1313 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nipunn1313 commented Oct 18, 2016

@alexcrichton commented on this pull request.

alexcrichton commented Oct 19, 2016

nipunn1313 commented Oct 22, 2016

nipunn1313 Oct 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nipunn1313 commented Oct 24, 2016

alexcrichton commented Oct 26, 2016

alexcrichton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nipunn1313 Oct 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nipunn1313 commented Oct 28, 2016

nipunn1313 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nipunn1313 commented Nov 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Nov 7, 2016

nipunn1313 commented Nov 9, 2016

bors commented Nov 15, 2016

bors commented Nov 15, 2016

bors commented Nov 16, 2016

Choose a reason for hiding this comment

nipunn1313 commented Nov 16, 2016

alexcrichton commented Nov 16, 2016

alexcrichton commented Nov 16, 2016

bors commented Nov 16, 2016

bors commented Nov 16, 2016

bors commented Nov 16, 2016

nipunn1313 commented Nov 16, 2016

alexcrichton commented Nov 16, 2016

bors commented Nov 16, 2016

bors commented Nov 16, 2016

bors commented Nov 16, 2016

nipunn1313 commented Oct 5, 2016 •

edited

Loading

nipunn1313 Oct 23, 2016 •

edited

Loading

nipunn1313 Oct 28, 2016 •

edited

Loading