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 resolv_conf_mv=auto option #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pemensik
Copy link
Contributor

@pemensik pemensik commented Jul 9, 2023

It requires stat -c %D to be working. Try to not require stat by checking it is executable first. Should be able to handle both bind-mounted /etc/resolv.conf and atomic move from temp file.

It should work also for issue #19

@rsmarples
Copy link
Member

That only works on Linux.

@pemensik
Copy link
Contributor Author

Is there any portable check to ensure two files are on the same filesystem, but they are not identical file? Something similar to test -ef , but without inode check? I admit I test different Linux flavours, but do not have other VM to test.

Checked openbsd stat manual. Does it need just %d format instead?

It requires stat -c %d to be working. Try to not require stat by
checking it is executable first. Should be able to handle both
bind-mounted /etc/resolv.conf and atomic move from temp file.

Not using %D format, that seems to be Linux-specific.
@pemensik pemensik force-pushed the resolv.conf-auto-mv branch from ecbd9ce to a4179ca Compare July 10, 2023 06:36
@rsmarples
Copy link
Member

There is no portable way of working this out.
stat format on BSD requires -f, not -c which Linux uses.

@pemensik
Copy link
Contributor Author

Ah. Okay, maybe configure could sort this detail at build time. It would require just trivial check on build and would not clutter those conditions more than necessary.

Linux uses stat -c to print just selected properies. Unfortunately *BSD
systems use -f for the same thing. Detect it during build and include
chosen variant.
@@ -106,6 +107,10 @@ dragonfly*|freebsd*)
if [ -z "$STATUSARG" ]; then
STATUSARG="onestatus"
fi
echo "STAT_FMT= stat -f" >>$CONFIG_MK
;;
*bsd*)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the whole stat check needs to be in it's own section.
I'm tempted to suggest that we assume BSD style by default and add an exception for Linux.

The only potential issue I think is kFreeBSD which may use the coreutils stat rather than the FreeBSD one.
Also some stat's just don't offer the functionality we need such as Plan9 or disabled as in the case for busybox.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those unusual cases should be able to handle by make STAT_FMT=. I am not sure how often is crossbuilding done for such systems. I think configure could test the output by actually checking stat -c %d . and that it does report a number. Not sure if such check would work on busybox systems. At least stat from alpine docker image and BusyBox v1.36.1 has -c %d support.

I expect Linux systems are dominant today. But if the BSD variant is likely to work on different systems, which may use openresolv, it would be okay. It should have own case "$OS" just for stat check, is that what you mean?

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