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

Split a -sys crate out #68

Merged
merged 19 commits into from Oct 15, 2015
Merged

Split a -sys crate out #68

merged 19 commits into from Oct 15, 2015

Conversation

@sfackler
Copy link
Contributor

sfackler commented Aug 26, 2015

I used some reexports to make this move backwards compatible for current users.

Review on Reviewable

@metajack
Copy link
Contributor

metajack commented Aug 26, 2015

Since when did a -sys crate start meaning Rust bindings to a C library? When I started this naming convention it was for creates that shipped a C library. freetype-sys was libfreetype2 Cargo-ified, and freetype was the Rust bindings to Freetype.

We are not shipping CoreFoundation.framework in Cargo packaging, so this is confusing.

@metajack
Copy link
Contributor

metajack commented Aug 26, 2015

cc @aturon @alexcrichton for input on the community standard. I'll be really disappointed if we have to rename everything because the community took the wrong cue from Servo conventions.

@metajack
Copy link
Contributor

metajack commented Aug 26, 2015

Beautiful. :(

So now what do C packages in Cargo get called? pkg-config is not a solution to things that aren't easily expected to be installed or must have specific versions or for Android, etc.

I'll a little bit cranky that I spent a bunch of time working this out, asking @alexcrichton and others for advice, only to have the solution co-opted and documented as being something other than what was decided. Now I get to do all the work again if we want to maintain community norms, and apparently I get to come up with a new naming convention that someone may then reappropriate in the future for something else!

@alexcrichton
Copy link

alexcrichton commented Aug 26, 2015

@metajack yeah the standard is for foo-sys crates to provide basically a header file for the C library foo. It's also responsible for doing anything necessary to actually link to the library foo, which can include things like:

  • Just printing -l flags
  • Running pkg-config to find the library
  • Building the library from source (either vendored or perhaps downloaded)

If the library can't actually be linked for whatever reason, it's the responsibility of the build script to print an error. In that sense the convention of foo and foo-sys crates is the same, it's just focusing the responsibility of the foo-sys crate to link and provide definitions for the library.

Sorry if there was any miscommunication earlier, this was the intention from the get-go and something may have just been lost along the way.

@sfackler sfackler force-pushed the sfackler:master branch from d7c5d08 to d9a3185 Sep 6, 2015
@sfackler
Copy link
Contributor Author

sfackler commented Sep 6, 2015

Rebased

@sfackler
Copy link
Contributor Author

sfackler commented Sep 22, 2015

ping @pcwalton?

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 15, 2015

I'll review this.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 15, 2015

Reviewed 30 of 43 files at r1, 1 of 1 files at r2, 9 of 9 files at r3.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from the review on Reviewable.io

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Oct 15, 2015

larsbergstrom added a commit that referenced this pull request Oct 15, 2015
Split a -sys crate out
@larsbergstrom larsbergstrom merged commit 6ee9bed into servo:master Oct 15, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sfackler
Copy link
Contributor Author

sfackler commented Oct 17, 2015

Thanks! Could you publish to Crates.io by any chance?

@metajack
Copy link
Contributor

metajack commented Nov 8, 2015

This got published.

jdm pushed a commit that referenced this pull request Feb 1, 2018
Fix silly copy-paste mistake
jdm pushed a commit that referenced this pull request Feb 1, 2018
Update to core-foundation 0.3

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-graphics-rs/68)
<!-- Reviewable:end -->
jdm pushed a commit that referenced this pull request Feb 1, 2018
Bump foundation and graphics versions for #62.

This removes the last of the compiler warnings.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-text-rs/68)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.