Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#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)

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 override build.cmd...?

I guess the only way to make that work would be a default value of no and force all ports to use cargo.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 think direct_call is an intuitive name.

How about we split the port group in two parts? Port group cargo_fetch would handle the cargo.crates options for fetching, but only PortGroup cargo would also override the build commands. The latter would include the former. Then mesalink and librsvg would only use PortGroup 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 both cargo.crates and cargo.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 setting OUT_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 the cargo 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 set destroot.cmd cargo install. Therefore my solution was to force Portfile authors to write their own destroot {} 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 always cargo build.

With a custom cargo, I would just change that to build.cmd .../path/to/cargo build.

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: newclosed

In 0d4b18fbffaf6907796e78809f694c59e527902f/macports-ports (master):

split cargo PG into cargo and cargo_fetch

Fixes #56377
Fixes #56195

comment:5 Changed 3 years ago by mascguy (Christopher Nielsen)

Cc: mascguy added
Note: See TracTickets for help on using tickets.