#56377 closed enhancement (fixed)
Additions to the cargo PG for all cases
Reported by: | MarcusCalhoun-Lopez (Marcus Calhoun-Lopez) | Owned by: | Marcus Calhoun-Lopez <marcuscalhounlopez@…> |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | Cc: | mascguy (Christopher Nielsen) | |
Port: | ripgrep fd mesalink racer librsvg cargo |
Description
Currently, there seem to be six ports that use the cargo build system:
Only ripgrep, fd, and mesalink use the cargo PortGroup, while the last three probably should (see, e.g, this ticket for cargo).
Ports that do not use configure and call cargo directly in the build phase:
- ripgrep
- fd
- racer
- cargo
Ports that use configure and make:
- mesalink
- librsvg
One possible solution:
options cargo.direct_call default cargo.direct_call yes option_proc cargo.direct_call handle_cargo_direct_call proc handle_cargo_direct_call {option action {value ""}} { if {${action} eq "set" && ${value}} { global build.jobs use_configure no build.cmd cargo build build.target build.pre_args --release --frozen -v -j${build.jobs} build.args # restore destroot.cmd default destroot.cmd {[portbuild::build_getmaketype]} } }
For the convenience of mesalink and librsvg, we might also consider removing
destroot { ui_error "No destroot phase in the Portfile!" ui_msg "Here is an example destroot phase:" ui_msg ui_msg "destroot {" ui_msg { xinstall -m 755 ${worksrcpath}/target/release/${name} ${destroot}${prefix}/bin/} ui_msg { xinstall -m 444 ${worksrcpath}/doc/${name}.1 ${destroot}${prefix}/share/man/man1/} ui_msg "}" ui_msg ui_msg "Please check if there are additional files (configuration, documentation, etc.) that need to be installed." error "destroot phase not implemented" }
from the cargo PG.
As a special case, librsvg seems to include its own crates, so the config file causes an error.
One possible solution:
options cargo.worksrcdir_crates default cargo.worksrcdir_crates no if {!${cargo.worksrcdir_crates}} { ... set conf [open "${cargo.home}/config" "w"] ... }
In order to respect build_arch
, we would need to set CARGO_BUILD_TARGET
.
This has the unfortunate consequence of changing the directly where the binary is built:
- ripgrep :
${worksrcpath}/target/release/rg
-->${worksrcpath}/target/$(CARGO_BUILD_TARGET)/release/rg
- fd:
${worksrcpath}/target/release/fd
-->${worksrcpath}/target/release/$(CARGO_BUILD_TARGET)/fd
- mesalink:
configure.args-append --enable-rusthost=\$CARGO_BUILD_TARGET
- racer:
${worksrcpath}/target/release/racer
-->${worksrcpath}/target/$(CARGO_BUILD_TARGET)/release/racer
- librsvg: patchfile
- cargo:
${worksrcpath}/target/release/cargo
-->${worksrcpath}/target/\$(CARGO_BUILD_TARGET)/release/cargo
In order to support universal builds, we would have to use the muniversal PG.
However, a shortcoming of muniversal is that it does not allow destroot in the Portfile.
So ripgrep, fd, racer, and cargo would have to be changed to use a Makefile for installation.
This may be too much of a maintenance headache.
Minor issue: if cargo is going to use the cargo PG, then the dependency will have to change.
One possible solution:
if {${subport} ne "cargo-bootstrap" && ${subport} ne "cargo-stage1" && ${subport} ne "cargo"} { depends_build-append port:cargo # do not force all Portfiles to switch from depends_build to depends_build-append proc cargo.add_dependencies {} { depends_build-delete port:cargo depends_build-append port:cargo } port::register_callback cargo.add_dependencies }
The existence of cargo-bootstrap
and cargo-stage1
subports assumes the inclusion of parts of the patch from 56195.
Minor issue: the libssh2-sys crate (used by cargo) has an annoying issue with header files.
One possible solution:
foreach {cname cversion chksum} ${cargo.crates} { # the libssh2-sys crate requires the header files from # a version of libssh2 that has not been released # (e.g. channel.c uses the error code LIBSSH2_ERROR_CHANNEL_WINDOW_FULL) # make sure these header files are found properly if {${cname} eq "libssh2-sys"} { foreach f [glob -tail -directory ${cargo.home}/macports/libssh2-sys-${cversion}/libssh2/include/ *.h] { ln -s ../include/${f} ${cargo.home}/macports/libssh2-sys-${cversion}/libssh2/src/ } } }
However, it is not at all clear that this is of general enough use to belong in the cargo PG.
Minor issue: if cargo is to use the cargo PG, it will have to change build.cmd
to some prebuilt cargo.
Such a change will be slightly easier if
build.cmd cargo build build.target
becomes
build.cmd cargo build.target build
.
Change History (5)
comment:1 Changed 7 years ago by raimue (Rainer Müller)
comment:2 Changed 7 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)
I have created a pull request, which hopefully addresses most of the concerns.
However, I was not able to get OUT_DIR
working.
comment:3 Changed 7 years ago by raimue (Rainer Müller)
Thank you for working on this! I am sure the support for custom build phases in muniversal will also help other ports. It even looks easier than I thought.
Maybe OUT_DIR
is not supported for all builds, but just by some applications use it in their build.rs
. I will make a mental note to look into this again when I update ripgrep to get rid of the hardcoded hash in that port.
comment:4 Changed 7 years ago by Marcus Calhoun-Lopez <marcuscalhounlopez@…>
Owner: | set to Marcus Calhoun-Lopez <marcuscalhounlopez@…> |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:5 Changed 3 years ago by mascguy (Christopher Nielsen)
Cc: | mascguy added |
---|
Hm, I always thought the main use of the cargo port group was to run a build with cargo. I am not sure the proposed way would work, because the default of
yes
would always overridebuild.cmd
...?I guess the only way to make that work would be a default value of
no
and force all ports to usecargo.direct_call yes
. However, that would be not consistent with other port groups, such as those for cmake or meson, that all just override the commands by default. I also do not thinkdirect_call
is an intuitive name.How about we split the port group in two parts? Port group
cargo_fetch
would handle thecargo.crates
options for fetching, but onlyPortGroup cargo
would also override the build commands. The latter would include the former. Then mesalink and librsvg would only usePortGroup cargo_fetch
.I am not sure I understand the problem that
cargo.worksrcdir_crates
is supposed to solve. Why is the config file causing a problem? I assume librsvg does not need any external crates? Why does librsvg need the cargo port group at all, if it is neither fetching crates nor using the build system commands...?We could choose not to create
cargo.home
if bothcargo.crates
andcargo.crates_github
are empty. Then we would not even need an extra setting for this.Could we solve the problem of the target directory with
CARGO_BUILD_TARGET
by settingOUT_DIR
? Most builds seem to respect that in the environment.Fixing muniversal to support
destroot {}
would be one possible solution. However, using muniversal seems overkill anyway, as it seems to do a lot of stuff that is not necessary for cargo at all. As you wrote, cargo already uses different output directories per build architecture, so all that would be necessary is to execute thecargo build
command multiple times, then merge the binaries/libraries. As all binary files seem to end up in a known place, that seems much simpler than what muniversal implements.However, this will probably not help with librsvg, which was probably your main use case for this...
It could be an alternative for +universal builds, there is also cargo-lipo. I have no experience with it, but it also seems to provide the functionality. Although it is meant to be used for iOS builds, though.
The way
destroot {}
is implemented right now is largely due to an upstream issue, as cargo cannot handle additional files such as man pages. Otherwise I would just have setdestroot.cmd cargo install
. Therefore my solution was to force Portfile authors to write their owndestroot {}
phase that installs all files that can be installed. The destroot phase in the cargo port group was intended to remind Portfile authors to take a closer look which files should be installed.I specifically left
build.target
empty. As I understand cargo, that would be the place to specify--all
,--bin <binary>
,--libs
, or similar arguments, as this is what cargo calls a "build target". The command we want to run is in fact alwayscargo build
.With a custom cargo, I would just change that to
build.cmd .../path/to/cargo build
.