Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add patch for OpenWrt #48

Closed
wants to merge 1 commit into from
Closed

Add patch for OpenWrt #48

wants to merge 1 commit into from

Conversation

small-5
Copy link
Contributor

@small-5 small-5 commented Aug 1, 2024

OpenWrt should use /var/run/n3n instead of /run/n3n.
The libcap is not required.

@hamishcoleman
Copy link
Contributor

Have you experienced errors with the use of "/run"? Can you link to an OpenWRT policy document that says not to use it?

I see that "/run" is a symlink to "/var/run", and thus it has no problem with staying unpatched. This would be the only OpenWRT-specific patch in the source code, so it should be avoided if at all possible. If not possible to avoid, we are the upstream, so we also should avoid carrying a patch file and just have an ifdef in the actual source code (as that is more resilient to changes)

@small-5
Copy link
Contributor Author

small-5 commented Aug 1, 2024

Yes, an error will be reported if /run is used.
OpenWrt does not have a /run directory, only /var/run (actually /tmp/run)
If don't use a patch, can only add a flag to determine whether it is OpenWrt.

@hamishcoleman
Copy link
Contributor

My openwrt systems all have a "/run" as a symlink to "/var/run", so I dont see any problem. Have openwrt changed they way they work recently? Can you link to this change?

@small-5
Copy link
Contributor Author

small-5 commented Aug 1, 2024

In my impression, OpenWrt projects all use /var/run or /tmp/run
image

@hamishcoleman
Copy link
Contributor

Equally, I can show you an openwrt system that has a symlink from /run to /var/run - which now appears to be a vendor patch.

Probably a better patch would be to plumb up the (currently unused) configure --runstatedir option to default to "/run" (as all modern linux systems use and expect) and allow the openwrt build to specify a --runstatedir=/var/run

@small-5
Copy link
Contributor Author

small-5 commented Aug 1, 2024

Your OpenWrt should used a third-party patch, so there is a /run directory.
For me, using configure --runstatedir or patch can achieve the same effect.

@hamishcoleman
Copy link
Contributor

Managing a patch file is more unreliable, harder to edit and more hidden from view. Their use is valuable when the upstream cannot fix or change things. Since this is the upstream project, we can fix the problem at the source by adding a configure feature

@small-5
Copy link
Contributor Author

small-5 commented Aug 1, 2024

Then let me modify the code and add runstatedir to test it.

@small-5 small-5 closed this by deleting the head repository Aug 1, 2024
@hamishcoleman
Copy link
Contributor

Just so you know, you can just reuse the same pull request, and push different patches into it - which has the advantage that the context, old patch history and conversation is not lost

hamishcoleman added a commit to hamishcoleman/n3n that referenced this pull request Aug 2, 2024
All modern Linux systems have the ephemeral local state directory
created at "/run" - however this is not universal even across just the
supported Linux systems.

Add a configure option to set the location of run dir that is compiled
into the binary.

(See PR n42n#48 and n42n#49)
hamishcoleman added a commit to hamishcoleman/n3n that referenced this pull request Aug 2, 2024
It turns out that the openwrt systems that I have been using to test my
OpenWRT package builds (All Turris Omnia systems) appears to have had a
vendor patch applied to create a symlink from "/run" to "/var/run".  On
a vanilla OpenWRT system, there is no "/run".  Use the new configure
option to set the correct run dir location.

(Also deal with the OpenWRT auto conf tools being too old to include the
runstatedir option)

(See PR n42n#48, n42n#49)
hamishcoleman added a commit to hamishcoleman/n3n that referenced this pull request Aug 2, 2024
It turns out that the openwrt systems that I have been using to test my
OpenWRT package builds (All Turris Omnia systems) appears to have had a
vendor patch applied to create a symlink from "/run" to "/var/run".  On
a vanilla OpenWRT system, there is no "/run".  Use the new configure
option to set the correct run dir location.

(Also deal with the OpenWRT auto conf tools being too old to include the
runstatedir option)

(See PR n42n#48, n42n#49)
@hamishcoleman hamishcoleman mentioned this pull request Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants