Page MenuHomeFreeBSD
Differential D32907

net/dhcpcd: Use HAS_CONFIGURE over GNU_CONFIGURE
ClosedPublic
Actions

Authored by driesm on Nov 9 2021, 3:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 6, 12:44 PM
Unknown Object (File)
Mon, Nov 4, 9:02 AM
Unknown Object (File)
Oct 18 2024, 11:12 AM
Unknown Object (File)
Oct 17 2024, 10:24 AM
Unknown Object (File)
Oct 16 2024, 2:11 AM
Unknown Object (File)
Oct 14 2024, 5:54 PM
Unknown Object (File)
Oct 13 2024, 10:12 PM
Unknown Object (File)
Oct 12 2024, 1:38 AM
View All 89 Files
Subscribers
mat

Details

Reviewers
diizzy
0mp
philip
Commits
R11:a36a14a79809: net/dhcpcd: Use HAS_CONFIGURE over GNU_CONFIGURE
Summary

Commit message:

net/dhcpcd: Use HAS_CONFIGURE over GNU_CONFIGURE

Historically the port used GNU_CONFIGURE but this is not needed.
Switch to HAS_CONFIGURE and adjust the needed configure args.
While here install the sample config file under etc/.

 Reported by:	diizzy
 Approved by:	tbd (mentor)
 Differential Revision: https://reviews.freebsd.org/D32907
Test Plan

Poudriere testport looks good

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 42700
Build 39588: arc lint + arc unit

Event Timeline

driesm created this revision.Nov 9 2021, 3:14 PM
Herald added a subscriber: mat. · View Herald TranscriptNov 9 2021, 3:14 PM
driesm requested review of this revision.Nov 9 2021, 3:14 PM
Harbormaster completed remote builds in B42675: Diff 98244.Nov 9 2021, 3:15 PM
driesm edited the summary of this revision. (Show Details)Nov 9 2021, 3:15 PM
driesm added reviewers: diizzy, 0mp, philip.
driesm edited the summary of this revision. (Show Details)
driesm edited the summary of this revision. (Show Details)
driesm edited the summary of this revision. (Show Details)Nov 9 2021, 4:20 PM
0mp requested changes to this revision.Nov 9 2021, 9:23 PM
0mp added inline comments.
net/dhcpcd/Makefile
26

Please use INSTALL_DATA instead of CP.

In general, there should be no need to ever use CP in the install targets.

I think that the reason is with INSTALL_* you control the ownership a bit more tight. Also there are some race conditions with cp but that may be an urban legend.

Also, shouldn't this be dhcpcd.conf.sample?

This revision now requires changes to proceed.Nov 9 2021, 9:23 PM
philip added a comment.Nov 10 2021, 2:29 AM

What @0mp said regarding ${INSTALL_DATA} instead of ${CP}.

Since it's only one configuration file, I would not bother creating an %%EXAMPLESDIR%% for it. It's easier to simply ${INSTALL_DATA} to ${STAGEDIR}${PREFIX}/etc/dhcpcd.conf.sample and mark it as @sample in pkg-plist.

driesm updated this revision to Diff 98300.Nov 10 2021, 9:10 AM

Switch to ${INSTALL_DATA} and install sample config file under etc/

Harbormaster completed remote builds in B42696: Diff 98300.Nov 10 2021, 9:10 AM
driesm edited the summary of this revision. (Show Details)Nov 10 2021, 9:11 AM
driesm edited the summary of this revision. (Show Details)
0mp accepted this revision.Nov 10 2021, 9:53 AM

Just last minor nit.

Otherwise, it seems like it's ready to go!

net/dhcpcd/pkg-plist
14

I think that if the first argument to @sample ends with ".sample" then there is no need to specify the second argument. ie, @sample etc/dhcpcd.conf.sample should be sufficient.

This revision is now accepted and ready to land.Nov 10 2021, 9:53 AM
diizzy added inline comments.Nov 10 2021, 10:31 AM
net/dhcpcd/pkg-plist
14

Yes, that's correct.

driesm updated this revision to Diff 98305.Nov 10 2021, 10:35 AM

drop second argument of @sample as it is not needed.

This revision now requires review to proceed.Nov 10 2021, 10:35 AM
driesm marked 3 inline comments as done.Nov 10 2021, 10:35 AM
Harbormaster completed remote builds in B42698: Diff 98305.Nov 10 2021, 10:35 AM
0mp requested changes to this revision.Nov 10 2021, 11:45 AM
This revision now requires changes to proceed.Nov 10 2021, 11:45 AM
0mp added inline comments.Nov 10 2021, 11:50 AM
net/dhcpcd/Makefile
26

Sorry, I've not seen this before. I think that we should actually use MV here instead of INSTALL_DATA. The reason is that we don't need ${STAGEDIR}${PREFIX}/etc/dhcpcd.conf in the stage dir anymore. The @sample keyword is going to handle the creation ${STAGEDIR}${PREFIX}/etc/dhcpcd.conf based on the contents of ${STAGEDIR}${PREFIX}/etc/dhcpcd.conf.sample automatically.

driesm updated this revision to Diff 98309.Nov 10 2021, 12:00 PM

use MV over INSTALL_DATA, @sample handles copying the sample.

Harbormaster completed remote builds in B42700: Diff 98309.Nov 10 2021, 12:00 PM
0mp accepted this revision.Nov 10 2021, 12:04 PM

Assuming that it still builds fine, you have my approval!

This revision is now accepted and ready to land.Nov 10 2021, 12:04 PM
Closed by commit R11:a36a14a79809: net/dhcpcd: Use HAS_CONFIGURE over GNU_CONFIGURE (authored by driesm). · Explain WhyNov 13 2021, 3:33 PM
This revision was automatically updated to reflect the committed changes.
driesm added a commit: R11:a36a14a79809: net/dhcpcd: Use HAS_CONFIGURE over GNU_CONFIGURE.

Revision Contents
Changeset List

PathSize
net/
dhcpcd/
8 lines
8 lines

Diff 98309

View Options

net/dhcpcd/Makefile

Loading...
View Options

net/dhcpcd/pkg-plist

Loading...