Join crate-ci org?

[epage]

Would you be willing to join forces and move development of your crate to the crate-ci org?

I think this quote from killercup explains the value of a shared org:

There is no reason why so many popular crates should live in user-repos instead of in community-managed organizations (speaking in Github terms). Writing and then publishing a bunch of code as a crate one thing, but maintaining it, fixing bugs, replying to issues and pull requests, that takes up a lot of time as well. Time, that a lot of developers don't have, or don't want to invest. cargo-edit, for example, which lives under my Github username, has two wonderful maintainers, who are more active than I am. But should I create a cargo-edit organization and move the repo there? If there was a good and definitive answer, which would neither make me deal with the organizational aspects not result in accumulating lot of junk code, I'd be really happy.

You can see more about my goal for this new org on my rust2018 post

We still need to work out the governance model but you can maintain as much control over your crate as you wish.

Specifically for this project, things I think could be nice to cargo-coverage renamed to cargo-kcov to recognize there are multiple ways to gather coverage information and to allow them to be installed.

[epage]

Looks like there is a cargo-kcov. Maybe we can join efforts?

[CAD97]

I can't speak for @roblabla, but as the author of the doc-upload portion of this library, I'd be happy to move that into the crate-ci org under whatever OSI approved license you'd prefer.

I've started to think that it'd be best to split that functionality out, actually. I have at least one project where I'd like to use the upload to GitHub Pages but I don't really want to do coverage (it's highly exploratory right now).

@epage if I break out the doc upload functionality into, say, cargo-gh-upload, would the org accept it? (See also my blog post Great Rust CI.) I'd be happy to help make Rust's CI story more obvious.

[epage]

I've added your blog post to my list of sources to pull from when writing my centralized docs (see crate-ci/crate-ci.github.io#2 ).

That'd be great if you want to get your tool up on the org. Without an org, I can see it being easy to bundle everything up in one repo. My hope was that these things would split up.

So to check my understanding, doc-upload is meant to upload to github pages? And you feel that renaming it to gh-upload would be clearer? Would that be clearer as ghp-upload then? Thats normally the acronym I see for that.

Could you clarify some things for me?

• What is gh-specific about it?
• Why do this compared to the automatic docs we get?
• What does this this get you over Travis' builtin deployment?

[CAD97]

Would that be clearer as ghp-upload then?

Yeah, probably, unless I can make it more generic to where it works with other service providers.

• What is gh-specific about it?

Mainly that I know how to use GitHub Pages.

• Why do this compared to the automatic docs we get?

Docs.rs only generates docs on publish, and only for one configuration of your crate. With the more manual approach, you can generate docs for multiple configurations, branches, and include documentation that isn't just the API docs as well.

Multi-crate workspaces really benefit from workspace doc -- all the crates are linked in the left sidebar [example]. Also search crosses crates that way.

• What does this this get you over Travis' builtin deployment?

Being able to contribute to the docs from multiple branches. If you have (say) two branches, stable and next, and want to upload docs from both, is it possible to do that with the builtin deploy? I don't think so, as it force pushes and gets rid of all history, whereas my script maintains the proper git history.

[epage]

Thanks for the clarifications. Sounds like I have good sound bites for when organizing that part of the documentation :)

[roblabla]

I'd be happy to move the cargo-travis crate to the crate-ci org as well. I read your rust2018 post, and I very much agree with the points outlined \o/. I've been less active than I would have liked, and this crate could use some more love.

Concerning the renaming of the commands, that's a fine idea, but I'm afraid that the potential breaking of people's build might make it problematic. Right now, the recommended way to install cargo-travis is to simply get the latest version at the start of each build. If we remove the cargo-coverage subcommand, everybody's coverage will break. If there's a way to avoid this, that'd be cool. I suppose we could rename the crate to cargo-ci-kcov, and have the current cargo-travis repo issue a warning asking people to migrate to the new one ?

Regarding cargo-kcov, I didn't even know that was a thing. When I started cargo-travis, I opted to depend on cargo directly as a crate to avoid repeats of the travis-cargo breakage. Because of this, it looks like cargo-travis and cargo-kcov are quite a bit different in shape: cargo-kcov runs cargo as a subcommand and parses its out, while cargo-travis pins cargo as a dep and uses its API.

This begs the question of which approach is best. I do not have the answer. Using cargo as a dependency certainly brings several disadvantages: it makes the build slower as it has to recompile cargo, and because of the different cargo version from the host, it will also recompile the project, even if they were previously ran. But it does make the code look saner, as we can simply reuse the relatively clean Cargo API instead of using subcommands and parsing the output.

And thanks for reaching out and championing this crate-ci project :D

[epage]

Because of this, it looks like cargo-travis and cargo-kcov are quite a bit different in shape: cargo-kcov runs cargo as a subcommand and parses its out, while cargo-travis pins cargo as a dep and uses its API.

