Allow examples to be library #3556

KalitaAlexey · 2017-01-17T19:31:07Z

This PR allows to specify crate-type to an example in Cargo.toml.
After this PR an example's crate-type can be:

lib
rlib
dylib
proc-macro

Please look at src/cargo/core/manifest.rs:116 because I am not sure whether I done it right.

I haven't added any tests.
I'd like to add them if someone says me how to do that.

Fixes #2358.

…w-examples-to-be-library

rust-highfive · 2017-01-17T19:31:20Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

alexcrichton · 2017-01-18T00:04:29Z

Looks good to me, thanks! The line with TODO looks ok to me and the failure on Travis seems relevant (just a style issue).

If you take a look at tests/*.rs you can probably just add a new tests to tests/test.rs

KalitaAlexey · 2017-01-18T04:50:42Z

@alexcrichton,
I compiled cargo using cargo build.
Which command should I run to check styles?

I will add some tests.

KalitaAlexey · 2017-01-18T06:46:26Z

I allowed an example to be built as a library.
Should cargo test --example work?

EDIT:
I think it should.
I will make it.

KalitaAlexey · 2017-01-18T07:14:56Z

@alexcrichton,
What should cargo do if I run test --example <some_example_name>?
I checked it with file:

fn main() {
    panic!("");
}

#[test]
fn f() {
    panic!("");
}

It does nothing.
It writes that it compiles and then nothing.

matklad · 2017-01-18T07:48:18Z

Which command should I run to check styles?

./tests/check-style.sh

This just checks for lines longer than 100 characters, nothing fancy.

KalitaAlexey · 2017-01-18T15:33:41Z

@alexcrichton,
Bot failed because of timeout.

alexcrichton · 2017-01-18T22:59:15Z

Thanks @KalitaAlexey! Could you also add assertions that the relevant files exist (e.g. dylibs and such) and also remove the TODO?

KalitaAlexey · 2017-01-19T07:10:19Z

@alexcrichton,
I will try.

KalitaAlexey · 2017-01-19T07:55:31Z

@alexcrichton,
Should cargo test also work?

KalitaAlexey · 2017-01-19T08:14:47Z

I don't know how to check whether a library exists.
I need help of someone who knows.

KalitaAlexey · 2017-01-19T12:23:38Z

I have found out that cargo uses rustc to determine the prefix and the suffix of a library.
I have no idea how I should expose that.

alexcrichton · 2017-01-19T18:48:44Z

@KalitaAlexey you can use the existing_file function to test that a file exists or you could just read the directory and assert some file exists there. I wouldn't worry too much about platform-specific extensions and such, the test can be fairly liberal (e.g. check for any dll, not just the one platform dll)

KalitaAlexey · 2017-01-19T18:58:59Z

@alexcrichton,
Should cargo test --example <example> work if is a library?

Am I right about library prefixes and extensions?

lib, rlib:
- Any platform: prefix: lib, extension: rlib
staticlib:
- Windows: prefix: none, extension: lib
- Not Windows: prefix: lib, extension: a
dylib:
- Windows: prefix: none, extension: dll
- Not Windows: prefix: lib, extension: so

alexcrichton · 2017-01-19T19:02:07Z

It should do whatever it does today. If it runs unit tests then it should run unit test. If it just builds the artifact then it should just build the artifact.

On OSX the dylib extension is "dylib", but other than that the info you have I believe is correct.

KalitaAlexey · 2017-01-19T19:03:16Z

In the PR cargo test --example <example> where <example> points to an example of some kind of library doesn't compile.

Today it doesn't compile because it is treated as a binary.

alexcrichton · 2017-01-19T19:05:33Z

What happens if you point it to an example that's a binary? Libraries should match the same behavior.

KalitaAlexey · 2017-01-19T19:07:37Z

@alexcrichton,
Okay.
It requires changes.
I will update the PR.

KalitaAlexey · 2017-01-19T20:46:50Z

It works.

alexcrichton · 2017-01-19T22:14:13Z

src/cargo/core/manifest.rs

-            TargetKind::Lib(ref kinds) => {
-                kinds.iter().map(|k| k.crate_type()).collect()
+            TargetKind::Lib(ref kinds) |
+            // TODO: I am not sure whether it should be encoded like a library or like an example


(it's ok to remove this now)

alexcrichton · 2017-01-19T22:15:35Z

Looks like there's a failure on Travis as well?

KalitaAlexey · 2017-01-20T05:13:59Z

@alexcrichton,
Why did it fail?
On my machine all tests had completed without failures.

KalitaAlexey · 2017-01-20T06:54:20Z

I hope it won't fail again.

alexcrichton · 2017-01-20T18:26:46Z

@bors: r+

Looks good, thanks! I think that rollup PR is for rust-lang/rust, not Cargo, so this can't be included.

bors · 2017-01-20T18:26:47Z

📌 Commit e73e270 has been approved by alexcrichton

bors · 2017-01-20T20:56:45Z

⌛ Testing commit e73e270 with merge de6bce9...

…excrichton Allow examples to be library This PR allows to specify **crate-type** to an example in **Cargo.toml**. After this PR an example's **crate-type** can be: * lib * rlib * dylib * proc-macro Please look at src/cargo/core/manifest.rs:116 because I am not sure whether I done it right. I haven't added any tests. I'd like to add them if someone says me how to do that. Fixes #2358.

bors · 2017-01-20T23:01:19Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing de6bce9 to master...

jethrogb · 2017-01-20T23:48:43Z

Contrary to what's in OP, cdylib and staticlib are now also supported with this PR. Yay!

jethrogb · 2017-01-20T23:55:36Z

Follow up issue in #3572

KalitaAlexey · 2017-01-23T07:39:36Z

@alexcrichton,
I want to change http://doc.crates.io/guide.html
because it contains "Example executable files go in the examples directory.".
It should be "Example executable or library files go in the examples directory".
How can I change that?

alexcrichton · 2017-01-23T17:08:46Z

@KalitaAlexey the docs are in src/doc in this repo, as I think you've found!

…es, r=alexcrichton Added information about examples as libraries to the documentation [Allow examples to be library](#3556) has been committed. The PR adds information to the documentation.

ehuss · 2017-02-06T01:35:26Z

I have a minor problem with this change. cargo metadata now outputs "kind": ["staticlib"] instead of "kind": ["example"] when you have an example library. I'm working on a tool that uses cargo metadata to determine the command-line arguments to use to build a specific target. Since there is no longer an indication that the target is an example, it is impossible to know that you need to use --example to build. Should I maybe open a new issue?

alexcrichton · 2017-02-06T01:51:21Z

@ehuss yeah want to open a new issue?

KalitaAlexey · 2017-02-06T04:19:20Z

Hi @ehuss,
I am sorry about that. When I was working on it I didn't know what it was for.

alexcrichton · 2017-02-06T23:38:47Z

@KalitaAlexey since we're seeing some breakage want to send a PR to revert back to the original behavior? We can probably add a new field to the metadata which indicates whether an example is a library or not.

KalitaAlexey · 2017-02-07T04:01:27Z

@alexcrichton,
I will send a PR today (in 3 hours).

KalitaAlexey added 2 commits January 17, 2017 22:23

Allow examples to be libraries

47b25fd

Merge branch 'master' of https://github.com/rust-lang/cargo into allo…

cc07450

…w-examples-to-be-library

rust-highfive assigned alexcrichton Jan 17, 2017

KalitaAlexey mentioned this pull request Jan 18, 2017

Describe in cargo test --help what cargo test --example <example> does #3559

Closed

KalitaAlexey added 2 commits January 18, 2017 10:56

Fixed style issues

c0fefb5

Added tests for building an example as a library

446985c

Fixed tests

b66b0b7

alexcrichton reviewed Jan 19, 2017

View reviewed changes

Fixed tests and removed the unnecessary comment

7e5b6da

Fixed the test

865e0e5

Merge branch 'master' into allow-examples-to-be-library

e73e270

bors merged commit e73e270 into rust-lang:master Jan 20, 2017

KalitaAlexey deleted the allow-examples-to-be-library branch January 21, 2017 07:39

KalitaAlexey mentioned this pull request Jan 23, 2017

Added information about examples as libraries to the documentation #3582

Merged

ehuss mentioned this pull request Feb 6, 2017

"cargo metadata" no longer displays "example" kind if example specifies crate-type #3654

Closed

ehuss mentioned this pull request Mar 12, 2017

Reverted the "kind" field to the previous value. #3668

Merged

ehuss added this to the 1.16.0 milestone Feb 6, 2022

Allow examples to be library #3556

Allow examples to be library #3556

Conversation

KalitaAlexey commented Jan 17, 2017 • edited Loading

rust-highfive commented Jan 17, 2017

alexcrichton commented Jan 18, 2017

KalitaAlexey commented Jan 18, 2017

KalitaAlexey commented Jan 18, 2017 • edited Loading

KalitaAlexey commented Jan 18, 2017

matklad commented Jan 18, 2017

KalitaAlexey commented Jan 18, 2017

alexcrichton commented Jan 18, 2017

KalitaAlexey commented Jan 19, 2017

KalitaAlexey commented Jan 19, 2017

KalitaAlexey commented Jan 19, 2017 • edited Loading

KalitaAlexey commented Jan 19, 2017 • edited Loading

alexcrichton commented Jan 19, 2017

KalitaAlexey commented Jan 19, 2017

alexcrichton commented Jan 19, 2017

KalitaAlexey commented Jan 19, 2017 • edited Loading

alexcrichton commented Jan 19, 2017

KalitaAlexey commented Jan 19, 2017

KalitaAlexey commented Jan 19, 2017

alexcrichton Jan 19, 2017

Choose a reason for hiding this comment

alexcrichton commented Jan 19, 2017

KalitaAlexey commented Jan 20, 2017

KalitaAlexey commented Jan 20, 2017

alexcrichton commented Jan 20, 2017

bors commented Jan 20, 2017

bors commented Jan 20, 2017

bors commented Jan 20, 2017

jethrogb commented Jan 20, 2017

jethrogb commented Jan 20, 2017

KalitaAlexey commented Jan 23, 2017

alexcrichton commented Jan 23, 2017

ehuss commented Feb 6, 2017

alexcrichton commented Feb 6, 2017

KalitaAlexey commented Feb 6, 2017

alexcrichton commented Feb 6, 2017

KalitaAlexey commented Feb 7, 2017

KalitaAlexey commented Jan 17, 2017 •

edited

Loading

KalitaAlexey commented Jan 18, 2017 •

edited

Loading

KalitaAlexey commented Jan 19, 2017 •

edited

Loading

KalitaAlexey commented Jan 19, 2017 •

edited

Loading

KalitaAlexey commented Jan 19, 2017 •

edited

Loading