Sunday, October 04, 2020

Re: Review for cabal.port.mk

Thanks for the prompt review!

Marc Espie <espie@nerim.net> writes:
> I'll have to look more closely, but I have a few concerns.
>
> On Sat, Oct 03, 2020 at 11:03:43PM -0700, Greg Steuck wrote:
>> MASTER_SITES0 = https://hackage.haskell.org/package/
> That's a minor one.
> This sets a precedent of hijacking a MASTER_SITESn for a specific module.
> I don't think it's a big concern because cabal is somewhat isolated.
> We'll deal with that if we must.

Moved to :9 to reduce the chance of collision. Unless you believe an
indirection is warranted I'll keep it at this.

>> DIST_SUBDIR ?= hackage
>>
>> # The .cabal files are explicitly copied over the ones extracted from
>> # archives by the normal extraction rules.
>> EXTRACT_CASES += *.cabal) ;;
>
> I'm not fond of this solution to the PKGSTEM issue.

Is this comment about EXTRACT_CASES above or does it belong with
DISTNAME below?

> I would prefer that you would reconstitute DISTNAME from MODCABAL variables
> exactly like stuff is done for github

There's a minor tension which I'm not sure how to resolve
best. Implementing your suggestion may complicate porting the
self-hosted main binary packages that aren't published to hackage, yet
grab the dependencies there.

So far all the cases I have are hackage packages. Which means I should
follow YAGNI and implement your suggestion. I'll give it a try, but
wanted to explain why I didn't do this before.

>> MODCABAL_HACKAGE_NAME = ${DISTNAME:C,-[0-9.]*$,,}
>> MODCABAL_HACKAGE_VERSION = ${DISTNAME:C,.*-([0-9.]*)$,\1,}
> and I think those variables might be a bit long, though very descriptive
> what's wrong with MODCABAL_STEM and MODCABAL_VERSION ?

Perfect, done.

>> .if !target(do-build)
>> do-build:
>> @${_MODCABAL_BUILD_TARGET}
>> .endif
>>
>> .if !target(do-install)
>> do-install:
>> @${_MODCABAL_INSTALL_TARGET}
>> .endif
>
> let's think these through. _MODCABAL_{BUILD,INSTALL}_TARGET are internal
> so not reusable from outside, strictly speaking.
>
> (this is probably taken from ruby.port.mk or something similar)

I was reading bsd.port.mk(5) and port-modules(5) instead. They provided
enough guidance to not require much peeking into examples. I probably
cribbed a thing or two from ghc.port.mk, but that's fair game :)

> If you define do-build/do-install manually, you might want to reference
> these.
>
> The reason the redirection exist is to allow non trivial build/install to
> happen when several modules are combined.
>
> in general, you may need to make these visible.

I erred on the side of hiding everything that I must not expose. It's
easier to expose than take away later. Do you feel I'm being too
cautious?

Thanks
Greg

No comments:

Post a Comment