Is it parsing human readable output or machine formatted metadata?

Human readable is a no-go in my mind. I'm unsure which is best between metadata and depending on cargo.

Concerning the renaming of the commands, that's a fine idea, but I'm afraid that the potential breaking of people's build might make it problematic. Right now, the recommended way to install cargo-travis is to simply get the latest version at the start of each build. If we remove the cargo-coverage subcommand, everybody's coverage will break. If there's a way to avoid this, that'd be cool. I suppose we could rename the crate to cargo-ci-kcov, and have the current cargo-travis repo issue a warning asking people to migrate to the new one ?

Yeah, we'd have to have a new crate name to avoid breaking people. What are your thoughts about our kcov and coveralls being separate crates?

[roblabla]

What are your thoughts about our kcov and coveralls being separate crates?

Well, cargo-coveralls just delegates the coveralls implementation to kcov (it's really just passing the --coveralls flag). So I'm not sure it's such a good idea, as the coveralls subcommand doesn't bring any additional dependencies or whatever. What we could do, however, is merge the coverage and coveralls subcommand into one kcov subcommand, that takes a --coverage flag ?

Is [cargo-kcov] parsing human readable output or machine formatted metadata?

I'm not sure how cargo-kcov works, I only took a very very quick look and saw it didn't have cargo as a dependency. https://github.com/kennytm/cargo-kcov/blob/master/src/target_finder.rs seems to suggest it's trying to parse human output, but kennytm would be better placed to answer this than I am.

[epage]

Well, cargo-coveralls just delegates the coveralls implementation to kcov (it's really just passing the --coveralls flag). So I'm not sure it's such a good idea, as the coveralls subcommand doesn't bring any additional dependencies or whatever. What we could do, however, is merge the coverage and coveralls subcommand into one kcov subcommand, that takes a --coverage flag ?

What are your thoughts on separating the two so that someone can run cargo coveralls on a coverage report from a tool besides kcov?

What I'm thinking of is decoupling how we gather coverage data from what service we upload to. This means we could migrate our recommendation away from kcov but reuse the same upload mechanism. Or say we find another service is better than coveralls and migrate to it, we just change out the upload mechanism.

Maybe thats just not ergonomic enough and we should just merge it all into kcov? We could then make coveralls support a dedicated crate so other coverage reporting tools can reuse it?

[CAD97]

@epage

Let me reiterate this: there is no coveralls specific code in this crate. All we do is tell kcov that the user requested the kcov coverage be uploaded to coveralls. There is no coveralls support we could break out.

As such, I think the hypothetical crate crate-ci/kcov should take a --coveralls flag.

For coverage tools that don't natively support coveralls, then an upload script would be required. For tools that do, such as kcov, the native support should be used.

[epage]

Oh, thanks for clarifying. I thought you meant your cargo-coverage was doing the upload, not kcov itself.

Ok, yeah, I agree about dumping cargo-coveralls.

[CAD97]

pinging @kennytm for cargo-kcov; we shouldn't ignore that work.

On linking cargo versus calling:

• https://doc.rust-lang.org/cargo/reference/external-tools.html (excerpts)

  Information about build

  When passing --message-format=json, Cargo will output the following information during the build:

  • compiler errors and warnings,
  • produced artifacts,
  • results of the build scripts (for example, native dependencies).

  Custom subcommands

  Custom subcommand may use CARGO environment variable to call back to Cargo. Alternatively, it can link to cargo crate as a library, but this approach has drawbacks:

  • Cargo as a library is unstable, API changes without deprecation,
  • versions of Cargo library and Cargo binary may be different.

cargo-kcov says it requires --message-format support in cargo, but I don't actually see a reference to the argument anywhere in the code.

I think that the json of cargo test --no-run --message-format=json is stable (or whatever stable means for json -- no removal, addition only?). And if so, it does indeed provide the necessary information without linking against cargo!

For an example, I ran cargo test --verbose --message-format=json >target/test.stdout 2>target/test.stderr on a cargo cleaned copy of rust-unic, to get a real-world example of what files and thus output are generated from cargo test:

Results gist https://gist.github.com/CAD97/f0e0cc45472544b32ed0b552cf230885

Of note lines:

• stderr ll.350-424

  The artifacts run for testing

• stdout ll.1-220

  The --message-format=json output listing "compiler errors and warnings, produced artifacts, [and] results of the build scripts". (These lines are the only stdout output with --no-run.)

All of the test binaries run are available by looking through the produced artifacts for {"filenames":{["filename"]},"profile":{"test":true},"reason":"compiler-artifact"}.

[kennytm]

@CAD97 ... I forgot why that requirement was written 😂. Probably it was meant to be cargo metadata or something like that.

I prefer not to link to cargo as a library simply because compiling that takes so long. One could also create a cargo_metadata library to abstract out the parsing code if it feels too dirty.

[epage]

I'm fine parsing data meant for machines. Seems reasonable to create a crate for it.

It'd be nice to avoid linking. The build times aren't a concern for me because we should be uploading prebuilt binaries to github and using those in our CIs.

[epage]

So it looks like we are

• Splitting out cargo doc-uploads into a cargo ghp-upload repo on crate-ci
• Merging cargo-travis and cargo-kcov?
• Moving one of the above or the merged into crate-ci

Is this correct? What are the next steps for moving forward? Should we move one of them independent of figuring out the merge? Which?

[CAD97]

For ghp-upload, I've just got to gather the time to pull it out, which should happen this weekend. (And to make sure it practices the best-practices that we're going to be preaching!) Since I'm putting in the work to split it from cargo-travis I also want to do what I can to decouple the Travis-based assumptions baked into the current design as well, relying more on the git interface.

I've got a few ideas that could apply to coverage, mainly on how to parse cargo test --no-run --message-format=json's output, but I'm still not sure whether it's stable.

[epage]

Feel free to pull it out and improve it later :)

[epage]

btw I've now created a gitter channel
https://gitter.im/crate-ci/general

[kennytm]

@CAD97 you may get more definite response if you’ve filed an issue on the cargo repository or posted on internals.rust-lang.org.

[roblabla]

https://users.rust-lang.org/t/how-stable-is-cargos-message-format-json/15662/2 apparently, it's stable @CAD97

[CAD97]

And that's why I went and posted it to users.rust-lang.org :)

[roblabla]

Currently working on adding cargo message-format parsing capabilities to cargo_metadata (have to use it in another project). I'll probably refactor cargo-travis to use that afterwards.

[roblabla]

oli-obk/cargo_metadata#53

[epage]

Is the goal of that PR to make cargo metadata process more than just the cargo-metadata command but the other cargo commands as well?

If so, sounds similar to https://github.com/crate-ci/escargot/

[roblabla]

The goal is to make cargo metadata able to parse the output of --message-format=json. Escargot looks nice, but it has a couple problems for my other use-case (they're all fixable):

1. It only allows cargo, doesn't let you use other wrappers (like xargo)
2. It doesn't stream/pull the output. Instead, it gets the full output, and then iterates on it at the end. This can be problematic for slow commands, as it means the user will have to wait until the command is done to get any output. In my other use-case, I want to stream the errors back to the user in real-time (it doesn't matter quite as much in cargo-travis).
3. It doesn't actually parse anything ? It seems like the structures for a Message isn't actually defined in escargot. In cargo-metadata, I'm trying to add those structs: https://github.com/roblabla/cargo_metadata/blob/8e2f400/src/lib.rs#L296-L369. The user will have a proper rust struct that represents the messages to work with. Escargot seems to offload that work to the user: https://docs.rs/escargot/0.3.1/escargot/struct.Message.html

IMO, the best thing would be for escargot to piggyback cargo-metadata, which would provide the low-level structs, while escargot would provide the high-level APIs.

[epage]

It only allows cargo, doesn't let you use other wrappers (like xargo)

I'm curious, how do you plan to support xargo?

It doesn't stream/pull the output. Instead, it gets the full output, and then iterates on it at the end. This can be problematic for slow commands, as it means the user will have to wait until the command is done to get any output. In my other use-case, I want to stream the errors back to the user in real-time (it doesn't matter quite as much in cargo-travis).

Would love an issue / PR for this :). Granted, if this isn't fixed by the time I'm integrating escargot into stager, then I'll probably be implementing it myself.

It doesn't actually parse anything ? It seems like the structures for a Message isn't actually defined in escargot. In cargo-metadata, I'm trying to add those structs: https://github.com/roblabla/cargo_metadata/blob/8e2f400/src/lib.rs#L296-L369. The user will have a proper rust struct that represents the messages to work with. Escargot seems to offload that work to the user: https://docs.rs/escargot/0.3.1/escargot/struct.Message.html

As I also just posted in oli-obk/cargo_metadata#45

The reason for escargot does not include struct definitions is to ensure clients have the control they need for reducing overhead.

• Clients can choose their allocation policy (Cow / &str vs String)
• Clients can ignore deserializing any parts of the payload they don't care about.

Granted, its probably not worth it with all the other overhead in these kind of operations.

[roblabla]

The parse_message_stream function in cargo_metadata just takes an io::Stream, so the user can just run xargo instead of cargo, and pass command.stdout themselves.

I could probably PR a polling system into escargot if I have some time. I'll make an issue when I'm not on the phone.

As for the parsing, I feel like this is super premature optimization. It's not like cargo generates that much data, and the messages don't have that many fields. I'd need to benchmark, but I really doubt any app would suffer from this overhead. Not to mention, without a stream parser, the current implementation basically allocates the whole thing. And since the serialization format is json, partial parsing isn't going to help perfs all that much.

Meanwhile, there is a very real cost in usability. Every user will end up having to redefine essentially the same structs, running rustc manually to discover what the fields are, etc...

[epage]

The parse_message_stream function in cargo_metadata just takes an io::Stream, so the user can just run xargo instead of cargo, and pass command.stdout themselves.

Ok, so it seems like your proposed solution is to only solve the parsing but not how the stream is created. In a way, this makes escargot and cargo_metadata solving two distinct halves of the problem and can be made to work together.

I'm still curious how you plan for your tool to work with both xargo and cargo, or if it already does, how it does it.

FYI it took me a sec to realize you were referring to a function you were adding.

As for the parsing, I feel like this is super premature optimization

I know; I admitted that. I also just wasn't up for taking the time to creating the structs. Thanks for taking the time to do it!

Meanwhile, there is a very real cost in usability. Every user will end up having to redefine essentially the same structs, running rustc manually to discover what the fields are, etc...

Yeah, defining the schema somewhere is really useful. That is the usability problem to solve. I was also viewing my usage of escargot as experiments in what static data queries might be like. For a while, I was looking into if it were possible to implement something like jsonpath with serde for being able to construct specific queries. Never ended up going anywhere with that.

[roblabla]

For cargo travis, I'll likely add a --cargo-invocation argument that allows specifying whether to run cargo build or xargo build or cargo xbuild or ...

Those will obviously need to support the --message-format=json arg (and implement it properly). I already have a fork of xargo that makes it understand and properly handle --message-output=json.

The other tool I'm writing this for is specifically to cross compile binaries for the nintendo switch, so it hardcodes xargo. See https://github.com/MegatonHammer/linkle/blob/master/src/bin/cargo-nro.rs

Maybe a simple way to fix the usability problem would be to have a doctest/example in escargot showing how to use the cargo_metadata types I'm adding to parse the messages? That way, escargot can focus on making things fast where it can, while still making the common case simple to use.

[epage]

For cargo travis, I'll likely add a --cargo-invocation argument that allows specifying whether to run cargo build or xargo build or cargo xbuild or ...

Ok. This is good stuff for me to consider for my work on stager, so I appreciate it!

Also, gives me ideas on how to maybe modify escargot to make it work with drop-in replacements for cargo.

Maybe a simple way to fix the usability problem would be to have a doctest/example in escargot showing how to use the cargo_metadata types I'm adding to parse the messages? That way, escargot can focus on making things fast where it can, while still making the common case simple to use

Once things are in and settled down for cargo metadata, I might experiment with how to output both.

[epage]

Started creating issues:

• Streaming support: Support streaming messages crate-ci/escargot#11
• xargo support: Support drop-in replacements for cargo crate-ci/escargot#12, Expand cargo-binary integration to support drop-in replacements for cargo like xargo. crate-ci/cargo-tarball#65
• cargo_metadata integration: [Usability] Integrate with potential new message format support in cargo_metadata crate-ci/escargot#13

[roblabla]

This is waaay overdue, but I started work porting to cargo-metadata #64.

I ended up not using escargot because I'm using cargo test --no-run instead of cargo build (which internally sets cargo's CompilationMode to Test), and escargot doesn't seem to provide a simple way to do that. I wanted to get something up and running fast, so I ended up doing manual binary invocations with all the dirty stuff this implies :'). I'll probably move to escargot once I sufficiently tested the idea.

[epage]

Understandable.

Could you open an issue on escargot about CompilationMode not being Test and what impact that has?

btw even if you don't want to use escargot's CargoTest, you could always use its definitions of the json messages from libtest or cargo test --no-run.

[epage]

Also, you might find clap-cargo useful. I'm using it in cargo-release to get me proper workspace support. The other features are trivial and only help if you haven't written support for those arguments already.

[roblabla]

Yup, I've been looking at clap-cargo for some time. I'm using docopt though, so I'd first need to switch to clap/structopt, which will likely come in a subsequent PR.

I'll open an issue on escargot once I figure out if the CompilationMode stuff has an impact or if I can just use cargo build --tests as a makeshift for cargo test --no-run. I believe the main difference is that cargo test --no-run automatically builds the doctests as well? I'd need to dig a bit.

And I don't need to parse the test output thankfully, since I only run the tests to gather coverage information - so I won't need escargot's json message definitions. But thanks :D.

[epage]

I believe the main difference is that cargo test --no-run automatically builds the doctests as well? I'd need to dig a bit.

Correct, it does not currently build or run the doctests. iirc cargo test --no-run will build but you wont be able to run them. I've started looking into that problem and it seems tarpaulin has a solution using an unstable flag to capture the doctest executables.