Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NMU: terminal-size patch #310

Closed
hasufell opened this issue Jul 3, 2021 · 14 comments
Closed

NMU: terminal-size patch #310

hasufell opened this issue Jul 3, 2021 · 14 comments
Labels

Comments

@hasufell
Copy link

hasufell commented Jul 3, 2021

biegunka/terminal-size#16

@bgamari

@gbaz gbaz added the NMU label Aug 10, 2021
@hasufell
Copy link
Author

Can this move forward?

@hasufell
Copy link
Author

hasufell commented Aug 24, 2021

ping

This blocks ghcup from being uploaded to hackage, btw.

@Bodigrim
Copy link

@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
Copy link
Member

phadej commented Sep 21, 2021

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
Copy link

@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
Copy link
Member

phadej commented Sep 21, 2021

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
Copy link
Member

phadej commented Sep 21, 2021

@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
Copy link
Member

phadej commented Sep 21, 2021

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

That's one problem @bgamari could solve.

@hasufell
Copy link
Author

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
Copy link
Author

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
Copy link

@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
Copy link
Author

hasufell commented Oct 11, 2021

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
Copy link
Contributor

gbaz commented Oct 11, 2021

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.

@gbaz gbaz closed this as completed Oct 11, 2021
@hasufell
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants