Page MenuHomeFreeBSD

Mk/Uses/cargo.mk: Add support for porting Rust applications to the ports framework
ClosedPublic

Authored by tobik on Jun 12 2017, 6:47 PM.

Details

Summary

This is a rough port of OpenBSD's cargo ports module for porting Rust
applications. I did this as an experiment a while back. It works ok
for an example ripgrep port (even in Poudriere). I'm not exactly sure
how to move forward with this though.

I've added a port of ripgrep and bingrep to demonstrate how it can be
used.

Rough port creation outline is as follows:

  1. Add USES=cargo
  2. Add distfile, make makesum
  3. make patch
  4. Run make cargo-crates-1 to generate CARGO_CRATES from the sources. This locks dependencies to the versions specified in the Cargo.toml file (though it should be possible to change the dependencies to a different version should the need arise). Add the output to the port Makefile.
  5. Rerun make makesum to add all checksums for the Crates
  6. make clean
  7. make

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Add devel/bingrep example

There are some occurrnces of grep or sed in the code, they are really called ${GREP} and ${SED} (or ${REINPLACE_CMD} if changing an existing file)

Mk/Uses/cargo.mk
8 ↗(On Diff #29500)

You should add a check to make sure no args are passed.

39 ↗(On Diff #29500)

Should probably be something along the lines of:

CARGO_DIST_SUBDIR?=    ${DIST_SUBDIR:Ucargo}
41–45 ↗(On Diff #29500)

Variables starting with an _ are supposed to be private, but nothing uses this one here.

50 ↗(On Diff #29500)
​MASTER_SITES+= CRATESIO:cargo

(I spent a lot of time going over all the ports tree for this, please don't add more :-)

57–65 ↗(On Diff #29500)

Instead of putting all the shell code in a variable that you then use later in the target, could you put all the code in the target directly ? (same for patch and confirgure.)

105 ↗(On Diff #29500)

I'm not sure creating a variable for this is interesting, it is only used once, and is not overridable.

186 ↗(On Diff #29500)

Same here, put everything in the target.

199 ↗(On Diff #29500)

same.

211–219 ↗(On Diff #29500)

Keep the targets sorted.

# make -V _TARGETS_STAGES
SANITY PKG FETCH EXTRACT PATCH CONFIGURE BUILD INSTALL TEST PACKAGE STAGE
devel/bingrep/distinfo
2–6 ↗(On Diff #29500)

All those file names are very strange.

textproc/ripgrep/pkg-descr
1 ↗(On Diff #29500)

Burp.

tobik marked 10 inline comments as done.Jun 13 2017, 8:29 PM
tobik added inline comments.
Mk/Uses/cargo.mk
57–65 ↗(On Diff #29500)

Moved everything to the relevant targets. I also removed the cargo-metadata target since this is basically the same now as post-patch.

devel/bingrep/distinfo
2–6 ↗(On Diff #29500)

Yes, any idea how to fix them?

cargo.mk adds entries like ansi_term/0.9.0/download:cargo to DISTFILES and fetch downloads crates from https://crates.io/api/v1/crates/ansi_term/0.9.0/download.

Is there a way to specify the output filename? We could use saner names like for example cargo/ansi_term-0.9.0.tar.gz.

textproc/ripgrep/pkg-descr
1 ↗(On Diff #29500)

Yeah :)

I will merge the ripgrep port with what was submitted in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=215212

tobik marked 2 inline comments as done.
  • Address feedback so far
  • Rename target cargo-crates-1 to cargo-crates and cargo-crates-2 to cargo-crates-licenses
  • Merge ripgrep port with PR215212
  • Add another example port editors/xi-core

I do not really like the Makefile.crates that are created, unless they are used by more than one port, the content should go in the Makefile.

Also, all those += is ugly, would be better off having one long CARGO_CRATES=

Mk/Uses/cargo.mk
114–134 ↗(On Diff #29570)

Should have thought about this earlier.

USES usually do not override pre-/post-, they create their own targets and hook them up with _USES_xxx, see other USES (say, perl5.mk, there is a fixup thing near the end).

It allows ports to have their own pre-/post- targets in case they also need to do something else.

devel/bingrep/distinfo
2–6 ↗(On Diff #29500)

Yes, there is, for example, https://crates.io/api/v1/crates/ansi_term/0.9.0/download?f=/ansi_term-0.9.0.tar.gz (or f is purely artificial, it does not mean anything to the web site, it is only used to fool fetch.) USE_GITHUB does the same thing.

Then, with setting DIST_SUBDIR=cargo, you'll end up with the files where you want them.

tobik marked 4 inline comments as done.
  • Download crates to ${DISTDIR}/cargo and fix crate filenames
  • Remove all Makefile.crate files again
  • Make cargo-crates output nicer
  • Move post-patch and post-extract to _USES_extract and _USES_patch
  • Actually fix cargo-patch to use the new distfiles in cargo/ (woops!)
textproc/ripgrep/pkg-descr
6 ↗(On Diff #29602)

empty line.

devel/bingrep/Makefile
8 ↗(On Diff #29602)

expand

10–14 ↗(On Diff #29602)
devel/cargo-generate-vendor/Makefile
8 ↗(On Diff #29602)

new ports really must have a maintainer.

Mk/Uses/cargo.mk
136–140 ↗(On Diff #29602)

I don't really like this. It would be better as being part of the do-configure. Something like:

${CARGO_CARGO_UPDATE} ${CARGO_CRATES_UPDATE:S/^/--package /};

or something.

  • Fix CARGO_CRATES_UPDATE
  • Fix a bunch of out-of-order USE_GITHUB
  • Fix comment/pkg-descr of devel/bingrep
  • Make cargo-crates-licenses output a table

Could you write up a small example on how to create a cargo based port ?
For inclusion in the future USES=cargo documentation in the porter's handbook that I'll be writing.

Oh, it is in the summary, silly me. Maybe an interactive version, so that I don't have to go and figure out exactly what does what :-)

Thank you for working on this, this is very helpful!

Mk/Uses/cargo.mk
42 ↗(On Diff #29606)

Could we use a subdirectory of ${DISTDIR}/rust? We already use this to put Rust and Cargo source archives & bootstraps.

Also, the name doesn't reflect the content: we would store crate archives, not Cargo itself.

I suggest:

​CARGO_DIST_SUBDIR?= rust/crates
This revision now requires changes to proceed.Jun 15 2017, 9:37 AM
tobik edited edge metadata.
  • Add another example port sysutils/exa which demonstrates how to deal with Rust applications that have a dependency pinned to a specific Git commit (Cargo will try to clone the repository during the build if this isn't dealt with by the port)
  • Set CARGO_DIST_SUBDIR to rust/crates by default
  • Add another example port sysutils/exa which demonstrates how to deal with Rust applications that have a dependency pinned to a specific Git commit (Cargo will try to clone the repository during the build if this isn't dealt with by the port)

Maybe the parsing you are doing in cargo-crates could include outputting the correct GH_TUPLE lines, and maybe a post-patch target 0:-)

  • Move cargo-crates script to ${SCRIPTSDIR}/cargo-crates.awk
  • cargo-crates now deals with git dependencies (for GitHub only) and outputs a hopefully useful GH_TUPLE.
  • Add new cargo-patch-github target that patches both Cargo.toml and Cargo.lock on its own with the paths to ${WRKSRC_<subdir>} from GH_TUPLE when CARGO_USE_GITHUB=yes.
  • Simplify sysutils/exa and add sysutils/flowgger as an example application with multiple GH_TUPLE.
Mk/Scripts/cargo-crates.awk
30 ↗(On Diff #29723)

Note that that bit of the url can contains many chars that are not allowed in our tags. Our tags end up being shell variables, so they must only contain [a-zA-Z0-9_] (as seen in the C locale, can't have accented characters and stuff).

35 ↗(On Diff #29723)

That is a bad idea, even for small repositories, collisions can come faster than you think.
You should either leave it as it is, as it is automatically generated, it does not really matter, or if you really want to shorten it, make it 10 or 12 chars long.

Mk/Uses/cargo.mk
123–126 ↗(On Diff #29723)

To make this easier to read, you might want to either pre-parse _tuple in local variables, or use the ${GH_(ACCOUNT|PROJET)_${_tagname}} variables that are created by the framework.

Also, this might not work if the port does not use the GH_TUPLE but the regular GH_* variables. You might want to iterate over something else like GH_ACCOUNT. (But it may not be ready at that time.

  • Make cargo-patch-github slighly more readable
  • Don't abbreviate commit hashes in cargo-crates.awk and only use tags that match [a-zA-Z_]
tobik added inline comments.
Mk/Uses/cargo.mk
123–126 ↗(On Diff #29723)

I'd nice if I was able to iterate over ${_GITHUB_GROUPS:NDEFAULT} but that of course doesn't work. :-(

Mk/Scripts/cargo-crates.awk
17–18 ↗(On Diff #29729)

So, reading more, this is the one that cannot contain anything else than [^a-zA-Z_].

Those two lines should probably be folded into one:

gsub("[^a-zA-Z_]", "", package_name)
31 ↗(On Diff #29729)

mmmm, I thought tag was the last part, not the commit id. Sorry about it.

50 ↗(On Diff #29729)

Mmmm, if the original software does not come from github, this will be a problem.

I see two cases here.

  1. The port has USE_GITHUB defined, either because its source comes from github (USE_GITHUB=yes), or some stuff it needs comes from github (USE_GITHUB=nodefault) then you do not output this line.
  2. The port does not already have USE_GITHUB defined (so the source comes from somewhere else) then output USE_GITHUB=nodefault.
Mk/Uses/cargo.mk
123–126 ↗(On Diff #29723)

You cannot iterate over it because the variable starts with an _ and is local to the github code from bsd.site.mk.
But in any way, you cannot iterate over it because it does not exist yet, at that point.

  • cargo-crates.awk: Examine the port's USE_GITHUB and print USE_GITHUB=nodefault if necessary
  • Remove some old never or only once used variables
  • Add some special handling for some crates and handle port dependencies for them.
  • The openssl-sys crate requires USES=ssl and that OPENSSL_{LIB,INCLUDE}_DIR are defined. It uses them to find OpenSSL.
  • The pkg-config crate requires USES=pkgconfig
  • The libgit2-sys crate can be coerced to use devel/libgit2 instead of building its own version (which would require adding cmake to BUILD_DEPENDS)

Cleanup after some wrong assumptions:

  • cargo-patch-github does not need to patch Cargo.lock. Cargo will take care of it on its own when all sources are in place and Cargo.toml has been properly patched. cargo update in the configure phase will now be run unconditionally and CARGO_CRATES_UPDATE was removed.
  • Add new CARGO_UPDATE_ARGS matching CARGO_{BUILD,INSTALL,TEST}_ARGS
  • Add x11/alacritty which is interesting because it has multiple Cargo.toml files with git dependencies which all require patching in cargo-patch-github. Add a new variable CARGO_GH_CARGOTOML which can be used to add more files to it.
  • Support WITH_DEBUG
  • Remove cargo-generate-vendor and move cargo-patch into cargo-extract

cargo-generate-vendor was used to generate a .cargo-checksum.json
file for each crate with the following structure:

{"package": "<crate distfile checksum>", "files": { ... }}

with entries in "files" for whe whole file tree under the crate's root:

{"<file path relative to crate-root>": "<file checksum>"}

But it turns out we can get away with an empty files object i.e.
just {"package": "<checksum>", "files": {}} suffices. Cargo seems
to never examine it with the commands we run i.e. all example ports
here still build fine.

Mk/Scripts/cargo-crates.awk
53 ↗(On Diff #29772)

I still think this should only be written in the USE_GITHUB=nodefault case. As it already exists otherwise.

  • cargo-crates.awk: Only output USE_GITHUB when it isn't set in the port

I am fresh out (it's 34°C here) of idea of things to make it better :-)

I would say commit it and see how it gets alive.

Do it first the cargo.mk/awk, then each port in its own commit.

This revision was automatically updated to reflect the committed changes.