Page MenuHomeFreeBSD

rc.d/NETWORKING: remove the NETWORK alias
ClosedPublic

Authored by cat1890693_gmail.com on Wed, Apr 8, 7:58 AM.
Tags
None
Referenced Files
F153473436: D56300.id175341.diff
Tue, Apr 21, 9:00 AM
F153453696: D56300.diff
Tue, Apr 21, 6:32 AM
F153441098: D56300.id175341.diff
Tue, Apr 21, 4:51 AM
F153436511: D56300.id175344.diff
Tue, Apr 21, 4:09 AM
F153431414: D56300.id175342.diff
Tue, Apr 21, 3:27 AM
F153429377: D56300.id.diff
Tue, Apr 21, 3:12 AM
Unknown Object (File)
Sun, Apr 19, 11:32 PM
Unknown Object (File)
Sat, Apr 18, 6:28 AM
Subscribers

Details

Summary

NETWORKING is the documented placeholder, while
/etc/rc.d/NETWORKING still provides the legacy alias
NETWORK.

The NETWORKING script was originally introduced to avoid
conflicts with NetBSD's lowercase network script on
case-insensitive file systems. The NETWORK alias was
retained for compatibility with older scripts.

Following the discussion in PR 293652, remove the legacy
NETWORK alias from 16-CURRENT. Keeping both names adds
more confusion than value now that NETWORKING is the
documented placeholder and current base system and ports
tree uses are already clean.

Add an UPDATING entry to note that local RC scripts using
REQUIRE: NETWORK should be migrated to REQUIRE: NETWORKING.

Bug: 293652
Relnotes: yes

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

cat1890693_gmail.com edited the test plan for this revision. (Show Details)
cat1890693_gmail.com edited the test plan for this revision. (Show Details)

This also needs Relnotes: yes and an entry in UPDATING. For me, a documentation issue dropping undocumented stuff.

This revision now requires changes to proceed.Sat, Apr 11, 6:19 PM

This also needs Relnotes: yes and an entry in UPDATING. For me, a documentation issue dropping undocumented stuff.

Thank you, that makes sense.

Before I update the patch, I want to make sure I understood your suggestion correctly.

My understanding is that, if we want to proceed with removing the NETWORK
alias, this review should also include an entry in UPDATING noting that local
rc scripts using REQUIRE: NETWORK need to be migrated to REQUIRE: NETWORKING,
and the change should be marked accordingly for relnotes.

I am still new to preparing FreeBSD patches, so I would also like to confirm
the mechanics here: if an UPDATING entry is needed, should I add it directly
to src/UPDATING? I also do not see a Relnotes field in the revision edit UI
on my side, so should I interpret this as a request to update src/RELNOTES as
well?

Is that what you had in mind?

This also needs Relnotes: yes and an entry in UPDATING. For me, a documentation issue dropping undocumented stuff.

Thank you, that makes sense.

Before I update the patch, I want to make sure I understood your suggestion correctly.

My understanding is that, if we want to proceed with removing the NETWORK
alias, this review should also include an entry in UPDATING noting that local
rc scripts using REQUIRE: NETWORK need to be migrated to REQUIRE: NETWORKING,
and the change should be marked accordingly for relnotes.

Yes, correct. Scripts in base are clean and in ports as well. This can now only affect third-party private ones.

I am still new to preparing FreeBSD patches, so I would also like to confirm
the mechanics here: if an UPDATING entry is needed, should I add it directly
to src/UPDATING? I also do not see a Relnotes field in the revision edit UI
on my side, so should I interpret this as a request to update src/RELNOTES as
well?

Correct, directly in src/UPDATING. Relnotes is a line in a commit message which will turn your commit message to the release notes afterwards. You don't need to do anything, but add the line. See commit e2f6bafc3887c7752986526f3758525d24701fce for example.

Is that what you had in mind?

Absolutely!

cat1890693_gmail.com edited the summary of this revision. (Show Details)
cat1890693_gmail.com edited the test plan for this revision. (Show Details)

Address reviewer feedback by adding an UPDATING entry and clarifying the
scope of the compatibility impact.

The patch now removes the NETWORK alias from /etc/rc.d/NETWORKING and adds
an UPDATING note to tell users that local rc scripts still using
"REQUIRE: NETWORK" should be migrated to "REQUIRE: NETWORKING".

This also needs Relnotes: yes and an entry in UPDATING. For me, a documentation issue dropping undocumented stuff.

Thank you, that makes sense.

Before I update the patch, I want to make sure I understood your suggestion correctly.

My understanding is that, if we want to proceed with removing the NETWORK
alias, this review should also include an entry in UPDATING noting that local
rc scripts using REQUIRE: NETWORK need to be migrated to REQUIRE: NETWORKING,
and the change should be marked accordingly for relnotes.

Yes, correct. Scripts in base are clean and in ports as well. This can now only affect third-party private ones.

I am still new to preparing FreeBSD patches, so I would also like to confirm
the mechanics here: if an UPDATING entry is needed, should I add it directly
to src/UPDATING? I also do not see a Relnotes field in the revision edit UI
on my side, so should I interpret this as a request to update src/RELNOTES as
well?

Correct, directly in src/UPDATING. Relnotes is a line in a commit message which will turn your commit message to the release notes afterwards. You don't need to do anything, but add the line. See commit e2f6bafc3887c7752986526f3758525d24701fce for example.

Is that what you had in mind?

Absolutely!

Thank you for the clarification and guidance.

Updated the patch to address the feedback.

This revision now includes an UPDATING entry noting that local rc scripts
still using REQUIRE: NETWORK should be migrated to REQUIRE: NETWORKING,
and it is now marked with Relnotes: yes as well.

There is an issue with the patch, Phabricator gives me:

grafik.png (346×1 px, 50 KB)

UPDATING
32

RC

33

shall

This revision now requires changes to proceed.Sun, Apr 12, 1:14 PM

There is an issue with the patch, Phabricator gives me:

grafik.png (346×1 px, 50 KB)

I may have identified what caused the intradiff parsing error.

The previous diff I uploaded only included about 20 lines of context, and then
for the update I regenerated it with essentially the whole file as context.
I wonder if that change in diff shape is what triggered the intradiff issue.

I checked the newly generated patch itself and could not find any line that
would violate the expected diff format (i.e. lines not beginning with "+",
"-", or " " in the hunk body), and I also do not see a "\ No newline at end
of file" line in the patch.

Does that seem likely to you?

LGTM, but I cannot consent commits to src formally, we need another commiter with src commit bit.

This revision is now accepted and ready to land.Sun, Apr 12, 2:16 PM

LGTM, but I cannot consent commits to src formally, we need another commiter with src commit bit.

Thank you very much for the review and for helping move this forward.

This is my first FreeBSD patch, so I am still learning the process. If there is anything else I can do from my side to help get this landed, please let me know. For example, if it would help to add another reviewer or committer with src commit bit, I would be happy to do that.

Thanks again for your help.

ziaee added a reviewer: jlduran.
ziaee added a subscriber: jlduran.

Thanks for tagging me! I think scripts in the wild surely will depend on the old syntax, and I'd like to see this go according to a deprecation plan, like we send a mail out saying we're going to do this, but overall I don't have enough to do with this part of the system to feel comfortable speaking on it. I think people will complain and their scripts will break, but its really easy for them to fix them. I think we just have to make sure to notify people. It seems @jlduran wants to see this though, so I added him in my place.

OK, if this is indeed what you prefer, I would only ask that in the commit message at least some of the discussion in the PR is stated. Something along these lines:
The NETWORKING script was created to avoid conflicts between the NETWORK script and the network script (present on NetBSD) when the file system is case-insensitive. NETWORK was maintained as an alias for compatibility with old scripts... (please feel free to paraphrase).

OK, if this is indeed what you prefer, I would only ask that in the commit message at least some of the discussion in the PR is stated. Something along these lines:
The NETWORKING script was created to avoid conflicts between the NETWORK script and the network script (present on NetBSD) when the file system is case-insensitive. NETWORK was maintained as an alias for compatibility with old scripts... (please feel free to paraphrase).

Thank you. I have updated the commit message accordingly to include that background from the PR.

@ziaee, I have cleaned up ports some time ago.

Are we good to merge this?

@ziaee, I have cleaned up ports some time ago.

By "in the wild" I meant people's internal scripts in their businesses. They certainly exist and certainly will break, although the fix is trivial.

Are we good to merge this?

No objection.

This revision was automatically updated to reflect the committed changes.