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

Fix chown typo #1676

Merged
merged 1 commit into from Jan 4, 2018
Merged

Fix chown typo #1676

merged 1 commit into from Jan 4, 2018

Conversation

OliverO2
Copy link
Contributor

@OliverO2 OliverO2 commented Jan 3, 2018

No description provided.

@gdha gdha added this to the ReaR v2.4 milestone Jan 4, 2018
@gdha gdha added enhancement Adaptions and new features minor bug An alternative or workaround exists and removed enhancement Adaptions and new features labels Jan 4, 2018
@gdha gdha self-assigned this Jan 4, 2018
@gdha gdha merged commit 4de87e8 into rear:master Jan 4, 2018
@gdha
Copy link
Member

gdha commented Jan 4, 2018

@OliverO2 Thanks for fixing this!

@jsmeix
Copy link
Member

jsmeix commented Jan 4, 2018

For me on SLE11 and SLE12
the OWNER.GROUP syntax also works

# touch /tmp/foo

# ls -l /tmp/foo
-rw-r--r-- 1 root root 0 Jan  4 10:07 /tmp/foo

# chown lp.lp /tmp/foo

# ls -l /tmp/foo
-rw-r--r-- 1 lp lp 0 Jan  4 10:07 /tmp/foo

But "man 1 chown" on SLES11 and SLES12 only talks about
the OWNER:GROUP syntax and 'colon' so that this pull request
is at least a "cleanup" of the code.

@OliverO2
I would like to know what system you use
where the OWNER.GROUP syntax may not work
so that there is a real bug here.

@OliverO2
Copy link
Contributor Author

OliverO2 commented Jan 4, 2018

@jsmeix chown on Ubuntu 16.04 also accepts the OWNER.GROUP syntax, but as far as I remember, it has never been documented. I'd regard this as sort-of deprecated.

Cf. https://www.gnu.org/software/coreutils/manual/html_node/chown-invocation.html:

Some older scripts may still use ‘.’ in place of the ‘:’ separator. POSIX 1003.1-2001 (see Standards conformance) does not require support for that, but for backward compatibility GNU chown supports ‘.’ so long as no ambiguity results. New scripts should avoid the use of ‘.’ because it is not portable, and because it has undesirable results if the entire owner‘.’group happens to identify a user whose name contains ‘.’.

Other code in ReaR (e.g. usr/share/rear/rescue/default/010_merge_skeletons.sh) consistently uses the colon notation so we can be confident that getting rid of the dot notation does not break things.

@OliverO2 OliverO2 deleted the fix/150_adjust_permissions.sh branch January 4, 2018 10:43
@jsmeix
Copy link
Member

jsmeix commented Jan 4, 2018

@OliverO2
many thanks for the info where the OWNER.GROUP syntax
is documented!

I never menat that this change to the OWNER:GROUP syntax
(which is the only right one according to my man pages)
could break anything.
I only liked to know if there is an actual bug somewhere.

For the fun of it:
There are various "interesting effects" that could happen
(depending on this and that in some particular environment)
when "unusual" characters are used in usernames
which is the reason why "man useradd" one SLES12 states

Usernames must start with a lower case letter
or an underscore, followed by lower case letters,
digits, underscores, or dashes.
They can end with a dollar sign.
In regular expression terms:
  [a-z_][a-z0-9_-]*[$]?
Usernames may only be up to 32 characters long.

In particular non-ASCII characters in usernames
have even "very interesting effects":
For my latest example you may have a look at
apple/cups#5143
or more in general you may also have a look at
"Use non-ASCII characters in usernames ..." in
https://en.opensuse.org/SDB:Plain_Text_versus_Locale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup fixed / solved / done minor bug An alternative or workaround exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants