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

Add support for Cargo (Rust) #42

Merged
merged 15 commits into from
Jul 26, 2021
Merged

Add support for Cargo (Rust) #42

merged 15 commits into from
Jul 26, 2021

Conversation

sideeffffect
Copy link
Contributor

No description provided.

@@ -37,6 +37,7 @@ trait BuildTool {

baseDirectory.mkdir()
val out = baseDirectory.toPath().resolve(name)
Files.createDirectories(out.getParent)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is necessary, so that the possibly deeply nested structure of directories is created before trying creating a new file.

@pomadchin
Copy link
Member

👋 I'm pretty excited to get it merged, but could you add some tests?

You could copy the simple project from here https://github.com/sbt/sbt-jni/tree/main/plugin/src/sbt-test/sbt-jni and call it ~ simple-cargo or smth and adjust to use Cargo instead of CMake.

@sideeffffect
Copy link
Contributor Author

We're green 🎉

Copy link
Member

@pomadchin pomadchin left a comment

Choose a reason for hiding this comment

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

That's super cool, thanks for making it 🚀 I'd like to use this one in my next projects honestly.

But I have a minor comment about the cargoReleaseProfile, what about making it configurable?

Could I also ask you to add a line into the CHANGELOG.md?

object BuildTool {
lazy val buildTools: Map[String, BuildTool] = Map(
CMake.name.toLowerCase -> CMake,
Cargo.release.name.toLowerCase -> Cargo.release
Copy link
Member

Choose a reason for hiding this comment

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

hmm, what do you about making the releaseFlag configurable? i.e. having a cargoReleaseProfile setting? It can still be useful for debugging purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is!

nativeBuildTool := Cargo.make(false)

Copy link
Member

Choose a reason for hiding this comment

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

@sideeffffect 💯 niice!

.dependsOn(native % Runtime)

lazy val native = project
.settings(nativeCompile / sourceDirectory := baseDirectory.value) // `baseDirectory`, not `sourceDirectory`
Copy link
Member

@pomadchin pomadchin Jul 26, 2021

Choose a reason for hiding this comment

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

Ha, what a trick, indeed, that's due to the Cargo project structure (a comment for myself):

├── Cargo.lock
├── Cargo.toml
├── src/
│   ├── lib.rs

I'll add it into docs later.

Copy link
Member

@pomadchin pomadchin left a comment

Choose a reason for hiding this comment

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

Thanks! 🚀 I'll merge it and will start working on a fresh release routine. I think I'm just going to release it as the 1.5.0 release and since this release we'll be following the semver.

@pomadchin pomadchin merged commit 94a2cc1 into sbt:main Jul 26, 2021
@sideeffffect sideeffffect deleted the cargo branch July 26, 2021 15:17
@sideeffffect
Copy link
Contributor Author

Btw, instead of maintaining a Changelog manually, we could use Release Drafter. Have a look here for inspiration:
https://github.com/zio/zio-prelude/blob/master/.github/release-drafter.yml
https://github.com/zio/zio-prelude/blob/master/.github/workflows/release-drafter.yml

@pomadchin
Copy link
Member

pomadchin commented Jul 26, 2021

@sideeffffect a very nice idea 👍. I'd still like to keep the CHANGELOG.md file though (I find it very useful to search through all changes placed in a single file); but mb we can use drafter + some other github action to preserve the file as well.

@sideeffffect
Copy link
Contributor Author

So shall we make a release? :)

@pomadchin
Copy link
Member

pomadchin commented Jul 26, 2021

@sideeffffect you mean drafter? or just the release?

I think ETA for release is today evening / tomorrow morning (EDT) - I'd like to add more words about Cargo into the README (you can help it btw! haha);

We also changed the groupid so I'm thinking about packages renaming (#49), it can be pretty confusing for users to see the groupid com.github.sbt but ch.jodersky.jni.nativeLoader in the code. That's a great opportunity for us to make this transitioning.

But I'd like to hear your opinon on that as well.

@sideeffffect
Copy link
Contributor Author

packages renaming

good idea 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants