Crate evaluation for 2017-07-11: gcc

libs
1

Crate evaluation for 2017-07-11: gcc

For additional contribution opportunities, see the main libz blitz thread.

This post is a wiki. Feel free to edit it.

Links

Needs your help!

Anything that is not checked off still needs your help! There is no need to sign up or ask for permission - follow any of these links and leave your thoughts:

Guidelines checklist

Legend

  • [y] = guideline is adhered to, no work needed.
  • [n] = guideline may need work, see comments nearby
  • [/] = guideline not applicable to this crate

Checklist

Guidelines checklist

This document is collaboratively editable. Pick a few of the guidelines, compare the gcc crate against them, and fill in the checklist with [y] if the crate conforms to the guideline, [n] if the crate does not conform, and [/] if the guideline does not apply to this crate. Each guideline is explained in more detail here. If [n], please add a brief note on the following line with an explanation. For example:

  - [n] Crate name is a palindrome (C-PALINDROME)
        - gcc backwards is ccg which is not the same as gcc

API guideline updates

What lessons can we learn from gcc that will be broadly applicable to other crates? Please leave a comment in that issue with your ideas!

Discussion topics

Should we rename this crate?

This crate includes some VS discovery code that seems a bit out of place. Do most build scripts that use C compilers need this module? Can it be moved into another crate?

ToolFamily is a closed enum. Should it be opened?

Tool claims to be represent a C compiler, but at least windows_registry will return Tool’s for things that are not compilers. What needs to be clarified here?

Can .o files be generated by this crate? This seems to want to produce static libraries only.

expand seems weird. What’s it doing? What does it mean to convert multiple input files to a Vec?

It’s pretty clear this library has evolved to accommodate practical matters as they arise - it does not appear to be a comprehensive C compiler driver. Should we try to featurefy it for 1.0?

Is the scope of this crate (disregarding windows_registry) simple driving C / C++ compilers to produce static libraries? What about dynamic libraries, what about object files, what about driving the linker directly? One could imagine e.g. extracting rustc’s entire backend linker abstraction so that others could use it - would that go here, another crate?

Inconsistency - compile_library() shorthand supports setting multiple files from a slice, but the “full” usage via Config does not (it requires multiple calls to .file()).

There’s cpp(bool), but it doesn’t specify the C++ version. Similarly there’s no way to specify C version (some compilers default to C99, some require opt-in, so an unspecified version is non-portable).

  • budziq: An enum or even a separate builder for compiler configuration/variants might be more appropriate here

This crate panics on errors, as do many build-time crates. What do we think of this?

Crate issues

  • Explain what library names are valid for compile_input
    • Is it just .a? .so, etc.?
  • Clarify doc language around ‘gcc’, ‘C compiler’, ‘tool’ etc.
    • These are all used fairly imprecisely
  • cargo_metadata - explain more precisely what this metadata is and where it is emitted

How are we tracking?

Pre-review checklist

Do these before the libs team review.

  • [x] Create evaluation thread based on this template
  • [ ] Work with author and issue tracker to identify known blockers
  • [ ] Compare crate to guidelines by filling in checklist
  • [ ] Record other questions and notes about crate
  • [ ] Draft several use case statements to serve as cookbook examples
  • [ ] Record recommendations for updated guidelines

Post-review checklist

Do these after the libs team review.

  • [ ] Publish libs team meeting video
  • [ ] Create new issues and tracking issue on crate’s issue tracker
  • [ ] Solicit use cases for cookbook examples related to the crate
  • [ ] File issues to implement cookbook examples
  • [ ] File issues to update guidelines
  • [ ] Post all approachable issues to TWiR call for participation thread
  • [ ] Update links in coordination thread
3

Oddities I’ve ran into:

  • compile_library() shorthand supports setting multiple files from a slice, but the “full” usage via Config does not (it requires multiple calls to .file()).

  • There’s cpp(bool), but it doesn’t specify the C++ version. Similarly there’s no way to specify C version (some compilers default to C99, some require opt-in, so an unspecified version is non-portable).

4

The name is especially confusing because the crate can be used to invoke the VC++ toolchain, which is an entirely different world completely.

5 Likes
5

An enum or even a separate builder for compiler configuration/variants might be more appropriate here

6

I agree, given that this is a wrapper for whatever the platform C compiler happens to be (generally gcc, clang, or MSVC), the name gcc is very misleading.

