* [PATCH v6 0/1] mingw: give more details about unsafe directory's ownership
From: Sören Krecker @ 2024-01-06 11:29 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <xmqqbka07te6.fsf@gitster.g>
So I change the commit message and the title of this commit.
Thanks for your feedback.
Sören Krecker (1):
mingw: give more details about unsafe directory's ownership
compat/mingw.c | 64 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 51 insertions(+), 13 deletions(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.39.2
^ permalink raw reply
* [PATCH v6 1/1] mingw: give more details about unsafe directory's ownership
From: Sören Krecker @ 2024-01-06 11:29 UTC (permalink / raw)
To: git; +Cc: sunshine, Sören Krecker
In-Reply-To: <20240106112917.1870-1-soekkle@freenet.de>
Add domain/username in error message, if owner sid of repository and
user sid are not equal on windows systems.
Old error message:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'S-1-5-21-571067702-4104414259-3379520149-500'
but the current user is:
'S-1-5-21-571067702-4104414259-3379520149-1001'
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
New error message:
'''
fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
'C:/Users/test/source/repos/git' is owned by:
'DESKTOP-L78JVA6/Administrator' (S-1-5-21-571067702-4104414259-3379520149-500)
but the current user is:
'DESKTOP-L78JVA6/test' (S-1-5-21-571067702-4104414259-3379520149-1001)
To add an exception for this directory, call:
git config --global --add safe.directory C:/Users/test/source/repos/git
'''
Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
compat/mingw.c | 64 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 51 insertions(+), 13 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 42053c1f65..6240387205 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void)
return result;
}
+static BOOL user_sid_to_user_name(PSID sid, LPSTR *str)
+{
+ SID_NAME_USE pe_use;
+ DWORD len_user = 0, len_domain = 0;
+ BOOL translate_sid_to_user;
+
+ /* returns only FALSE, because the string pointers are NULL*/
+ LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain,
+ &pe_use);
+ /*Alloc needed space of the strings*/
+ ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user);
+ translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user,
+ *str, &len_domain, &pe_use);
+ if (translate_sid_to_user == FALSE)
+ FREE_AND_NULL(*str);
+ else
+ (*str)[len_domain] = '/';
+ return translate_sid_to_user;
+}
+
static int acls_supported(const char *path)
{
size_t offset = offset_1st_component(path);
@@ -2765,27 +2785,45 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
strbuf_addf(report, "'%s' is on a file system that does "
"not record ownership\n", path);
} else if (report) {
- LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL;
+ LPSTR str1, str2, str3, str4, to_free1 = NULL, to_free3 = NULL, to_local_free2=NULL, to_local_free4=NULL;
- if (ConvertSidToStringSidA(sid, &str1))
+ if (user_sid_to_user_name(sid, &str1))
to_free1 = str1;
else
str1 = "(inconvertible)";
-
- if (!current_user_sid)
- str2 = "(none)";
- else if (!IsValidSid(current_user_sid))
- str2 = "(invalid)";
- else if (ConvertSidToStringSidA(current_user_sid, &str2))
- to_free2 = str2;
+ if (ConvertSidToStringSidA(sid, &str2))
+ to_local_free2 = str2;
else
str2 = "(inconvertible)";
+
+ if (!current_user_sid) {
+ str3 = "(none)";
+ str4 = "(none)";
+ }
+ else if (!IsValidSid(current_user_sid)) {
+ str3 = "(invalid)";
+ str4 = "(invalid)";
+ } else {
+ if (user_sid_to_user_name(current_user_sid,
+ &str3))
+ to_free3 = str3;
+ else
+ str3 = "(inconvertible)";
+ if (ConvertSidToStringSidA(current_user_sid,
+ &str4))
+ to_local_free4 = str4;
+ else
+ str4 = "(inconvertible)";
+ }
strbuf_addf(report,
"'%s' is owned by:\n"
- "\t'%s'\nbut the current user is:\n"
- "\t'%s'\n", path, str1, str2);
- LocalFree(to_free1);
- LocalFree(to_free2);
+ "\t'%s' (%s)\nbut the current user is:\n"
+ "\t'%s' (%s)\n",
+ path, str1, str2, str3, str4);
+ free(to_free1);
+ LocalFree(to_local_free2);
+ free(to_free3);
+ LocalFree(to_local_free4);
}
}
--
2.39.2
^ permalink raw reply related
* Re: [GSOC][RFC] Heed core.bare from template config file when no command line override given, as a microproject.
From: Ghanshyam Thakkar @ 2024-01-06 12:07 UTC (permalink / raw)
To: Junio C Hamano, Elijah Newren; +Cc: Christian Couder, git, johannes.schindelin
In-Reply-To: <xmqqjzonpy9l.fsf@gitster.g>
On Fri Jan 5, 2024 at 9:29 PM IST, Junio C Hamano wrote:
> Elijah Newren <newren@gmail.com> writes:
>
> > If you look back at the mailing list discussion on the series that
> > introduced this TODO comment you are trying to address, you'll note
> > that both Glen and I dug into the code and attempted to explain it to
> > each other, and we both got it wrong on our first try.
>
> I think you meant 0f7443bd (init-db: document existing bug with
> core.bare in template config, 2023-05-16), where it says:
>
> The comments in create_default_files() talks about reading config from
> the config file in the specified `--templates` directory, which leads to
> the question of whether core.bare could be set in such a config file and
> thus whether the code is doing the right thing.
>
> But I suspect the all of the above comes from a misunderstanding.
> The comment the above commit log message talks about is:
>
> /*
> * First copy the templates -- we might have the default
> * config file there, in which case we would want to read
> * from it after installing.
> *
> * Before reading that config, we also need to clear out any cached
> * values (since we've just potentially changed what's available on
> * disk).
> */
>
> This primarily comes from my 4f629539 (init-db: check template and
> repository format., 2005-11-25), whose focus was to control the way
> HEAD symref is created, but care was taken to avoid propagating
> values from the configuration variables in the template that do not
> make sense for the repository being initialized. The most important
> thing being the repository format version, but the intent to avoid
> nonsense combination between the characteristic the new repository
> has and the configuration values copied from the template was not
> limited to the format version.
>
> Specifically, the commit that introduced the comment never wanted to
> honor core.bare in the template. I do not think I has core.bare in
> mind when I wrote the comment, but I would have described it as the
> same category as the repository format version, i.e. something you
> would not want to copy, if I were pressed to clarify back then.
Then I suppose this warrants updating the TODO comment in
create_default_files(), which currently can be interpreted as this
being a unwanted behavior. And also amending the testcases which
currently display this as knwon breakage.
> Besides, create_default_files() is way too late, even if we wanted
> to create a bare repository when the template config file says
> core.bare = true, as the caller would already have created before
> passing $DIR (when "git --bare init $DIR" was run) or $DIR/.git
> (when "git init $DIR" was run) to the function.
>
> If somebody wants to always create a bare repository by having
> core.bare=true in their template and if we wanted to honor it (which
> I am dubious of the value of, by the way), I would think the right
> place to do so would be way before create_default_files() is called.
> When running "git init [$DIR]", long before calling init_db(), we
> decide if we are about to create a bare repository and either create
> $DIR or $DIR/.git. What is in the template, if we really wanted to
> do so, should be read before that happens, no?
That is what I proposed in my original email, after which I had a
working solution which passed all the tests. That solution was indeed to
check for core.bare in the template before we set GIT_DIR_ENVIRONMENT,
which subsequently creates either $DIR or $DIR/.git as you described
above.
Regardless, I can send the patch with updated comments to clarify
that ignoring core.bare from template files is the intended behavior and
amend the test_expect_failure testcases, with Elijah's consensus.
Thanks.
^ permalink raw reply
* [PATCH] branch: clarify <oldbranch> term
From: Rubén Justo @ 2024-01-06 14:38 UTC (permalink / raw)
To: Git List
Since 52d59cc645 (branch: add a --copy (-c) option to go with --move
(-m), 2017-06-18) <oldbranch> is used in more operations than just -m.
Let's also clarify what we do if <oldbranch> is omitted.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
Documentation/git-branch.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 4395aa9354..233264549c 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -312,7 +312,8 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
option is omitted, the current HEAD will be used instead.
<oldbranch>::
- The name of an existing branch to rename.
+ The name of an existing branch. If this option is omitted,
+ the name of the current branch will be used instead.
<newbranch>::
The new name for an existing branch. The same restrictions as for
--
2.42.0
^ permalink raw reply related
* Bug: git log with log.showSignature enabled
From: Maxim Iorsh @ 2024-01-06 16:56 UTC (permalink / raw)
To: git
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
iorsh-linux:~/devel/fontforge/build> git log -n 1 --oneline
2aa98f6a5 (HEAD -> master) Dummy signed commit
iorsh-linux:~/devel/fontforge/build> git config log.showSignature true
iorsh-linux:~/devel/fontforge/build> git log -n 1 --oneline
2aa98f6a5 (HEAD -> master) gpg: Signature made 17:47:50 2024 ינו 06 ש' IST
gpg: using RSA key XXXXXXXXXX
gpg: issuer "iorsh@users.sourceforge.net"
gpg: Good signature from "Maxim Iorsh <iorsh@users.sourceforge.net>" [ultimate]
Dummy signed commit
What did you expect to happen? (Expected behavior)
When asked for oneliner or any specific format, `git log` shouldn't
show signature event when configured.
What happened instead? (Actual behavior)
`git log` always show signatures with log.showSignature enabled, even
for explicit formatting requests.
What's different between what you expected and what actually happened?
Anything else you want to add:
This behavior obviously breaks any script which relies on git log to
produce preformatted info about commits. It's also quite rare, becaus
e probably very few users set log.showSignature config option.
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.34.1
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.15.0-91-generic #101-Ubuntu SMP Tue Nov 14 13:30:08 UTC
2023 x86_64
compiler info: gnuc: 11.4
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/tcsh
[Enabled Hooks]
^ permalink raw reply
* Re: Bug: git log with log.showSignature enabled
From: brian m. carlson @ 2024-01-06 18:21 UTC (permalink / raw)
To: Maxim Iorsh; +Cc: git
In-Reply-To: <CADQ_TR56X_iMitPEiaOyR_h=1dp9ThUQMu_Vqjest1i8xbh9Tw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2506 bytes --]
On 2024-01-06 at 16:56:29, Maxim Iorsh wrote:
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
>
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> iorsh-linux:~/devel/fontforge/build> git log -n 1 --oneline
> 2aa98f6a5 (HEAD -> master) Dummy signed commit
> iorsh-linux:~/devel/fontforge/build> git config log.showSignature true
> iorsh-linux:~/devel/fontforge/build> git log -n 1 --oneline
> 2aa98f6a5 (HEAD -> master) gpg: Signature made 17:47:50 2024 ינו 06 ש' IST
> gpg: using RSA key XXXXXXXXXX
> gpg: issuer "iorsh@users.sourceforge.net"
> gpg: Good signature from "Maxim Iorsh <iorsh@users.sourceforge.net>" [ultimate]
> Dummy signed commit
>
> What did you expect to happen? (Expected behavior)
>
> When asked for oneliner or any specific format, `git log` shouldn't
> show signature event when configured.
>
> What happened instead? (Actual behavior)
>
> `git log` always show signatures with log.showSignature enabled, even
> for explicit formatting requests.
>
> What's different between what you expected and what actually happened?
>
> Anything else you want to add:
>
> This behavior obviously breaks any script which relies on git log to
> produce preformatted info about commits. It's also quite rare, becaus
> e probably very few users set log.showSignature config option.
I think this is behaving as designed. git log is a porcelain command,
which means it's designed for the user to use, but is not intended for
scripting. The documentation for `log.showSignature` says this:
If true, makes git-log(1), git-show(1), and git-whatchanged(1) assume --show-signature.
As long as the output is what you would have gotten if you manually
added --show-signature, the result is correct.
However, if you want to script things, you can use git rev-list, which
is a plumbing command that's specifically designed to allow scripting.
It works very similarly to git log, but doesn't have configuration
options that change its behaviour, so it doesn't suffer from this
problem.
For example, on one of my branches, I get this:
$ git rev-list -n 1 --oneline HEAD
404bf1f83a credential: add an argument to keep state
Note that the implicit HEAD of git log doesn't work with git rev-list,
so you need to specify HEAD explicitly.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Leveraging --rebase-merges --update-refs mechanism to rebase several branches in one run
From: Yann Dirson @ 2024-01-06 19:05 UTC (permalink / raw)
To: git
In-Reply-To: <763603689.1820092086.1704566524953.JavaMail.root@zimbra39-e7>
This idea comes from a repo[1] where I am experimenting with code variants:
on one side I have a branch where I add to the core mechanisms, and on the
other(s) I have several branches where I experiment with different rendering
options.
There are indeed 2 different workflows here:
- improving the variants themselves, where occasionally some fixup or new
commit for the core branch gets introduced
- adding feature to the core branch, then merging that into each variant,
where fixups already appear quite regularly
That produces a set closely-related branches with lots of merges, and applying
the fixups is a bit tricky.
The "core + 1 variant" case pretty much works out of the box, with --rebase-merges
and --update-refs generating a perfect instructions sheet.
But if I was to rebase just one variant while rewriting the core branch, obviously
all other variants would still fork off the pre-rewrite core branch, and we'd loose
all chances of automating the same work on the other variants.
OTOH, if I get `git-rebase` to generate the instruction sheets for those other
variants first, strip them (manually) from the common part, and insert them in the
instruction sheet of my "core + 1 variant" case ... I do get the whole of my branches
rebased together, and sharing the updated core.
So the question is, would there be any obstacles to let git-rebase automate this
completely? By chance it could even be a trivial change?
I guess we'd only want this feature to be enabled under certain conditions, like
--update-refs being specified so the many heads of the rebase would be reachable.
[1] https://github.com/ydirson/test-yew-tutorial/tree/opr is the "core" branch,
and branches opr-* are the variants
^ permalink raw reply
* [PATCH v3] fetch: add new config option fetch.all
From: Tamino Bauknecht @ 2024-01-06 20:17 UTC (permalink / raw)
To: git; +Cc: Tamino Bauknecht
In-Reply-To: <xmqqa5pjpxm6.fsf@gitster.g>
This commit introduces the new boolean configuration option fetch.all
which allows to fetch all available remotes by default. The config
option can be overridden by explicitly specifying a remote or by using
--no-all.
The behavior for --all is unchanged and calling git-fetch with --all and
a remote will still result in an error.
The config option was also added to the config documentation and new
tests cover the expected behavior.
Additionally, --no-all was added to the command-line documentation of
git-fetch.
Signed-off-by: Tamino Bauknecht <dev@tb6.eu>
---
Thank you for your detailed explanation. I misunderstood your suggestion
and didn't know that the concept of negated options ("no-...") already
exists for most options in git and utilizing it definitely makes sense.
I also agree with you that "default" would be ambiguous.
I reworked the patch to respect --no-all and added the flag to the
documentation.
The previous tests also had an issue that the check could be true even
if no fetch was executed for those that tested for the "default" fetch
behavior (since all the branches from origin already existed anyway).
If someone can think of a more elegant solution, I'll gladly improve it
further.
Documentation/config/fetch.txt | 6 ++
Documentation/fetch-options.txt | 5 +-
builtin/fetch.c | 20 +++-
t/t5514-fetch-multiple.sh | 161 ++++++++++++++++++++++++++++++++
4 files changed, 187 insertions(+), 5 deletions(-)
diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
index aea5b97477..d7dc461bd1 100644
--- a/Documentation/config/fetch.txt
+++ b/Documentation/config/fetch.txt
@@ -50,6 +50,12 @@ fetch.pruneTags::
refs. See also `remote.<name>.pruneTags` and the PRUNING
section of linkgit:git-fetch[1].
+fetch.all::
+ If true, fetch will attempt to update all available remotes.
+ This behavior can be overridden by passing `--no-all` or by
+ explicitly specifying one or more remote(s) to fetch from.
+ Defaults to false.
+
fetch.output::
Control how ref update status is printed. Valid values are
`full` and `compact`. Default value is `full`. See the
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index a1d6633a4f..5e70f6d2e4 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -1,5 +1,6 @@
---all::
- Fetch all remotes.
+--[no-]all::
+ Fetch all remotes. This overrides the configuration option
+ `fetch.all`.
-a::
--append::
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a284b970ef..5a0b249c07 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -102,6 +102,7 @@ static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
+ int all;
int prune;
int prune_tags;
int show_forced_updates;
@@ -115,6 +116,11 @@ static int git_fetch_config(const char *k, const char *v,
{
struct fetch_config *fetch_config = cb;
+ if (!strcmp(k, "fetch.all")) {
+ fetch_config->all = git_config_bool(k, v);
+ return 0;
+ }
+
if (!strcmp(k, "fetch.prune")) {
fetch_config->prune = git_config_bool(k, v);
return 0;
@@ -2121,6 +2127,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
{
struct fetch_config config = {
.display_format = DISPLAY_FORMAT_FULL,
+ .all = -1,
.prune = -1,
.prune_tags = -1,
.show_forced_updates = 1,
@@ -2132,7 +2139,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
const char *bundle_uri;
struct string_list list = STRING_LIST_INIT_DUP;
struct remote *remote = NULL;
- int all = 0, multiple = 0;
+ int all = -1, multiple = 0;
int result = 0;
int prune_tags_ok = 1;
int enable_auto_gc = 1;
@@ -2148,7 +2155,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
struct option builtin_fetch_options[] = {
OPT__VERBOSITY(&verbosity),
- OPT_BOOL(0, "all", &all,
+ OPT_COUNTUP(0, "all", &all,
N_("fetch from all remotes")),
OPT_BOOL(0, "set-upstream", &set_upstream,
N_("set upstream for git pull/fetch")),
@@ -2337,11 +2344,18 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
fetch_bundle_uri(the_repository, bundle_uri, NULL))
warning(_("failed to fetch bundles from '%s'"), bundle_uri);
- if (all) {
+ if (all > 0) {
if (argc == 1)
die(_("fetch --all does not take a repository argument"));
else if (argc > 1)
die(_("fetch --all does not make sense with refspecs"));
+ }
+
+ if (all > 0 || (config.all > 0 && !argc && all < 0)) {
+ /*
+ * Only use fetch.all config option if no remotes were
+ * explicitly given and if no --no-all was passed
+ */
(void) for_each_remote(get_one_remote_for_fetch, &list);
/* do not do fetch_multiple() of one */
diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
index a95841dc36..63cd730718 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -24,6 +24,15 @@ setup_repository () {
)
}
+setup_test_clone () {
+ test_dir="$1" &&
+ git clone one "$test_dir" &&
+ for r in one two three
+ do
+ git -C "$test_dir" remote add "$r" "../$r" || return 1
+ done
+}
+
test_expect_success setup '
setup_repository one &&
setup_repository two &&
@@ -209,4 +218,156 @@ test_expect_success 'git fetch --multiple --jobs=0 picks a default' '
git fetch --multiple --jobs=0)
'
+create_fetch_all_expect () {
+ cat >expect <<-\EOF || return 1
+ one/main
+ one/side
+ origin/HEAD -> origin/main
+ origin/main
+ origin/side
+ three/another
+ three/main
+ three/side
+ two/another
+ two/main
+ two/side
+ EOF
+}
+
+for fetch_all in true false
+do
+ test_expect_success "git fetch --all (works with fetch.all = $fetch_all)" '
+ test_dir="test_fetch_all_$fetch_all" &&
+ setup_test_clone "$test_dir" &&
+ (
+ cd "$test_dir" &&
+ git config fetch.all $fetch_all &&
+ git fetch --all &&
+ create_fetch_all_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+ '
+done
+
+test_expect_success 'git fetch (fetch all remotes with fetch.all = true)' '
+ setup_test_clone test9 &&
+ (
+ cd test9 &&
+ git config fetch.all true &&
+ git fetch &&
+ git branch -r >actual &&
+ create_fetch_all_expect &&
+ test_cmp expect actual
+ )
+'
+
+create_fetch_one_expect () {
+ cat >expect <<-\EOF || return 1
+ one/main
+ one/side
+ origin/HEAD -> origin/main
+ origin/main
+ origin/side
+ EOF
+}
+
+test_expect_success 'git fetch one (explicit remote overrides fetch.all)' '
+ setup_test_clone test10 &&
+ (
+ cd test10 &&
+ git config fetch.all true &&
+ git fetch one &&
+ create_fetch_one_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+'
+
+create_fetch_two_as_origin_expect () {
+ cat >expect <<-\EOF || return 1
+ origin/HEAD -> origin/main
+ origin/another
+ origin/main
+ origin/side
+ EOF
+}
+
+test_expect_success 'git config fetch.all false (fetch only default remote)' '
+ setup_test_clone test11 &&
+ (
+ cd test11 &&
+ git config fetch.all false &&
+ git remote set-url origin ../two &&
+ git fetch &&
+ create_fetch_two_as_origin_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+'
+
+for fetch_all in true false
+do
+ test_expect_success "git fetch --no-all (fetch only default remote with fetch.all = $fetch_all)" '
+ test_dir="test_no_all_fetch_all_$fetch_all" &&
+ setup_test_clone "$test_dir" &&
+ (
+ cd "$test_dir" &&
+ git config fetch.all $fetch_all &&
+ git remote set-url origin ../two &&
+ git fetch --no-all &&
+ create_fetch_two_as_origin_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+ '
+done
+
+test_expect_success 'git fetch --no-all (fetch only default remote without fetch.all)' '
+ setup_test_clone test12 &&
+ (
+ cd test12 &&
+ git config --unset-all fetch.all || true &&
+ git remote set-url origin ../two &&
+ git fetch --no-all &&
+ create_fetch_two_as_origin_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'git fetch --all --no-all (fetch only default remote)' '
+ setup_test_clone test13 &&
+ (
+ cd test13 &&
+ git remote set-url origin ../two &&
+ git fetch --all --no-all &&
+ create_fetch_two_as_origin_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'git fetch --no-all one (fetch only explicit remote)' '
+ setup_test_clone test14 &&
+ (
+ cd test14 &&
+ git fetch --no-all one &&
+ create_fetch_one_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'git fetch --no-all --all (fetch all remotes)' '
+ setup_test_clone test15 &&
+ (
+ cd test15 &&
+ git fetch --no-all --all &&
+ create_fetch_all_expect &&
+ git branch -r >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3] fetch: add new config option fetch.all
From: Eric Sunshine @ 2024-01-06 23:32 UTC (permalink / raw)
To: Tamino Bauknecht; +Cc: git
In-Reply-To: <20240106202352.7253-2-dev@tb6.eu>
On Sat, Jan 6, 2024 at 3:25 PM Tamino Bauknecht <dev@tb6.eu> wrote:
> This commit introduces the new boolean configuration option fetch.all
> which allows to fetch all available remotes by default. The config
> option can be overridden by explicitly specifying a remote or by using
> --no-all.
> The behavior for --all is unchanged and calling git-fetch with --all and
> a remote will still result in an error.
>
> The config option was also added to the config documentation and new
> tests cover the expected behavior.
> Additionally, --no-all was added to the command-line documentation of
> git-fetch.
>
> Signed-off-by: Tamino Bauknecht <dev@tb6.eu>
> ---
> diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
> @@ -209,4 +218,156 @@ test_expect_success 'git fetch --multiple --jobs=0 picks a default' '
> +create_fetch_all_expect () {
> + cat >expect <<-\EOF || return 1
> + ...
> + EOF
> +}
This is really minor, but the `|| return 1` is superfluous. The `cat`
command itself will exit with success or failure, and since it's the
last command in the function, its return value will be the value
returned by the function. Thus, there is no need to use `|| return 1`
to signal failure when the `cat` command itself will do so anyhow.
For such a minor issue, I would typically say "not worth a reroll",
however, this sort of unnecessary code may confuse future readers into
thinking that something unusual and non-obvious is going on. As such,
it might be worth a reroll after all, but if you do choose to reroll,
wait until others have chimed in since they might have more to add
(either about this or other parts of the patch).
^ permalink raw reply
* Re: [PATCH v3] fetch: add new config option fetch.all
From: Eric Sunshine @ 2024-01-06 23:37 UTC (permalink / raw)
To: Tamino Bauknecht; +Cc: git
In-Reply-To: <CAPig+cQ3y7rGbrNjMZB_HpMAWYOhDg8qKSoR5EF=T5+jc6rgvA@mail.gmail.com>
On Sat, Jan 6, 2024 at 6:32 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Jan 6, 2024 at 3:25 PM Tamino Bauknecht <dev@tb6.eu> wrote:
> > +create_fetch_all_expect () {
> > + cat >expect <<-\EOF || return 1
> > + ...
> > + EOF
> > +}
>
> This is really minor, but the `|| return 1` is superfluous. The `cat`
> command itself will exit with success or failure, and since it's the
> last command in the function, its return value will be the value
> returned by the function. Thus, there is no need to use `|| return 1`
> to signal failure when the `cat` command itself will do so anyhow.
Just for completeness, the other `|| return 1` in your patch...
> > + for r in one two three
> > + do
> > + git -C "$test_dir" remote add "$r" "../$r" || return 1
> > + done
... is necessary, thus correct, since shell for-loops don't terminate
automatically when a command in the loop body fails; the loop
continues regardless. Thus, this `|| return 1` ensures that we
correctly abort (signalling a failure) as soon as the error is
discovered.
^ permalink raw reply
* git config --global needs to be run in a git directory
From: Trix Knotts @ 2024-01-07 5:56 UTC (permalink / raw)
To: Git
Hi! This is a minor issue, but I noticed when I ran `git config --global` from my home directory it gave me this error: `fatal: not in a git directory`. I was able to resolve this simply by moving to a git directory on my machine and rerunning it, so it is not really a big issue. However, I think for convenience it would be nice to be able to run global things like this anywhere. What do y'all think? Would this be easy to change?
BTW this is my first time emailing the git mailing list, so sorry if I did something wrong. Please let me know if so!
--
Trix Knotts (she/they)
^ permalink raw reply
* Re: git config --global needs to be run in a git directory
From: Trix Knotts @ 2024-01-07 7:14 UTC (permalink / raw)
To: Git
In-Reply-To: <NnXUPNo--3-9@tuta.io>
My bad, I had `--global` after the item to be configured.
--
Trix Knotts (she/they)
Jan 6, 2024, 11:56 PM by enderk@tuta.io:
> Hi! This is a minor issue, but I noticed when I ran `git config --global` from my home directory it gave me this error: `fatal: not in a git directory`. I was able to resolve this simply by moving to a git directory on my machine and rerunning it, so it is not really a big issue. However, I think for convenience it would be nice to be able to run global things like this anywhere. What do y'all think? Would this be easy to change?
>
> BTW this is my first time emailing the git mailing list, so sorry if I did something wrong. Please let me know if so!
>
> --
> Trix Knotts (she/they)
>
^ permalink raw reply
* Re: Leveraging --rebase-merges --update-refs mechanism to rebase several branches in one run
From: Johannes Sixt @ 2024-01-07 8:57 UTC (permalink / raw)
To: Yann Dirson; +Cc: git
In-Reply-To: <133456672.1820149400.1704567932308.JavaMail.root@zimbra39-e7>
Am 06.01.24 um 20:05 schrieb Yann Dirson:
> The "core + 1 variant" case pretty much works out of the box, with --rebase-merges
> and --update-refs generating a perfect instructions sheet.
>
> But if I was to rebase just one variant while rewriting the core branch, obviously
> all other variants would still fork off the pre-rewrite core branch, and we'd loose
> all chances of automating the same work on the other variants.
>
> OTOH, if I get `git-rebase` to generate the instruction sheets for those other
> variants first, strip them (manually) from the common part, and insert them in the
> instruction sheet of my "core + 1 variant" case ... I do get the whole of my branches
> rebased together, and sharing the updated core.
Not a complete automation, but... You can merge all variant branches
into a temporary branch (or detached HEAD), even if that are merely -s
ours merges, and then rebase the temporary branch with --rebase-merges
--update-refs. This will generate the instruction sheet that you want.
You can remove the final merge instructions (the temporary ones) from
the instruction sheet if you do not want them to be executed.
-- Hannes
^ permalink raw reply
* Re: Leveraging --rebase-merges --update-refs mechanism to rebase several branches in one run
From: Yann Dirson @ 2024-01-07 11:37 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <ed9f9fc5-c398-4424-9b5b-dbe618cca2ed@kdbg.org>
Johannes Sixt wrote:
> Am 06.01.24 um 20:05 schrieb Yann Dirson:
> > The "core + 1 variant" case pretty much works out of the box, with
> > --rebase-merges
> > and --update-refs generating a perfect instructions sheet.
> >
> > But if I was to rebase just one variant while rewriting the core
> > branch, obviously
> > all other variants would still fork off the pre-rewrite core
> > branch, and we'd loose
> > all chances of automating the same work on the other variants.
> >
> > OTOH, if I get `git-rebase` to generate the instruction sheets for
> > those other
> > variants first, strip them (manually) from the common part, and
> > insert them in the
> > instruction sheet of my "core + 1 variant" case ... I do get the
> > whole of my branches
> > rebased together, and sharing the updated core.
>
> Not a complete automation, but... You can merge all variant branches
> into a temporary branch (or detached HEAD), even if that are merely
> -s
> ours merges, and then rebase the temporary branch with
> --rebase-merges
> --update-refs. This will generate the instruction sheet that you
> want.
> You can remove the final merge instructions (the temporary ones) from
> the instruction sheet if you do not want them to be executed.
Nice idea, and this is indeed automatable for the most part, Q&D PoC below.
There are a few things I can see missing in this PoC:
- removal of the final merge from instruction sheet
Could be done by wrapping $EDITOR - I'm not particularly fond doing things
behind the user's back, but I lack better ideas.
- restoration of HEAD
In the general case it cannot be done from the script, so we would naturally
want to do that from the instruction sheet?
While I was at manually removing the final merge, I experimented with changing
the "reset onto" to "reset <a branch name>", but that resulted in moving HEAD
to the pre-rebase version of the requested branch.
- When aborting the rebase HEAD still points to the extra merge
This is indeed a special case of the above, where instruction sheet cannot
be used, and where the script could help since we won't be in the middle of
a rebase when git-rebase stops.
There does not seem to be any documented exit-code protocol to tell the
git-rebase caller the user aborted. I guess "HEAD pointing to this commit"
could be used to identify the abort.
---- 8< ---- git-rebase-batch
#!/bin/bash
set -e
die() {
echo >&2 "ERROR: $0: $*"
exit 1
}
REBASE_OPTS=(--interactive --rebase-merges --update-refs)
# all args before "--" are passed to git-rebase
while [ $# -ge 1 ]; do
case "$1" in
--) shift; break;;
*) REBASE_OPTS+=("$1"); shift;;
esac
done
[ $# -ge 3 ] || die "need cutting-point and at least 2 refs to rebase"
CUT="$1"
shift
git checkout --detach "$CUT"
git merge -s ours "$@" -m "temporary handle for all rebased branches"
git rebase "${REBASE_OPTS[@]}" "$CUT" HEAD
# here we can be in the middle of interactive rebase, cannot perform
# any kind of cleanup (which would include restoring HEAD ref to its
# original destination)
---- 8< ----
^ permalink raw reply
* Re: [Outreachy][PATCH v4] Port helper/test-ctype.c to unit-tests/t-ctype.c
From: René Scharfe @ 2024-01-07 12:45 UTC (permalink / raw)
To: Achu Luma, git
Cc: gitster, chriscool, christian.couder, phillip.wood, steadmon, me,
Phillip Wood
In-Reply-To: <20240105161413.10422-1-ach.lumap@gmail.com>
Am 05.01.24 um 17:14 schrieb Achu Luma:
> In the recent codebase update (8bf6fbd00d (Merge branch
> 'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
> merged, providing a standardized approach for testing C code. Prior to
> this update, some unit tests relied on the test helper mechanism,
> lacking a dedicated unit testing framework. It's more natural to perform
> these unit tests using the new unit test framework.
>
> This commit migrates the unit tests for C character classification
> functions (isdigit(), isspace(), etc) from the legacy approach
> using the test-tool command `test-tool ctype` in t/helper/test-ctype.c
> to the new unit testing framework (t/unit-tests/test-lib.h).
>
> The migration involves refactoring the tests to utilize the testing
> macros provided by the framework (TEST() and check_*()).
>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Helped-by: René Scharfe <l.s.r@web.de>
> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
> Helped-by: Taylor Blau <me@ttaylorr.com>
> Signed-off-by: Achu Luma <ach.lumap@gmail.com>
> ---
[snip]
> diff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c
> deleted file mode 100644
> index e5659df40b..0000000000
> --- a/t/helper/test-ctype.c
> +++ /dev/null
> @@ -1,70 +0,0 @@
> -#include "test-tool.h"
> -
> -static int rc;
> -
> -static void report_error(const char *class, int ch)
> -{
> - printf("%s classifies char %d (0x%02x) wrongly\n", class, ch, ch);
> - rc = 1;
> -}
> -
> -static int is_in(const char *s, int ch)
> -{
> - /*
> - * We can't find NUL using strchr. Accept it as the first
> - * character in the spec -- there are no empty classes.
> - */
> - if (ch == '\0')
> - return ch == *s;
> - if (*s == '\0')
> - s++;
> - return !!strchr(s, ch);
> -}
> -
> -#define TEST_CLASS(t,s) { \
> - int i; \
> - for (i = 0; i < 256; i++) { \
> - if (is_in(s, i) != t(i)) \
> - report_error(#t, i); \
> - } \
> - if (t(EOF)) \
> - report_error(#t, EOF); \
> -}
> -
> -#define DIGIT "0123456789"
> -#define LOWER "abcdefghijklmnopqrstuvwxyz"
> -#define UPPER "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
> -#define PUNCT "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
> -#define ASCII \
> - "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
> - "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
> - "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f" \
> - "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f" \
> - "\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f" \
> - "\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f" \
> - "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f" \
> - "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
> -#define CNTRL \
> - "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
> - "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
> - "\x7f"
> -
> -int cmd__ctype(int argc UNUSED, const char **argv UNUSED)
> -{
> - TEST_CLASS(isdigit, DIGIT);
> - TEST_CLASS(isspace, " \n\r\t");
> - TEST_CLASS(isalpha, LOWER UPPER);
> - TEST_CLASS(isalnum, LOWER UPPER DIGIT);
> - TEST_CLASS(is_glob_special, "*?[\\");
> - TEST_CLASS(is_regex_special, "$()*+.?[\\^{|");
> - TEST_CLASS(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~");
> - TEST_CLASS(isascii, ASCII);
> - TEST_CLASS(islower, LOWER);
> - TEST_CLASS(isupper, UPPER);
> - TEST_CLASS(iscntrl, CNTRL);
> - TEST_CLASS(ispunct, PUNCT);
> - TEST_CLASS(isxdigit, DIGIT "abcdefABCDEF");
> - TEST_CLASS(isprint, LOWER UPPER DIGIT PUNCT " ");
> -
> - return rc;
> -}
[snip]
> diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
> new file mode 100644
> index 0000000000..3a338df541
> --- /dev/null
> +++ b/t/unit-tests/t-ctype.c
> @@ -0,0 +1,78 @@
> +#include "test-lib.h"
> +
> +static int is_in(const char *s, int ch)
> +{
> + /*
> + * We can't find NUL using strchr. Accept it as the first
> + * character in the spec -- there are no empty classes.
> + */
> + if (ch == '\0')
> + return ch == *s;
> + if (*s == '\0')
> + s++;
> + return !!strchr(s, ch);
> +}
> +
> +/* Macro to test a character type */
> +#define TEST_CTYPE_FUNC(func, string) \
> +static void test_ctype_##func(void) { \
> + for (int i = 0; i < 256; i++) { \
> + if (!check_int(func(i), ==, is_in(string, i))) \
> + test_msg(" i: 0x%02x", i); \
> + } \
> +}
> +
> +#define TEST_CHAR_CLASS(class) TEST(test_ctype_##class(), #class " works")
> +
> +#define DIGIT "0123456789"
> +#define LOWER "abcdefghijklmnopqrstuvwxyz"
> +#define UPPER "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
> +#define PUNCT "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
> +#define ASCII \
> + "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
> + "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
> + "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f" \
> + "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3a\x3b\x3c\x3d\x3e\x3f" \
> + "\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x4a\x4b\x4c\x4d\x4e\x4f" \
> + "\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f" \
> + "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f" \
> + "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
> +#define CNTRL \
> + "\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f" \
> + "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
> + "\x7f"
> +
> +TEST_CTYPE_FUNC(isdigit, DIGIT)
> +TEST_CTYPE_FUNC(isspace, " \n\r\t")
> +TEST_CTYPE_FUNC(isalpha, LOWER UPPER)
> +TEST_CTYPE_FUNC(isalnum, LOWER UPPER DIGIT)
> +TEST_CTYPE_FUNC(is_glob_special, "*?[\\")
> +TEST_CTYPE_FUNC(is_regex_special, "$()*+.?[\\^{|")
> +TEST_CTYPE_FUNC(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~")
> +TEST_CTYPE_FUNC(isascii, ASCII)
> +TEST_CTYPE_FUNC(islower, LOWER)
> +TEST_CTYPE_FUNC(isupper, UPPER)
> +TEST_CTYPE_FUNC(iscntrl, CNTRL)
> +TEST_CTYPE_FUNC(ispunct, PUNCT)
> +TEST_CTYPE_FUNC(isxdigit, DIGIT "abcdefABCDEF")
> +TEST_CTYPE_FUNC(isprint, LOWER UPPER DIGIT PUNCT " ")
> +
> +int cmd_main(int argc, const char **argv) {
> + /* Run all character type tests */
> + TEST_CHAR_CLASS(isspace);
> + TEST_CHAR_CLASS(isdigit);
> + TEST_CHAR_CLASS(isalpha);
> + TEST_CHAR_CLASS(isalnum);
> + TEST_CHAR_CLASS(is_glob_special);
> + TEST_CHAR_CLASS(is_regex_special);
> + TEST_CHAR_CLASS(is_pathspec_magic);
> + TEST_CHAR_CLASS(isascii);
> + TEST_CHAR_CLASS(islower);
> + TEST_CHAR_CLASS(isupper);
> + TEST_CHAR_CLASS(iscntrl);
> + TEST_CHAR_CLASS(ispunct);
> + TEST_CHAR_CLASS(isxdigit);
> + TEST_CHAR_CLASS(isprint);
> +
> + return test_done();
> +}
> --
> 2.42.0.windows.2
>
Quite an improvement over v3! Now you only need to repeat the class
names once.
Can we do any better? We could simply have one test per character per
class like this:
#define TEST_CHAR_CLASS(class, expect) \
for (int i = 0; i < 256; i++) \
TEST(check_int(class(i), ==, is_in(expect, i)), \
"%s(0x%02x) works", #class, i)
Which would be used like this:
TEST_CHAR_CLASS(isspace, " \n\r\t");
With that there is no need to define any functions anymore. We also
don't need any custom output, as the test name includes the character
code. Downside: We'd have thousands of tests. But is that actually
a downside or is that how the unit test framework is supposed to be
used?
If we need to aggregate the results by class for some reason, we
could use strings, like we already do for defining the expected class
members. We need special handling for NUL, as that character
terminates C strings, but we can put all other characters into a
string and then use check_str:
#define TEST_CHAR_CLASS(class, expect) \
do { \
int expect_nul = expect[0] == '\0'; \
char expect_rest[256] = {0}; \
char actual_rest[256] = {0}; \
for (int i = 1, j = 0; i < 256; i++) \
if (strchr(&expect[expect_nul], i)) \
expect_rest[j++] = i; \
for (int i = 1, j = 0; i < 256; i++) \
if (class(i) == 1) \
actual_rest[j++] = i; \
TEST(check_int(class(0), ==, expect_nul) && \
check_str(actual_rest, expect_rest), \
#class " works"); \
} while (0)
check_str escapes non-printable characters when reporting a mismatch,
so this shouldn't mess up your terminal.
By the way: Like the original code these checks are stricter than
required by the C standard in requiring the result to be 1 instead of
just true (any non-zero value). Perhaps they should be relaxed. But
that's a tangent and independent of the convergence to a unit test.
René
^ permalink raw reply
* Storing private config files in .git directory?
From: Stefan Haller @ 2024-01-07 13:03 UTC (permalink / raw)
To: git
Our git client (lazygit) has a need to store per-repo config files that
override the global one, much like git itself. The easiest way to do
that is to store those in a .git/lazygit.cfg file, and I'm wondering if
there's any reason why this is a bad idea?
Another alternative would be to store the config values in .git/config
(that's the path taken by git gui, for example), but since our config
file format is yaml, this would require translation. It would be trivial
for scalar values such as int or string, but I'm not sure how well this
would work for more complex settings like lists of objects.
Any thoughts?
^ permalink raw reply
* Analyzing a corrupted index file
From: Nathan Manceaux-Panot @ 2024-01-07 15:22 UTC (permalink / raw)
To: git
Hello all,
I have a corrupted git index file, and am trying to read it by hand, to understand what's wrong with it. Are there any tools that'll let me parse the on-disk, binary version of the index file, to unpack it into a human-readable data structure?
I found the spec for the index binary format, but reading it in that way is beyond my abilities.
For context, I'm writing a git tool, and my code is causing the index file to be screwed up in some unknown way. When it's in that state, git (v2.43) becomes inconsistent with itself:
- In the workdir, the file contains A
- According to `git show :file`, the file in the index contains B
- But `git diff` reports no changes!
The inconsistency goes away with a simple `git reset`, but can then be easily reproduced by re-executing my tool.
Thank you,
Nathan
^ permalink raw reply
* Interactive rebase doc (Was: Leveraging --rebase-merges --update-refs mechanism to rebase several branches in one run)
From: Yann Dirson @ 2024-01-07 15:31 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt
In-Reply-To: <1930018756.1822864601.1704627466836.JavaMail.root@zimbra39-e7>
> Johannes Sixt wrote:
> > Am 06.01.24 um 20:05 schrieb Yann Dirson:
> > > The "core + 1 variant" case pretty much works out of the box,
> > > with
> > > --rebase-merges
> > > and --update-refs generating a perfect instructions sheet.
> > >
> > > But if I was to rebase just one variant while rewriting the core
> > > branch, obviously
> > > all other variants would still fork off the pre-rewrite core
> > > branch, and we'd loose
> > > all chances of automating the same work on the other variants.
> > >
> > > OTOH, if I get `git-rebase` to generate the instruction sheets
> > > for
> > > those other
> > > variants first, strip them (manually) from the common part, and
> > > insert them in the
> > > instruction sheet of my "core + 1 variant" case ... I do get the
> > > whole of my branches
> > > rebased together, and sharing the updated core.
> >
> > Not a complete automation, but... You can merge all variant
> > branches
> > into a temporary branch (or detached HEAD), even if that are merely
> > -s
> > ours merges, and then rebase the temporary branch with
> > --rebase-merges
> > --update-refs. This will generate the instruction sheet that you
> > want.
> > You can remove the final merge instructions (the temporary ones)
> > from
> > the instruction sheet if you do not want them to be executed.
>
> Nice idea, and this is indeed automatable for the most part, Q&D PoC
> below.
>
> There are a few things I can see missing in this PoC:
>
> - removal of the final merge from instruction sheet
>
> Could be done by wrapping $EDITOR - I'm not particularly fond doing
> things
> behind the user's back, but I lack better ideas.
>
> - restoration of HEAD
>
> In the general case it cannot be done from the script, so we would
> naturally
> want to do that from the instruction sheet?
>
> While I was at manually removing the final merge, I experimented
> with changing
> the "reset onto" to "reset <a branch name>", but that resulted in
> moving HEAD
> to the pre-rebase version of the requested branch.
Related to this, I turned to the rebase manpage to get reference
information about update-ref, but I could not find anything about it:
only --update-refs is described, but this description also only seems to
address the non-interactive behavior.
In fact:
- there does not appear to be a reference to the interactive
instruction sheet in the rebase doc, only in the default template
- --interactive only directs the user to "Splitting commits", not to
"interactive mode"
- the "interactive mode" section really looks more like a didactic intro
to interactive rebase than like a reference doc
Would it seem OK to change things as follows?
- move current "interactive mode", "splitting commits", and "rebasing merges"
contents into a new gitrebase(7) guide
- leave in git-rebase(1) only an "interactive mode" with the reference doc
for the instruction sheet, and a pointer to the guide for detailed walkthrough
- selectively move back a few things like --strategy paragraph from
"rebasing merges"
Best regards,
--
Yann
^ permalink raw reply
* Limited operations in unsafe repositories
From: brian m. carlson @ 2024-01-07 19:40 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]
Right now, any time we try to set up a repository in that's owned by
another user, we die. While good for security, this is inconvenient in
a bunch of ways.
For example, when Git LFS wants to push data locally, it needs to know
where the `.git` directory is because it pushes the objects into
`.git/lfs`. Thus, we want to do `git rev-parse --absolute-git-dir` to
find the remote Git directory, but we can't do that if the repository is
owned by a different user.
That issue also affects the Git LFS SSH transfer server (Scutiger),
which also needs to read the configuration (to set the umask
appropriately for `core.sharedrepository`).
I had looked at sending a patch to make `git rev-parse` operate in a
special mode where it's impossible to invoke any binaries at all, but
unfortunately, `get_superproject_working_tree` invokes binaries, so
that's not possible. (If anyone is interested in picking this up, there
is a start on it, failing many tests, in the `rev-parse-safe-directory`
on my GitHub remote.)
I guess I'm looking for us to provide some basic functionality that is
guaranteed to work in this case, including `git rev-parse` and `git
config -l`. I don't think it's useful for every program that wants to
work with Git to need to implement its own repository discovery and
config parsing, and those are essential needs for tooling that needs to
work with untrusted repositories.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/1] mingw: give more details about unsafe directory's ownership
From: Johannes Sixt @ 2024-01-07 20:02 UTC (permalink / raw)
To: Sören Krecker; +Cc: sunshine, git
In-Reply-To: <20240106112917.1870-2-soekkle@freenet.de>
Am 06.01.24 um 12:29 schrieb Sören Krecker:
> Add domain/username in error message, if owner sid of repository and
> user sid are not equal on windows systems.
>
> Old error message:
> '''
> fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
> 'C:/Users/test/source/repos/git' is owned by:
> 'S-1-5-21-571067702-4104414259-3379520149-500'
> but the current user is:
> 'S-1-5-21-571067702-4104414259-3379520149-1001'
> To add an exception for this directory, call:
>
> git config --global --add safe.directory C:/Users/test/source/repos/git
> '''
>
> New error message:
> '''
> fatal: detected dubious ownership in repository at 'C:/Users/test/source/repos/git'
> 'C:/Users/test/source/repos/git' is owned by:
> 'DESKTOP-L78JVA6/Administrator' (S-1-5-21-571067702-4104414259-3379520149-500)
> but the current user is:
> 'DESKTOP-L78JVA6/test' (S-1-5-21-571067702-4104414259-3379520149-1001)
> To add an exception for this directory, call:
>
> git config --global --add safe.directory C:/Users/test/source/repos/git
> '''
I am not a fan of putting everything and the kitchen sink inside quotes.
In particular, the single-quotes around the user names are unnecessary,
IMO. Would you mind dropping them? (I do not mean to remove the quotes
around the path names because you do not touch this part of the code.)
>
> Signed-off-by: Sören Krecker <soekkle@freenet.de>
> ---
> compat/mingw.c | 64 ++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 51 insertions(+), 13 deletions(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 42053c1f65..6240387205 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2684,6 +2684,26 @@ static PSID get_current_user_sid(void)
> return result;
> }
>
> +static BOOL user_sid_to_user_name(PSID sid, LPSTR *str)
> +{
> + SID_NAME_USE pe_use;
> + DWORD len_user = 0, len_domain = 0;
> + BOOL translate_sid_to_user;
> +
> + /* returns only FALSE, because the string pointers are NULL*/
> + LookupAccountSidA(NULL, sid, NULL, &len_user, NULL, &len_domain,
> + &pe_use);
> + /*Alloc needed space of the strings*/
This comment line doesn't follow our style. Please either fix that (add
blanks after /* and before */ or (my preference) remove it altogether;
the code is clear without it.
> + ALLOC_ARRAY((*str), (size_t)len_domain + (size_t)len_user);
> + translate_sid_to_user = LookupAccountSidA(NULL, sid, (*str) + len_domain, &len_user,
> + *str, &len_domain, &pe_use);
> + if (translate_sid_to_user == FALSE)
We prefer to write this condition as
if (!translate_sid_to_user)
> + FREE_AND_NULL(*str);
> + else
> + (*str)[len_domain] = '/';
> + return translate_sid_to_user;
> +}
> +
> static int acls_supported(const char *path)
> {
> size_t offset = offset_1st_component(path);
> @@ -2765,27 +2785,45 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report)
> strbuf_addf(report, "'%s' is on a file system that does "
> "not record ownership\n", path);
> } else if (report) {
> - LPSTR str1, str2, to_free1 = NULL, to_free2 = NULL;
> + LPSTR str1, str2, str3, str4, to_free1 = NULL, to_free3 = NULL, to_local_free2=NULL, to_local_free4=NULL;
This line grew a bit long now. Maybe break it to have lines not exceed
80 characters? While you do so, please insert blanks around = signs.
>
> - if (ConvertSidToStringSidA(sid, &str1))
> + if (user_sid_to_user_name(sid, &str1))
> to_free1 = str1;
> else
> str1 = "(inconvertible)";
> -
> - if (!current_user_sid)
> - str2 = "(none)";
> - else if (!IsValidSid(current_user_sid))
> - str2 = "(invalid)";
> - else if (ConvertSidToStringSidA(current_user_sid, &str2))
> - to_free2 = str2;
> + if (ConvertSidToStringSidA(sid, &str2))
> + to_local_free2 = str2;
> else
> str2 = "(inconvertible)";
> +
> + if (!current_user_sid) {
> + str3 = "(none)";
> + str4 = "(none)";
> + }
> + else if (!IsValidSid(current_user_sid)) {
> + str3 = "(invalid)";
> + str4 = "(invalid)";
> + } else {
> + if (user_sid_to_user_name(current_user_sid,
> + &str3))
> + to_free3 = str3;
> + else
> + str3 = "(inconvertible)";
> + if (ConvertSidToStringSidA(current_user_sid,
> + &str4))
> + to_local_free4 = str4;
> + else
> + str4 = "(inconvertible)";
> + }
> strbuf_addf(report,
> "'%s' is owned by:\n"
> - "\t'%s'\nbut the current user is:\n"
> - "\t'%s'\n", path, str1, str2);
> - LocalFree(to_free1);
> - LocalFree(to_free2);
> + "\t'%s' (%s)\nbut the current user is:\n"
> + "\t'%s' (%s)\n",
> + path, str1, str2, str3, str4);
> + free(to_free1);
> + LocalFree(to_local_free2);
> + free(to_free3);
> + LocalFree(to_local_free4);
> }
> }
>
Aside from this, the patch works well for me. It is a real usability
improvement. Thank you for working on it.
-- Hannes
^ permalink raw reply
* [PATCH 0/1] completion: complete dir-type option args to am, format_patch
From: Britton Leo Kerin @ 2024-01-07 21:41 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
I think are a few other places this could be done as well but I wanted to
start with just a couple to show the idea.
Britton Leo Kerin (1):
completion: dir-type optargs for am, format-patch
contrib/completion/git-completion.bash | 38 ++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.43.0
^ permalink raw reply
* [PATCH 1/1] completion: dir-type optargs for am, format-patch
From: Britton Leo Kerin @ 2024-01-07 21:42 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240107214200.29684-1-britton.kerin@gmail.com>
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..c3b0a3699c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1356,6 +1356,30 @@ __git_count_arguments ()
printf "%d" $c
}
+
+# Complete actual dir (not pathspec), respecting any -C options.
+#
+# Usage: __git_complete_refs [<option>]...
+# --cur=<word>: The current dir to be completed. Defaults to the current word.
+__git_complete_dir ()
+{
+ local cur_="$cur"
+
+ while test $# != 0; do
+ case "$1" in
+ --cur=*) cur_="${1##--cur=}" ;;
+ *) return 1 ;;
+ esac
+ shift
+ done
+
+ local context_dir=$(__git rev-parse --show-toplevel --show-prefix 2>/dev/null | paste -s -d '/' 2>/dev/null)
+ [ -d "$context_dir" ] || return
+
+ COMPREPLY=$(cd $context_dir 2>/dev/null && compgen -d -- "$cur_")
+}
+
+
__git_whitespacelist="nowarn warn error error-all fix"
__git_patchformat="mbox stgit stgit-series hg mboxrd"
__git_showcurrentpatch="diff raw"
@@ -1374,6 +1398,10 @@ _git_am ()
__gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
return
;;
+ --directory=*)
+ __git_complete_dir --cur="${cur##--directory=}"
+ return
+ ;;
--patch-format=*)
__gitcomp "$__git_patchformat" "" "${cur##--patch-format=}"
return
@@ -1867,7 +1895,17 @@ __git_format_patch_extra_options="
_git_format_patch ()
{
+ case "$prev,$cur" in
+ -o,*)
+ __git_complete_dir
+ return
+ ;;
+ esac
case "$cur" in
+ --output-directory=*)
+ __git_complete_dir --cur="${cur##--output-directory=}"
+ return
+ ;;
--thread=*)
__gitcomp "
deep shallow
--
2.43.0
^ permalink raw reply related
* [PATCH 0/1] completion: send-email: don't complete revs when --no-format-patch
From: Britton Leo Kerin @ 2024-01-08 9:36 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
Along the way I taught __git_find_last_on_cmdline to understand '--', which
isn't stricly necessary but I think reads more clearly at the call sites.
__git_find_on_cmdline could be changed to work the same, or this part dropped
if people don't like it.
Britton Leo Kerin (1):
completion: don't comp revs when --no-format-patch
contrib/completion/git-completion.bash | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.43.0
^ permalink raw reply
* [PATCH 1/1] completion: don't comp revs when --no-format-patch
From: Britton Leo Kerin @ 2024-01-08 9:36 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
In-Reply-To: <20240108093601.136370-1-britton.kerin@gmail.com>
In this case the user has specifically said they don't want send-email
to run format-patch so revs aren't valid argument completions (and it's
likely revs and dirs do have some same names or prefixes as in
Documentation/MyFirstContribution.txt 'psuh').
Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
---
contrib/completion/git-completion.bash | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 185b47d802..c983f3b2ab 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1242,10 +1242,12 @@ __git_find_last_on_cmdline ()
while test $# -gt 1; do
case "$1" in
--show-idx) show_idx=y ;;
+ --) shift && break ;;
*) return 1 ;;
esac
shift
done
+ [ $# -eq 1 ] || return 1 # return 1 if we got wrong # of non-opts
local wordlist="$1"
while [ $c -gt "$__git_cmd_idx" ]; do
@@ -2429,7 +2431,9 @@ _git_send_email ()
return
;;
esac
- __git_complete_revlist
+ if [ "$(__git_find_last_on_cmdline -- "--format-patch --no-format-patch")" != "--no-format-patch" ]; then
+ __git_complete_revlist
+ fi
}
_git_stage ()
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 0/1] completion: send-email: don't complete revs when --no-format-patch
From: Dragan Simic @ 2024-01-08 9:40 UTC (permalink / raw)
To: Britton Leo Kerin; +Cc: git
In-Reply-To: <9627364b-c0c9-4b85-a81a-ba1ef0735c9a@smtp-relay.sendinblue.com>
On 2024-01-08 10:36, Britton Leo Kerin wrote:
> Along the way I taught __git_find_last_on_cmdline to understand '--',
> which
> isn't stricly necessary but I think reads more clearly at the call
> sites.
> __git_find_on_cmdline could be changed to work the same, or this part
> dropped
> if people don't like it.
If I may suggest, there's no need for a cover letter for a single patch.
If you want to include some notes in the patch submission, which aren't
supposed to be part of the commit summary, you can do that in the patch
itself.
> Britton Leo Kerin (1):
> completion: don't comp revs when --no-format-patch
>
> contrib/completion/git-completion.bash | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>
> base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
> --
> 2.43.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox