Bug 251096 - sysutils/lsof: Update to 4.94
Summary: sysutils/lsof: Update to 4.94
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: Larry Rosenman
URL: https://github.com/lsof-org/lsof/rele...
: 251266 (view as bug list)
Depends on:
Reported: 2020-11-13 09:38 UTC by Yasuhiro Kimura
Modified: 2021-01-30 13:13 UTC (History)
6 users (show)

See Also:
bugzilla: maintainer-feedback? (ler)
dim: merge-quarterly+

Patch file (9.53 KB, patch)
2020-11-13 09:38 UTC, Yasuhiro Kimura
no flags Details | Diff
Updated patch file (10.02 KB, patch)
2020-11-18 10:05 UTC, Yasuhiro Kimura
no flags Details | Diff
Updated patch file (10.38 KB, patch)
2020-11-21 19:47 UTC, Yasuhiro Kimura
no flags Details | Diff
Updated patch file (10.38 KB, patch)
2020-11-27 02:43 UTC, Yasuhiro Kimura
no flags Details | Diff
Patch using new pwddesc structure instead (2.32 KB, patch)
2020-12-29 21:56 UTC, Dimitry Andric
no flags Details | Diff
Updated patch file (10.64 KB, patch)
2021-01-01 01:28 UTC, Yasuhiro Kimura
no flags Details | Diff
Build on 14-CURRENT (10.76 KB, patch)
2021-01-30 03:06 UTC, Cy Schubert
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yasuhiro Kimura freebsd_committer 2020-11-13 09:38:34 UTC
Created attachment 219630 [details]
Patch file

* Update to 4.94.
* Update or remove patches that are included in upstream.

Change Log: https://github.com/lsof-org/lsof/releases/tag/4.94.0
Comment 1 Yasuhiro Kimura freebsd_committer 2020-11-18 10:05:16 UTC
Created attachment 219789 [details]
Updated patch file

Fix build with __FreeBSD_version >= 1300130.
Comment 2 Tomoaki AOKI 2020-11-21 06:49:49 UTC
(In reply to Yasuhiro KIMURA from comment #1)

This allows me to build and install on head. Thanks!
Comment 3 Cy Schubert freebsd_committer 2020-11-21 15:27:40 UTC
Checking __FreeBSD_version >= 1300130 is less reliable than checking for PWDDESC_KVM_LOAD_PWD. Wouldn't you think?
Comment 4 Yasuhiro Kimura freebsd_committer 2020-11-21 19:47:19 UTC
Created attachment 219864 [details]
Updated patch file

Fix build with Clang 11 without setting 'CFLAGS+=-fcommon' in Makefile.
Comment 5 Yasuhiro Kimura freebsd_committer 2020-11-21 19:54:16 UTC
(In reply to Cy Schubert from comment #3)

What do you mean? In base r367777 FILEDESC_KVM_LOAD_PWD is replaced with PWDDESC_KVM_LOAD_PWD and also __FreeBSD_version is bumped to 1300130. So there is no difference between checking if PWDDESC_KVM_LOAD_PWD is defined and checking if __FreeBSD_version is 1300130 or later.
Comment 6 Cy Schubert freebsd_committer 2020-11-21 22:34:23 UTC
I really shouldn't have to explain myself as it should be obvious: There's a huge difference. If or when patch is MFCed, you or someone will need to add additional #if logic to fix the build yet again, this time for -STABLE, making for complex CPP logic. That's another PR and another patch that needs to be applied, additional churn to the repository.
Comment 7 Tomoaki AOKI 2020-11-22 01:42:30 UTC
(In reply to Cy Schubert from comment #6)

One thing to mention.
There is no MFC target specified in the commit message.
If it's not a mistake, checking for __FreeBSD_version would be sufficient.

Moreover, MFC'ing the commit could be POLA violation.
There is no warranty that no other software, regardless in ports tree or in the wild, depend on the renamed macro.

Of course, you are right if MFC is planned and the change is NOT considered as POLA violation.
Comment 8 Cy Schubert freebsd_committer 2020-11-22 01:51:21 UTC
MFC can happen even if MFC after is not specified. I've done it. And, anyone can MFC another committer's commits. Especially if the commit is a prereq.

Anyhow, it's up to the port maintainer wants to do and maintainability. But personally, I'd try to avoid churn from the get-go.
Comment 9 Yasuhiro Kimura freebsd_committer 2020-11-27 02:43:56 UTC
Created attachment 220023 [details]
Updated patch file

Check if PWDDESC_KVM_LOAD_PWD instead of __FreeBSD_version is 1300130 or later.
Comment 10 Yasuhiro Kimura freebsd_committer 2020-11-27 02:49:11 UTC
(In reply to Cy Schubert from comment #6)

Thanks for explanation. I didn't think of MFC at all.
Comment 11 Alexander Kabaev freebsd_committer 2020-12-23 20:14:37 UTC
This bit cannot possibly ever be correct:
++#else   /* defined(PWDDESC_KVM_LOAD_PWD) */
+ 	    pwd_addr = (KA_T)FILEDESC_KVM_LOAD_PWD(&fd);
you are passing filedesc to where pwddesc is expected.
Comment 12 Dimitry Andric freebsd_committer 2020-12-29 21:56:22 UTC
Created attachment 221093 [details]
Patch using new pwddesc structure instead

(In reply to Alexander Kabaev from comment #11)

Yes, indeed. Here's an alternative patch for the dproc.c part, which hopefully does the right thing. Builds OK, and while I can run lsof just fine, I'm unsure if I have correctly exercised this new part of the code.
Comment 13 Yasuhiro Kimura freebsd_committer 2021-01-01 01:28:24 UTC
Created attachment 221153 [details]
Updated patch file

Include Dimitry's patch.
Comment 14 Cy Schubert freebsd_committer 2021-01-30 02:53:53 UTC
Is there a reason this hasn't been committed yet?
Comment 15 Cy Schubert freebsd_committer 2021-01-30 03:06:57 UTC
Created attachment 222017 [details]
Build on 14-CURRENT

Build and install on 14-CURRENT.
Comment 16 Dimitry Andric freebsd_committer 2021-01-30 13:04:36 UTC
*** Bug 251266 has been marked as a duplicate of this bug. ***
Comment 17 commit-hook freebsd_committer 2021-01-30 13:12:58 UTC
A commit references this bug:

Author: dim
Date: Sat Jan 30 13:12:17 UTC 2021
New revision: 563403
URL: https://svnweb.freebsd.org/changeset/ports/563403

  sysutils/lsof: Update to 4.94

  * Update to 4.94.
  * Update or remove patches that are included in upstream.
  * Fix build with Clang 11 without setting 'CFLAGS+=-fcommon' in Makefile.
  * Use new pwddesc structure if applicable.

  Change Log: https://github.com/lsof-org/lsof/releases/tag/4.94.0

  Submitted by:	Yasuhiro Kimura <yasu@utahime.org>
  PR:		251096
  MFH:		2021Q1