Bug 233324 - x11/libX11: Remove unnecessary perl5 dependency
Summary: x11/libX11: Remove unnecessary perl5 dependency
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-x11 (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-19 16:10 UTC by Steve Wills
Modified: 2021-06-27 11:42 UTC (History)
7 users (show)

See Also:
bugzilla: maintainer-feedback? (x11)
koobs: maintainer-feedback? (zeising)


Attachments
patch to remove perl dep from libX11 (496 bytes, patch)
2018-11-19 16:10 UTC, Steve Wills
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Wills freebsd_committer freebsd_triage 2018-11-19 16:10:11 UTC
libX11 seems to have a dependency on perl5 that isn't needed. The attached patch removes the dependency and the package does not seem to change at all. I think this will need an exp-run and review by maintainer.
Comment 1 Steve Wills freebsd_committer freebsd_triage 2018-11-19 16:10:49 UTC
Created attachment 199346 [details]
patch to remove perl dep from libX11

Somehow bugzilla failed to attach the patch. Here it is.
Comment 2 Niclas Zeising freebsd_committer freebsd_triage 2018-11-19 18:22:29 UTC
Talking to upstream a bit, it seems like perl is used during the build somehow.  I have to look more into this.
Comment 3 Steve Wills freebsd_committer freebsd_triage 2018-11-19 21:47:11 UTC
(In reply to Niclas Zeising from comment #2)
Configure seems to check for it, but I can't find any evidence that it uses it.
Comment 4 Mathieu Arnold freebsd_committer freebsd_triage 2018-11-22 16:34:15 UTC
no reason for portmgr to get involved here.
Comment 5 Steve Wills freebsd_committer freebsd_triage 2018-11-23 12:54:46 UTC
(In reply to Mathieu Arnold from comment #4)
Are you saying you think this doesn't need an exp-run? If so, great, just want to be clear.
Comment 6 Niclas Zeising freebsd_committer freebsd_triage 2019-01-06 09:39:02 UTC
According to upstream, perl is used during the build.  I haven't investigated further exactly where.
Comment 7 Jan Beich freebsd_committer freebsd_triage 2020-06-11 18:56:56 UTC
The code has clear references:
- specs/* but the port passes --disable-specs
- LOG_COMPILER but only used in "make check" (not "make test")

$ rg -g \*am PERL
specs/i18n/compose/Makefile.am
2:if HAVE_PERL
17:endif HAVE_PERL

specs/i18n/compose/docbook-nl.am
26:     $(AM_V_GEN)$(PERL) $(srcdir)/compose-chart.pl           \
30:     $(AM_V_GEN)$(PERL) $(srcdir)/compose-chart.pl           \

nls/Makefile.am
39:if HAVE_PERL
40:LOG_COMPILER = $(PERL)
42:endif HAVE_PERL
Comment 8 Kevin Bowling freebsd_committer freebsd_triage 2021-06-25 21:19:01 UTC
Thanks for your contribution!
Comment 9 commit-hook freebsd_committer freebsd_triage 2021-06-25 21:19:18 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=30b239b4ddbc30652ef5dd985136318fbd685bd9

commit 30b239b4ddbc30652ef5dd985136318fbd685bd9
Author:     Steve Wills <swills@FreeBSD.org>
AuthorDate: 2021-06-25 21:16:37 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2021-06-25 21:16:37 +0000

    x11/libX11: Remove unnecessary perl5 dependency

    PR:             233324
    Reviewed by:    jbeich
    Approved by:    maintainer timeout

 x11/libX11/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Comment 10 Niclas Zeising freebsd_committer freebsd_triage 2021-06-25 22:35:03 UTC
As both Jan and I have stated, perl5 is being used by the x11 build.  Please revert this ASAP.
Comment 11 Niclas Zeising freebsd_committer freebsd_triage 2021-06-25 22:45:20 UTC
Also, comitting this as maintainer timeout, when I clearly have given feedback and stated that the fix is not correct, is quite strange to me.
Comment 12 Michael Gmelin freebsd_committer freebsd_triage 2021-06-25 23:16:22 UTC
(In reply to Niclas Zeising from comment #11)

Just looking at this from a non-social perspective.

Jan wrote a year ago:

> The code has clear references:
> - specs/* but the port passes --disable-specs
> - LOG_COMPILER but only used in "make check" (not "make test")

and this still seems to be the case. Build works fine after the change (tried in a clean environment, "make check" also works from the source directory, but skips things due to perl not being there[0]). So I can understand from a port manager perspective why they want to remove the dependency and I don't see how this change could do any harm. Technically.

This doesn't solve the social question of course.

p.s. If "make check" with all possible tests is wanted in the build, it could be added as an option that also brings in perl as a build dependency. Right now it's not exposed in the port skeleton Makefile.

[0]
@HAVE_PERL_TRUE@LOG_COMPILER = $(PERL)
@HAVE_PERL_TRUE@TESTS = compose-check.pl
Comment 13 Kevin Bowling freebsd_committer freebsd_triage 2021-06-26 01:54:21 UTC
(In reply to Niclas Zeising from comment #10)
There seems to be some kind of misunderstanding that would best be fixed by anyone wanting to have an opinion looking at the software's build process and artifacts before commenting further.

jbeich did this for swills and the ticket was ignored for a year.  As a native English speaker what Jan said makes clear sense to me and he is not saying anywhere the patch is incorrect.  Instead it's uncontroversial evidence the dependency is frivolous in our build.  He also documented his approach, which is a better way to progress PRs like this:  rg is ripgrep which is a life changing tool for developers and maintainers.  Try it, it's great.

(In reply to Michael Gmelin from comment #12)
The port as it stands will never run 'make check', a specific bsd.port.mk feature would be needed to hit a target like that and it is not present.  Again, jbeich clearly documented the lack of use so the only issues are imagined and not factual.

There's no disrespect, it's getting the job done if there's a trivial PR this old without followup and a correct fix with multiple developer's eyes on it.

Time to move on there are much bigger fish to fry and this patch was integral from the beginning.
Comment 14 Warner Losh freebsd_committer freebsd_triage 2021-06-26 02:24:00 UTC
(In reply to Kevin Bowling from comment #13)
And the maintainer said no. So there was no maintainer timeout. That's the portmgr standard.

And if it was such a trivial issue, why bother force a fight over it?
Comment 15 Kevin Bowling freebsd_committer freebsd_triage 2021-06-26 03:03:11 UTC
(In reply to Warner Losh from comment #14)
Sorry, but you're simply wrong on all accounts and I don't care to bicker about it.  There is no fight.  Three ports committers that understood an issue put in the work and are done.
Comment 16 Niclas Zeising freebsd_committer freebsd_triage 2021-06-26 08:25:58 UTC
(In reply to Kevin Bowling from comment #15)

I'm sorry, but Warner is exactly right.  I left a comment  stating that perl is, according to upstream, being used, implicitly stating that the PR is not approved.  I should have been clearer in not approving the PR and just closed it with won't fix though.

Now I'm asking you, again, to please revert it.
Comment 17 Niclas Zeising freebsd_committer freebsd_triage 2021-06-26 08:28:48 UTC
The patch is also wrong.  You need to bump portrevision when changing dependencies.
Comment 18 Jan Beich freebsd_committer freebsd_triage 2021-06-26 08:43:51 UTC
(In reply to Niclas Zeising from comment #17)
> You need to bump portrevision when changing dependencies.

Citation needed. PORTREVISION bumps are for tracking changes to binary package contents.

https://docs.freebsd.org/en/books/porters-handbook/makefiles/#makefile-portrevision
Comment 19 Kevin Bowling freebsd_committer freebsd_triage 2021-06-26 08:45:10 UTC
(In reply to Niclas Zeising from comment #16)
Nothing will be reverted without technical justification because this is juvenile and lacks any technical rigor.  You had three years to do something.  You are not the only maintainer and this work doesn't belong to you.  I have no problem working with you but I won't tolerate petulant behavior from anyone.  Someone cared enough to submit it, another cared enough to do code inspection, and I cared enough to review all that and commit it with a fair maintainer timeout.  Find another PR to work on because this one's done.

(In reply to Niclas Zeising from comment #17)
It's fine, the commit immediately followed the version bump on purpose.  Since it doesn't change the artifact in a notable way (due to the above inspections) there's no need to waste time closing this small race condition.
Comment 20 Michael Gmelin freebsd_committer freebsd_triage 2021-06-26 09:03:52 UTC
(In reply to Kevin Bowling from comment #13)
(In reply to Kevin Bowling from comment #13)

I think you misunderstood what I wrote.

> (In reply to Michael Gmelin from comment #12)
> The port as it stands will never run 'make check', a specific bsd.port.mk
> feature would be needed to hit a target like that and it is not present.  

What I meant is: One could add an OPTION to the port that runs "make check" as part of the build process and then add perl as a dependency (hypothetical, future development, wouldn't need changes to bsd.port.mk. But no need to depend on perl unconditionally right now, so it could be removed).

> Again, jbeich clearly documented the lack of use so the only issues are imagined and not factual.

That's exactly what I said and confirmed - I spent the time to double-check if it's still true (including running "make check" in SRCDIR), as Jan's comment was from a year ago. So we both agree that perl is not required in the build process. I think the dependency should be removed, like you do.

BUT

Calling a maintainer timeout when the maintainer said "no" is a problem, even if the PR is old and seems obvious.
Comment 21 Michael Gmelin freebsd_committer freebsd_triage 2021-06-26 09:09:29 UTC
(In reply to Kevin Bowling from comment #19)

> You are not the only maintainer and this work doesn't belong to you.

That's actually an interesting point - maintainership of
X11 related ports is a bit opaque from the outside.
Comment 22 Niclas Zeising freebsd_committer freebsd_triage 2021-06-26 09:14:35 UTC
(In reply to Michael Gmelin from comment #21)

I thought it was very clear who X11 are, but someone pointed out that the list of x11 maintainers that was on the wiki has been removed.

In any case, it's me and manu who are the most active at the moment.  Bapt is helping out from time to time, as is jkim.  kwm and miwi used to be, but I haven't seen either around the project in a long time.  jmd used to help out with some things, especially related to gpu firmware.  johalun did the drivers, but he has stepped down.
Comment 23 Warner Losh freebsd_committer freebsd_triage 2021-06-26 14:37:00 UTC
(In reply to Kevin Bowling from comment #19)
> You are not the only maintainer and this work doesn't belong to you.  I have no > problem working with you but I won't tolerate petulant behavior from anyone.

Kevin, actually x11@ does own this port. Like all ports that have mailing lists listed as MAINTAINER=, a group decides what to do about the port. This isn't petulant behavior to push back at an unauthorized change by members of that MAINTAINER list. It's the standard way things work in the ports tree, and is somewhat different than the src tree. In addition, there has been real breakage that caused the group considerable time in the past by uncoordinated changes, which is why you are seeing the reaction you are seeing. 

I've already invited you to the x11 bi-weekly meetings. Let's work together in the future to get more work done in this area. I'd much rather work together to get things done than continue to litigate the details of this incident.
Comment 24 Kevin Bowling freebsd_committer freebsd_triage 2021-06-26 19:39:05 UTC
(In reply to Warner Losh from comment #23)
Which is why I invoked maintainer timeout.  Please stop repeating false claims, you do this from time to time until people fatigue in dealing with you and it is annoying behavior that wont work on me.  Jan used unix tools and the PR system to clearly communicate, and the maintainer(s) abdicated responsibility for over a year so this isn't some nit or sniping at the two week window.  And there's no negative judgement from me for that, people get busy or are working on other things so one hand washes the other and both wash the face.  Nobody holds exclusive locks on a PR for 3-4 years while not doing the work.  A correct build optimization that reduces unnecessary time, flash wear and electricity waste on builders is in and it is you and Niclas that keep trying to litigate nothing, the rest of us have moved on.
Comment 25 George V. Neville-Neil freebsd_committer freebsd_triage 2021-06-26 20:10:14 UTC
A quick note from core@.  The conversation on this bug has clearly gotten contentious, if not a bit abusive.  We've already received a complaint from those simply watching this bug about the level of conversation.  So, in short, please bring this back to the level of appropriate discussion from here on out.

If a reversions is necessary and requested by those working in this area, then that is what should happen next.
Comment 26 Scott Long freebsd_committer freebsd_triage 2021-06-27 02:38:56 UTC
It seems like the commit was based on sound technical analysis, data, and testing, and the results have been a non-event.  Speaking with my core@ hat on, I see no reason to back out the commit.  Doing so would be to "prove a point", which is not what this project should be about.  We need more people learning, contributing, and growing the project, not barriers, slow-downs, and procedural paralysis.
Comment 27 Michael Gmelin freebsd_committer freebsd_triage 2021-06-27 11:42:28 UTC
(In reply to Niclas Zeising from comment #22)

Maybe it would help if important teams like x11@ would be presented like administrative teams (https://www.freebsd.org/administration) and contain some info how to get in touch/interact with them efficiently - especially for teams which might use non-project-provided resources to coordinate their efforts.

> kwm and miwi used to be,

Did anybody hear from miwi lately? He seems to have disappeared, which is a bit worrisome, given that his twitter states "Kuala Lumpur" as his location.