Bug 256955 - dns/mDNSResponder_nss: convert port as a slave to net/mDNSResponder
Summary: dns/mDNSResponder_nss: convert port as a slave to net/mDNSResponder
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Nuno Teixeira
URL:
Keywords:
Depends on:
Blocks: 269333
  Show dependency treegraph
 
Reported: 2021-07-03 12:37 UTC by Matthieu Volat
Modified: 2023-07-03 08:25 UTC (History)
3 users (show)

See Also:
eduardo: maintainer-feedback+


Attachments
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder (2.00 KB, patch)
2021-07-03 12:37 UTC, Matthieu Volat
no flags Details | Diff
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder (2.15 KB, patch)
2023-02-06 10:51 UTC, Matthieu Volat
no flags Details | Diff
master/slave config v0 (3.08 KB, patch)
2023-06-03 07:13 UTC, Nuno Teixeira
no flags Details | Diff
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder (7.64 KB, patch)
2023-06-03 12:47 UTC, Matthieu Volat
mazhe: maintainer-approval+
Details | Diff
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder (3.04 KB, patch)
2023-06-03 15:06 UTC, Matthieu Volat
no flags Details | Diff
master port preparation v0 (990 bytes, patch)
2023-06-24 17:52 UTC, Nuno Teixeira
no flags Details | Diff
master port preparation v1 (1.06 KB, patch)
2023-06-24 18:08 UTC, Nuno Teixeira
no flags Details | Diff
242576: convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder (3.18 KB, patch)
2023-06-25 07:03 UTC, Matthieu Volat
no flags Details | Diff
unified net/mDNSResponder and dns/mDNSResponder_nss master/slave convertion v3 (3.24 KB, patch)
2023-06-26 05:25 UTC, Matthieu Volat
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthieu Volat 2021-07-03 12:37:56 UTC
Created attachment 226191 [details]
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder

Hi,

For a few years, I have been maintaining dns/mDNSResponder_nss, which is enabling the nsswitch plugin of net/mDNSResponder which is not available for FreeBSD builds in upstream.

This has been fine and working well, but it means tracking versions when this could be made a slave port. Until now, it was not, to avoid disruption in the net/mDNSResponder port, but I think I managed to do it and would like to submit it  if it seems reasonable.

--------
Overview
--------
Modify only dns/mDNSResponder_nss (the one I am maintaining). Rewrite it as a slave port as porter handbook present.

Both port only collide on the post-install target, and the master port install a rc script. Both post-install and install-rc-script targets are disabled through TARGET_ORDER_OVERRIDE.


--
QA
--

# portlint .
FATAL: Makefile: extra item "MASTERDIR" placed in the PORTNAME section.
FATAL: Makefile: extra item "LIB_DEPENDS" placed in the LICENSE section.
2 fatal errors and 0 warnings found.

I am not sure portlint has a place for MASTERDIR? I did try a few slave ports, all with this issue. Should I also add a LICENSE section, even if it is the same as the master port?

# make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
===> Checking for items in pkg-plist which are not in STAGEDIR
===> No pkg-plist issues found (check-plist)

# make package
===>  Building package for mDNSResponder_nss-1310.100.10


Any feedback is welcome, thanks

-- Mazhe
Comment 1 Nuno Teixeira freebsd_committer freebsd_triage 2023-02-06 10:03:03 UTC
Hello Matthieu,

To make this as a slave port, master need to be adapted and maintainer is sunpoet@.

I will send a feedback to him to check his opinion.

Cheers
Comment 2 Matthieu Volat 2023-02-06 10:51:53 UTC
Created attachment 239942 [details]
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder

Just in case, I am reuploading/updating the patch to the one I lived with in my port tree since 1.5 year.

A bit the same as initaly : portlint report pretty bad practices in this attempt, but I am not sure how to properly handle those.
Comment 3 Po-Chuan Hsieh freebsd_committer freebsd_triage 2023-04-02 08:07:01 UTC
It seems the patch (attachment 239942 [details]) does not touch net/mDNSResponder. Therefore, it does not need my approval. However, I cannot guarantee that net/mDNSRespondes updates would not break dns/mDNSResponder_nss if it is a slave port.
Comment 4 Matthieu Volat 2023-04-02 08:26:02 UTC
That would be fine as it was before, I maintain the small nsswitch module.

Things may very rarely break, but this would at least allow to auto-sync both mDNSResponder and nsswitch module 98% of the time.
Comment 5 Matthieu Volat 2023-05-30 06:18:41 UTC
Hi, I think we got everything, net/mDNSResponder maintainer OKed this (as it does not impact the main port) and as both proposer and dns/mDNSResponder_nss, I also OK this change.

Can it be commited?

Regards
Comment 6 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-03 07:13:28 UTC
Created attachment 242567 [details]
master/slave config v0

Hello,

Master port needs some ajustments in vars so slave port can be configured correctly:

**Master:

-CATEGORIES=
+CATEGORIES?= same categories but first category (physical) changed

-MAINTAINER=
+MAINTAINER?=  different maintainers

