__git: mode surprisingly applies recursively #48

New Issue

ungleich-gitea · 2021-11-20T11:25:10Z

ungleich-gitea commented

2021-11-20 11:25:10 +00:00

I would expect __git stuff --source origin --mode 0755 not to apply recursively, but instead just to the upper-level directory.

Git saves parameters in the repository itself, so in most cases that's what I, as a user, would want.

I propose adding a --mode-recursive boolean parameter for people who absolutely need current behaviour and changing the default behaviour to what I described before.

Currently preparing a MR with some fixes for non-Linux OSs, so closing this issue after some discussion would be easily done there as well.

I would expect `__git stuff --source origin --mode 0755` not to apply recursively, but instead just to the upper-level directory. Git saves parameters in the repository itself, so in most cases that's what I, as a user, would want. I propose adding a `--mode-recursive` boolean parameter for people who absolutely need current behaviour and changing the default behaviour to what I described before. Currently preparing a MR with some fixes for non-Linux OSs, so closing this issue after some discussion would be easily done there as well.

evilham was assigned by ungleich-gitea

2021-11-20 11:25:10 +00:00

ungleich-gitea commented

2021-11-20 15:30:46 +00:00

I would expect the --mode to only affect the top level directory.
I wonder if there should be a --umask param instead?

I would expect the --mode to only affect the top level directory. I wonder if there should be a --umask param instead?

ungleich-gitea commented

2021-11-20 15:30:48 +00:00

@evilham Actually, @nico and @steven have the last say :)
In my opinion, this is likely a bug. Since all files have mode 644 or 755, git actually only pays attention to the executable bit, I wouldn't want all files to have executable bit set.

So I am more for fixing this in the way that mode is applied only to the upper level directory.

@evilham Actually, @nico and @steven have the last say :) In my opinion, this is likely a bug. Since all files have mode 644 or 755, git actually only pays attention to the executable bit, I wouldn't want all files to have executable bit set. So I am more for fixing this in the way that mode is applied only to the upper level directory.

ungleich-gitea commented

2021-11-20 15:30:49 +00:00

This was very likely a bug in __git that nobody had noticed. I would be tempted to skip the releng overhead and trust that someone running things as root on their systems will check the changelog, but @poljakowski has the last say on that.
Your approach sounds good from a more conservative releng POV, so I'm totally fine implementing these changes once I have @poljakowski's feedback.

This was very likely a bug in `__git` that nobody had noticed. I would be tempted to skip the releng overhead and trust that someone running things as root on their systems will check the changelog, but @poljakowski has the last say on that. Your approach sounds good from a more conservative releng POV, so I'm totally fine implementing these changes once I have @poljakowski's feedback.

ungleich-gitea commented

2021-11-20 15:30:50 +00:00

The --mode-recursive flag is problematic because the default is changed from "recursive mode-setting" to "shallow mode-setting".
This potentially breaks existing setups for users requiring the recursive behaviour.

But I'm curious if recursive mode-setting is actually used/required by anyone?
This looks like a strong indicator of a broken repository or configuration to me.

So my proposal would be to have a --mode-shallow parameter and immediately mark it as deprecated.
I would leave a note in the man page that says that the "shallow" behaviour will be the default soon and if a user actually requires recursive mode-setting they shall shout in the issue tracker or the feature will be removed.

The `--mode-recursive` flag is problematic because the default is changed from "recursive mode-setting" to "shallow mode-setting". This potentially breaks existing setups for users requiring the recursive behaviour. But I'm curious if recursive mode-setting is actually used/required by anyone? This looks like a strong indicator of a broken repository or configuration to me. So my proposal would be to have a `--mode-shallow` parameter and immediately mark it as deprecated. I would leave a note in the man page that says that the "shallow" behaviour will be the default soon and if a user actually requires recursive mode-setting they shall shout in the issue tracker or the feature will be removed.

ungleich-gitea commented

2021-11-20 15:30:51 +00:00

mentioned in merge request !894