On Fri, Feb 16, 2024 at 03:15:02PM -0800, Evan Silberman wrote:
> Stuart Henderson <stu@spacehopper.org> wrote:
> > On 2023/09/25 22:25, Evan Silberman wrote:
> > > Hi ports,
> > > 
> > > Here's a port of Nickel (https://nickel-lang.org/), a programmable
> > > configuration language that allows both strong-typing and more dynamic
> > > contract-driven specification of configurations.
> > > 
> > > There doesn't seem to be a uniform approach to building ports of
> > > applications that happen to be distributed as Rust crates; I'm just
> > > using the cargo index as SITES here.
> > 
> > That approach seema reasonable to me.
> > 
> > Two things I noticed:
> > 
> > - see line 1 of crates.inc
> > - LIB_DEPENDS is not valid unless there's a WANTLIB coming from that
> > port - there should be an error message due to this when running "make
> > package", it's explained in bsd.port.mk(5)
> 
> Got back to this this week and this turned out to be a bit of a rabbit
> hole. I originally added oniguruma when a crate build failed without it
> but when I looked recently I realized that the final exe didn't link to
> libonig. I followed the Cargo.toml trail a bit and determined that
> oniguruma was required by an optional feature of nickel's direct
> dependency comrak which nickel doesn't actually use. It seems like
> upstream intended to build comrak without that feature but "features =
> []" in Cargo.toml is evidently not the same as "default-features =
> false". In any case, the patches in this tarball prevent the onig crate
> from being built uselessly, and then add a feature requirement to a
> dependency directly that was previously being relied upon indirectly.
> I'll try to upstream that but I wanted to get the port out for review
> first.

Disabling the unused feature seems like th right approach.
A brief patch comment explaining that would help, otherwhise it is
entirely unclear to porters reading just the patch, imho.

No idea about nickel, but port-wise this this looks fine and builds.

Tests do not compile and thus don't run, though, is that a known issue?