-COMMENT=
+COMMENT?=  different comments

-WWW=
+WWW?= different www's

-CONFLICTS_INSTALL=
+CONFLICTS_INSTALL?= different conflicts, slave can be configured to add it (+=) to master conflicts or to restrict to slave only (=)

**Slave:

-USES=           cpe  (already set in Master)
-CPE_VENDOR=     apple (already set in Master)
-PKGORIGIN= dns/mDNSResponder_nss (why we need that?)

Am I missing something?

Remember that slave port .include "${MASTERDIR}/Makefile" so a check need to be done to avoid duplicates and unwanted configs, etc.
Comment 7 Matthieu Volat 2023-06-03 12:47:18 UTC
Created attachment 242572 [details]
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder

Thanks, thihs is the kind of feedback I was hoping for!

I've tested your changes + removed redundant WWW, CPE* variables.

This works for me, but we need net/mDNSreponder appoval for the master part.
Comment 8 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-03 13:52:45 UTC
(In reply to Matthieu Volat from comment #7)

Please upload a clean diff for master/slave port. It seams that other changes are included and doesn't belong to this PR.
Comment 9 Matthieu Volat 2023-06-03 15:06:35 UTC
Created attachment 242576 [details]
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder

Oh my... Sorry, seems I messed up and brought local patches indeed. Apologies, here's the diff for only dns/mDNSResponder_nss and net/mDNSResponder.
Comment 10 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-03 16:38:51 UTC
Comment on attachment 242576 [details]
convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder

Hello sunpoet,

There's some small changes to master port (net/mDNSResponder) needed to slave (dns/mDNSResponder_nss) work correctly:

Change 'VAR=' to 'VAR?=' on:
CATEGORY, MAINTAINER, COMMENT, WWW and CONFLICTS_INSTALL.

Also I found that
"@${REINPLACE_CMD} -e '/^CFLAGS_DEBUG/ s|-Os|${CFLAGS}|' ${WRKSRC}/mDNSPosix/Makefile"" isn't doing anything since the closest match on ${WRKSRC}/mDNSPosix/Makefile is "CFLAGS_DEBUGGING = -O0 -DMDNS_DEBUGMSGS=0"

Maybe you want take a look at it so we can commit slave port.

Yours,
eduardo
Comment 11 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-24 17:50:32 UTC
maintainer timeout
Comment 12 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-24 17:52:34 UTC
Created attachment 242973 [details]
master port preparation v0

+ PORTREVISION?= so slave port doesn't depends on master revision
Comment 13 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-24 18:05:08 UTC
Please do all necessary tests with "master port preparation vN" uploaded diff.

While I am working on this some doubts have came out:

- pkg-message doesn't show up
- files dir include a patch, shouln't it be applied in master port?
- PORTREVISION must be set to 0 and increased only when slave port have changed and Master port should be set slave revision to 0 when updated (will include that in v1 patch)
Comment 14 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-24 18:08:33 UTC
Created attachment 242974 [details]
master port preparation v1

- add comment about reseting revision on slave port after an upgrade
Comment 15 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-24 18:12:16 UTC
(In reply to Nuno Teixeira from comment #13)
(...)

- does pkg-descr necessary?
- distinfo can be deleted as is same as master
Comment 16 Matthieu Volat 2023-06-25 06:03:58 UTC
Hi,

1. yes, distinfo should have been removed

2. I'm not sure what is wrong with pkg-descr?

3. pkg-message is problematic, I looked in doc and Mk/*.mk, apparently there is no override variable (appart SUB-ing a files/pkg-messages.in, and that won't work either here).

The only workaround for 3. I see would be the same "trick" as in php and nextcloud USE mk files, adding a custom target that create the pkg message file in the staging dir... It is not really pretty, I am open to other ideas, but if you are OK with it, I'll propose an updated patch.
Comment 17 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-25 06:43:42 UTC
(In reply to Matthieu Volat from comment #16)

> 3. pkg-message is problematic, I looked in doc and Mk/*.mk, apparently there
> is no override variable (appart SUB-ing a files/pkg-messages.in, and that
> won't work either here).

Have you try setting PATCHDIR and check if pkg-message.in is respected?

PATCHDIR= ${.CURDIR}/files
SUB_FILES= pkg-message
Comment 18 Matthieu Volat 2023-06-25 07:03:12 UTC
Created attachment 242983 [details]
242576: convert dns/mDNSResponder_nss as a slave port to net/mDNSResponder

I must have missed something, redefining PATCHDIR and using SUB do not help, but the variable controling this is PKGMESSAGE and quite a number of ports use it (I saw it in Mk/bsd.ports.mk but it seemed to be for the final, potentially SUB-ed, file).

Here is an updated patch that address 1. and 3.

Thanks for taking time to review this.
Comment 19 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-25 09:47:44 UTC
(In reply to Matthieu Volat from comment #18)

Nice! pkg-message is working.

Final check is related to a patch included in files (patch-mDNSPosix__nss_mdns.c) that it isn't applied:

===>  Patching for mDNSResponder_nss-1790.80.10_1
===>  Applying FreeBSD patches for mDNSResponder_nss-1790.80.10_1 from /usr/ports/dns/mDNSResponder_nss/../../net/mDNSResponder/files

Could you check that?
Maybe you need to configure PATCHDIR= or PATCHDIR+= to include patches from slave.

With this solved we can go forward. We need to be sure that master don't need more changes to accept new slave port.

Note to self:

 - set PORTREVISION=0 on slave
Comment 20 Matthieu Volat 2023-06-26 05:25:04 UTC
Created attachment 243000 [details]
unified net/mDNSResponder and dns/mDNSResponder_nss master/slave convertion v3

You are right, I wonder how some of this worked without, others would have been problematic at runtime (expecting config file in /etc, but I wonder if anybody has to change the default).

Here is a proposition of using EXTRA_PATCHES rather than completely overriding PATCHDIR, in case some of the master port patches would be also helpful for this port in the future. Is it OK for you?
Comment 21 commit-hook freebsd_committer freebsd_triage 2023-06-26 09:33:32 UTC
A commit in branch main references this bug:

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

commit e88ca25e34e4e4df55d03c7793767c6420206e4a
Author:     Nuno Teixeira <eduardo@FreeBSD.org>
AuthorDate: 2023-06-26 08:19:08 +0000
Commit:     Nuno Teixeira <eduardo@FreeBSD.org>
CommitDate: 2023-06-26 09:33:11 +0000

    net/mDNSResponder: Convert to master port

    Convert to master port of dns/mDNSResponder_nss (slave port).

    PR:             256955
    Approved by:    maintainer timeout, >2 weeks

 net/mDNSResponder/Makefile | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
Comment 22 commit-hook freebsd_committer freebsd_triage 2023-06-26 09:33:33 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=9c1fa59c5c8914145c95c66517604addfd36b27f

commit 9c1fa59c5c8914145c95c66517604addfd36b27f
Author:     Matthieu Volat <mazhe@alkumuna.eu>
AuthorDate: 2023-06-26 09:25:55 +0000
Commit:     Nuno Teixeira <eduardo@FreeBSD.org>
CommitDate: 2023-06-26 09:33:11 +0000

    dns/mDNSResponder_nss: Convert port as a slave to net/mDNSResponder

    PR:             256955

 dns/mDNSResponder_nss/Makefile        | 29 ++++++++++++-----------------
 dns/mDNSResponder_nss/distinfo (gone) |  3 ---
 2 files changed, 12 insertions(+), 20 deletions(-)
Comment 23 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-26 09:43:24 UTC
(In reply to Matthieu Volat from comment #20)

EXTRA_PATCHES works really nice:
---
===>  Patching for mDNSResponder_nss-1790.80.10
===>  Applying extra patch /usr/ports/dns/mDNSResponder_nss/files/patch-mDNSPosix__nss_mdns.c
===>  Applying FreeBSD patches for mDNSResponder_nss-1790.80.10 from /usr/ports/dns/mDNSResponder_nss/../../net/mDNSResponder/files
---

Added a missing field to respect slave pkg-descr:
---
DESCR=          ${.CURDIR}/pkg-descr
---

And grouped tweaks (inspired by lang/rust-nightly):
---
MASTERDIR=      ${.CURDIR}/../../net/mDNSResponder
EXTRA_PATCHES=  ${.CURDIR}/files/patch-mDNSPosix__nss_mdns.c
PLIST=          ${.CURDIR}/pkg-plist
PKGMESSAGE=     ${.CURDIR}/pkg-message
DESCR=          ${.CURDIR}/pkg-descr
---

Please update your ports tree and tell me if everything ok so I can close PRs.

Cheers
Comment 24 Matthieu Volat 2023-06-26 18:56:52 UTC
Updated from another machine, looks all good, thank you very much for your reviews, guidance, patience!

Cheers,
Comment 25 Nuno Teixeira freebsd_committer freebsd_triage 2023-06-26 19:25:50 UTC
(In reply to Matthieu Volat from comment #24)

Nice to hear that everything is fine!

Cheers
Comment 26 Po-Chuan Hsieh freebsd_committer freebsd_triage 2023-07-01 18:17:08 UTC
No. I don't want this. Please revert it.

It would be better not to use master/slave port here, especially when maintained by different people.

Since Apple does not update mDNSResponder frequently, it should be acceptable to update dns/mDNSResponder_nss when necessary.

Thanks.
Comment 27 Nuno Teixeira freebsd_committer freebsd_triage 2023-07-01 19:17:15 UTC
(In reply to Po-Chuan Hsieh from comment #26)

Like you said in #c3 this port doesn't touch master port and can be configured independent.
Comment 28 Matthieu Volat 2023-07-03 08:25:07 UTC
While it is true that some variable in net/mDNSResponder had to be changed to a conditional affectation, this looked an acceptable solution to properly sync the versions (which have diverged in the past and could stay so for a while in a quarterly version).

Could you share what burden you feel this change have on you? I can assure you this is not the intent and in the event a technical challenge would make this version not work, I would ask to revert to an "independent" port.