The cc crate is a cycle collector from pre-1.0, has no dependent crates, doesn't compile on Rust 1.0 or later, and hasn't been updated since 2014 (there's one commit in GitHub from 2015 to fix one 1.0 compat issue, but there are others that are not fixed). It's possible that we could just ask @cmr if we could take over the name.

Other alternate obvious names could be compilec or ccompiler (or snake or kebab case versions, like compile-c, compile_c, c-compiler, or c_compiler) but those are a lot more cumbersome. cc would be my preference, followed by compilec, followed by just sticking with gcc and accepting the confusion that causes.

1 Like
7

This link goes to an API design guidelines issue for error-chain. Not sure if that issue should be made generic, or should file a new one for gcc and link to that.

8

I’m happy to give up cc. I made it for a scheme interpreter for some coursework.

11 Likes
9

This crate’s interesting as a layer of gloss over other tools. My first impression was that it should make more effort to return errors rather than panicking. But maybe it’s a bit special for being a tool you use in a build.rs, where panicking is how you bork the build.

I don’t really use build scripts or write C / C++ though, so am interested in what kinds of things people are putting in them.

10

I didn’t see a cookbook issue for this so I made one: https://github.com/brson/rust-cookbook/issues/236. It’s a weird one since it’s a build-time crate, but it would be good to figure out how to teach people about it.

11

I just removed the link and updated the template to not include the link. Pre-opening guidelines issues hasn't been that useful. When they come up we can note them in here and in the op and file them later.

12

I did a quick review of the crate and added discussion points and potential issues to the OP.

13

Sure. Had been asking about that since I was wondering if we should add a guideline on crate naming to the guidelines. For instance, a few people have noted that "gcc" is a confusing name for a crate that's used to invoke MSVC, clang, gcc, or other C compilers depending on the target. Might be worth adding a guideline for checking the crate name to make sure that it's reasonably discoverable and descriptive of what the crate is about.

14

Thanks to those that filled out the checklist! Big, big help. If you have time you might translate the obvious ones to issue titles in the op under the ‘crate issues’ section.

This one is going to the libs team tomorrow. It went by too quick… but fortunately it’s a small crate.

This one is an interesting crate, because it is a very practical crate - it’s obviously evolved organically to accommodate whatever needed to be done in the real world. And it also seems to me like there’s quite a lot one might want to do with a C compiler that this doesn’t let you do. This seems firmly oriented at compiling a set of individual C/C++ files into static libraries so they can be statically linked to Rust crates; with more complex build tasks relegated to other crates like cmake / pkg-config. The exception being the weirdo windows_registry module which I think is currently the best way to do any VS stuff one might want.

I wonder how much we want to try to add features to the design and/or clean up the existing design.

It looks to me like it would make sense to punt the VS code to yet another crate, and have this one depend on it just so it can implement Config / Tool for MSVC.

15

I’ve also collected other points from this thread into discussion topics in the op.

16

I’ve ran into a limitation of this crate - it’s not possible to check what features (flags) compiler supports, e.g. OpenMP, or specific rare optimization flags -falign-stack=maintain-16-byte.

17

Probably. At least it should add the Intel compiler.

18

How do you open an enum?

19

ring’s build.rs at https://github.com/briansmith/ring/blob/master/build.rs is probably one of the more advanced users of this crate. In particular, note that we compile asm code and C code and if you look at the history of the file we compiled C++ code too. We also extensively customized the compilation and linking with additional options, in order to achieve parity with the Makefile-based system that the usage of gcc-rs replaced. We also spent considerable effort getting cross-compilation to various targets—especially ARM/Aarch64 and ARM/Aarch64 Android & iOS—to work.

Note that gcc-rs has a way of doing parallel building, but we were not able to use it efficiently because we had other build steps that we also wanted to run in parallel, and gcc-rs’s parallel build support doesn’t compose well with additional build parallelism implemented in the build.rs. It would be good to factor parallel build support and dependency tracking out into a more general (not just asm/C/C++) library.

On Windows, we target -msvc but IIRC we didn’t need to use the stuff in gcc-rs’s windows_registry module at all. Maybe nowadays Cargo and rustc and gcc-rs do things automatically enough where that module isn’t needed by dependent crates’ bulid scripts and longer.

1 Like
20

Here's the etherpad from the review meeting. I intend to file bugs shortly:

re ToolFamily, this is actually a private type, so opening it doesn't matter.

@kornel do you mind filing a bug on this? Hopefully it's something that could be added backwards compatibly.

Offhand, I don't know how you would check a compiler for supported flags. Maybe you can mention on the bug how this might be done.

@gnzlbg by convention you add a #[doc(hidden)] __Nonexhaustive variant.

Awesome feedback @briansmith. I'll make sure to include this subject in the tracking issue. Coordinating parallelism across the build is kind of a hot topic these days, with cargo implementing (and presently reverting) make job server support. (Also my windows machine gets utterly DOSed every time I run cargo without -j1 :unamused:).

Yeah, this module doesn't belong. Plan is to pull it out into a different crate.

21

OK, I’ve filed the tracking issue for this evaluation:

https://github.com/alexcrichton/gcc-rs/issues/198

There are several here that need some bikeshedding still, or where the details are not clear to me:

https://github.com/alexcrichton/gcc-rs/issues/174

https://github.com/alexcrichton/gcc-rs/issues/184

https://github.com/alexcrichton/gcc-rs/issues/189

https://github.com/alexcrichton/gcc-rs/issues/190

https://github.com/alexcrichton/gcc-rs/issues/191

https://github.com/alexcrichton/gcc-rs/issues/195

This also uncovered one new guidelines issue:

https://github.com/brson/rust-api-guidelines/issues/104

1 Like