haskell-infra / hackage-trustees

Issue tracker for Hackage maintainance and trustee operations
https://hackage.haskell.org/packages/trustees/
42 stars 7 forks source link

NMU: terminal-size patch #310

Closed hasufell closed 2 years ago

hasufell commented 3 years ago

https://github.com/biegunka/terminal-size/pull/16

@bgamari

hasufell commented 2 years ago

Can this move forward?

hasufell commented 2 years ago

ping

This blocks ghcup from being uploaded to hackage, btw.

Bodigrim commented 2 years ago

@hasufell terminal-size supports GHCs back to 7.0, while the PR linked requires CApiFFI, which is unavailable before GHC 7.10. This is a decision which I would very much prefer to be taken by a package maintainer instead of Hackage Trustees. Any chance you'd be interested to take over the package, if maintainers are absent?

phadej commented 2 years ago

CAPiFFI is available since GHC-7.6. @bgamari's blog post is incorrect in that (important) detail. Dropping pre GHC-7.6 support in patch version is fine: it's very old GHC and old versions of terminal-size are still there. (And these old GHCs don't work on Apple M1, so the bug doesn't surface).

Bodigrim commented 2 years ago

@phadej GHC manual is also incorrect in this case: http://downloads.haskell.org/ghc/9.2.1-rc1/docs/html/users_guide/exts/ffi.html#the-capi-calling-convention

I agree that from practical viewpoint it is fine, but I'd like to exhaust other possible options before going for NMU.

phadej commented 2 years ago

The patch itself is not good enough for NMU, at least other-extensions: CApiFFI should be declared. Also IIRC

               9.2.0.20210821  9.0.1  8.10.4  8.8.4  8.6.5  8.4.4  8.2.2  8.0.2  7.10.3  7.8.4  7.6.3  7.4.2  7.2.2  7.0.4
terminal-size  NO-IP           OK     OK      OK     OK     OK     OK     OK     OK      OK     OK     OK     FAIL   FAIL 

Build profile: -w ghc-7.2.2 -O0
In order, the following will be built (use -v for more details):
 - terminal-size-0.3.2.1 (lib) (first run)
Configuring library for terminal-size-0.3.2.1..
Preprocessing library for terminal-size-0.3.2.1..
Building library for terminal-size-0.3.2.1..

src/System/Console/Terminal/Posix.hsc:1:14:
    Unsupported extension: CApiFFI

Build profile: -w ghc-7.0.4 -O0
In order, the following will be built (use -v for more details):
 - terminal-size-0.3.2.1 (lib) (first run)
Configuring library for terminal-size-0.3.2.1..
Preprocessing library for terminal-size-0.3.2.1..
Building library for terminal-size-0.3.2.1..

src/System/Console/Terminal/Posix.hsc:1:14:
    Unsupported extension: CApiFFI
phadej commented 2 years ago

@Bodigrim the policy requires us (= Trustees) to contact the maintainer too. And wait some time so they (hopefully) reply.

Addition: that step is non-commiting anything:

Step 3 in https://github.com/haskell-infra/hackage-trustees/blob/master/policy.md#policyprocedure-2

When the issue is opened, a trustee must try to contact the maintainer(s), primarily by e-mail to try and resolve things without needing to force anything. They must record in the ticket when they first tried to contact the maintainer(s). This documents the start of the 2-week deadline. (Many users do not have github notifications set up.)

phadej commented 2 years ago

Btw, terminal-size is not buildable with cabal-install because there is no hsc2hs which is buildable with GHC-9.2. The https://github.com/haskell/hsc2hs/issues/61 issue can be modified to GHC-9.2 one.

That's one problem @bgamari could solve.

hasufell commented 2 years ago

Any chance you'd be interested to take over the package, if maintainers are absent?

Not interested. I'll probably just fork the modules into my own repo.

hasufell commented 2 years ago

So what's the reason this isn't fixed? Due to the lack of action here I've already forked it into my own repos, so my projects aren't blocked from being used on hackage.

Regardless, this bug may break other peoples packages.

Bodigrim commented 2 years ago

@hasufell The reason is that Hackage Trustee are not "maintainers of everything" and are foremost responsible to exclude unbuildable configurations by manipulating package bounds. NMUs are extremely rare and mostly limited to mechanical migration of impactful packages to newer versions of dependencies.

Generally speaking, we have no expertise or domain knowledge to approve bug fixes, especially of subtle nature, as the PR in question. That's why I am not comfortable to undertake a NMU in this case. Other Trustees may be of different opinions.

hasufell commented 2 years ago

Generally speaking, we have no expertise or domain knowledge to approve bug fixes

What? You're part of CLC. The bug was reported by a GHC developer, who's extremely knowledgeable about C API and published a blog post about these sort of issues: https://www.haskell.org/ghc/blog/20210709-capi-usage.html

gbaz commented 2 years ago

I double checked and the point about nmu procedure is that it is for trustees to fix version bump induced issues: https://github.com/haskell-infra/hackage-trustees/blob/master/policy.md#3-source-changes-simple-patches

Fixing other bugs is not in the purview of people as trustees and not in the purview of the nmu process. Afaik the only approved process we have for fixing bugs that are not simply version bump induced breakages is a package takeover (which can mean being added as a maintainer, rather than simply kicking out the prior maintainer).

Thanks for pushing for clarity on this, and my apologies for not paying attention and trying to clear this up sooner. I'm going to close this for now -- acting on stuff like this will go beyond what the role of trustees is supposed to be and set precedents that could cause more confusion in the future.

hasufell commented 2 years ago

Thanks for the clarification... the wording is a bit implicit. Maybe it should be clarified.