Bug 240390

Comment 1 Kubilay Kocak 2019-09-14 11:31:25 UTC

Comment on attachment 207259 [details]
new port .shar

New (uncommitted) ports don't (cant) need maintainer-approval

Comment 2 Kai Knoblich 2019-09-15 11:38:09 UTC

Hi Alfonso,

thank you for your contribution and the work on sysctlinfo! The port builds fine on 12.0-RELEASE and 13.0-CURRENT@r352110 amd64 + i386.

On 11.2-, 11.3-RELEASE amd64 + i386 the ports fails to build and following error messages are emitted:

> sysctlinfo.c:292:12: error: no member named 'oid_label' in 'struct sysctl_oid'; did you mean 'oid_name'?
>                 if (oid->oid_label == NULL) {
>                          ^~~~~~~~~
>                          oid_name
> /usr/src/sys/sys/sysctl.h:186:14: note: 'oid_name' declared here
>         const char      *oid_name;
>                         ^
> sysctlinfo.c:300:37: error: no member named 'oid_label' in 'struct sysctl_oid'; did you mean 'oid_name'?
>                         error = SYSCTL_OUT_STR(req, oid->oid_label);
>                                                          ^~~~~~~~~
>                                                          oid_name
> /usr/src/sys/sys/sysctl.h:196:56: note: expanded from macro 'SYSCTL_OUT_STR'
> #define SYSCTL_OUT_STR(r, p)    (r->oldfunc)(r, p, strlen(p) + 1)
>                                                          ^
> /usr/src/sys/sys/sysctl.h:186:14: note: 'oid_name' declared here
>         const char      *oid_name;
>                         ^
> sysctlinfo.c:300:37: error: no member named 'oid_label' in 'struct sysctl_oid'; did you mean 'oid_name'?
>                         error = SYSCTL_OUT_STR(req, oid->oid_label);
>                                                          ^~~~~~~~~
>                                                          oid_name
> /usr/src/sys/sys/sysctl.h:196:46: note: expanded from macro 'SYSCTL_OUT_STR'
> #define SYSCTL_OUT_STR(r, p)    (r->oldfunc)(r, p, strlen(p) + 1)
>                                                 ^
> /usr/src/sys/sys/sysctl.h:186:14: note: 'oid_name' declared here
>         const char      *oid_name;
>                          ^
> 3 errors generated.
> *** Error code 1

For the case if this is expected/intended then we'll need to exclude out those releases from the Makefile, e.g. in a similar way as "graphics/drm-current-kmod" does.

At last but not least, I have two suggestions:

- The port is a kernel module so it makes sense to rename it to "sysutil/sysctlinfo-kmod" to make it more distinguishable.
- Maybe the pkg-descr can have 2-3 more lines with an info that's the port is a kernel module.

What do you think about it?

Comment 3 Alfonso S. Siciliano 2019-09-15 17:59:45 UTC

Created attachment 207510 [details]
new port .shar (sysctlinfo-kmod)

Thank you for your report,

Makefile fixed (FreeBSD 11 has not the `char *oid_label'), testing:

 * poudriere testport -j 130Ramd64 -p default -i -o sysutils/sysctlinfo-kmod
 * poudriere testport -j 120Ramd64 -p default -i -o sysutils/sysctlinfo-kmod
 * poudriere testport -j 113Ramd64 -p default -i -o sysutils/sysctlinfo-kmod


> - The port is a kernel module so it makes sense to rename it to "sysutil/sysctlinfo-kmod" to make it more distinguishable.
> - Maybe the pkg-descr can have 2-3 more lines with an info that's the port is a kernel module.

> What do you think about it?

It's right, the attachment (.shar) is updated


Alfonso

Comment 4 Conrad Meyer 2019-09-15 19:44:52 UTC

Hi Alfonso,

Would you consider adapting this for inclusion in base instead of ports?  The sysctl kernel interface is old and crufty and clearly doesn't meet your needs; I don't see why an improved interface should live out of tree.

(For others looking for context, the Gitlab link is currently in the URL field and I'll copy it here: https://gitlab.com/alfix/sysctlinfo .)

Comment 5 Alfonso S. Siciliano 2019-09-18 04:06:13 UTC

Hi Conrad,

I created a review D21700

Comment 6 Kai Knoblich 2019-09-20 06:46:19 UTC

(In reply to Alfonso S. Siciliano from comment #3)
Thank you for the updated patch.

(In reply to Conrad Meyer from comment #4)
(In reply to Alfonso S. Siciliano from comment #5)

Should we wait for the further process in the review (e.g. if greater changes are required) or can we handle this PR independently?

If the the latter is true, I could commit port in the next few days.

Comment 7 Conrad Meyer 2019-09-20 14:08:29 UTC

Go ahead with the port for now, Kai; I won't have time to look at this any time soon :(.

Comment 8 Kai Knoblich 2019-09-23 20:35:30 UTC

Created attachment 207750 [details]
sysctlinfo-kmod-revised.patch

(In reply to Conrad Meyer from comment #7)

Alright, will do so then. Thanks for your feedback.

(In reply to Alfonso S. Siciliano from comment #3)

Attached is a small modification for the IGNORE statement. It gives a bit more info about the introduction of "oid_label" that occurred in base r310051 (= FreeBSD 1200019 / base r310149).

Once review D21700 lands we can then expand the statement (e.g. "|| ${OSVERSION} >= 13000NN)" to exclude the build for that and later versions.

If you're fine with that small change we're all set and the port is ready to land.

Comment 9 commit-hook 2019-09-24 09:41:57 UTC

A commit references this bug:

Author: kai
Date: Tue Sep 24 09:41:23 UTC 2019
New revision: 512707
URL: https://svnweb.freebsd.org/changeset/ports/512707

Log:
  [NEW PORT]: sysutils/sysctlinfo-kmod

  Kernel module to implement the sysctlinfo interface, it explores
  the sysctl tree to pass the info of the nodes to the userland;
  a header file with constants and helper macros is provided, too.

  WWW: https://gitlab.com/alfix/sysctlinfo

  PR:		240390
  Submitted by:	Alfonso S. Siciliano <alfix86@gmail.com>

Changes:
  head/sysutils/Makefile
  head/sysutils/sysctlinfo-kmod/
  head/sysutils/sysctlinfo-kmod/Makefile
  head/sysutils/sysctlinfo-kmod/distinfo
  head/sysutils/sysctlinfo-kmod/pkg-descr
  head/sysutils/sysctlinfo-kmod/pkg-message

Comment 10 Kai Knoblich 2019-09-24 09:51:12 UTC

Committed, thank you for the contribution and the work on the sysctlinfo interface!