* [PATCH v3 07/12] builtin/show-ref: stop using global vars for `show_one()`
From: Patrick Steinhardt @ 2023-10-31 8:16 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698739941.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 6148 bytes --]
The `show_one()` function implicitly receives a bunch of options which
are tracked via global variables. This makes it hard to see which
subcommands of git-show-ref(1) actually make use of these options.
Introduce a `show_one_options` structure that gets passed down to this
function. This allows us to get rid of more global state and makes it
more explicit which subcommands use those options.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 62 ++++++++++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 22 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index d0de69e29dd..fb0960ac507 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -18,10 +18,17 @@ static const char * const show_ref_usage[] = {
NULL
};
-static int deref_tags, show_head, tags_only, heads_only, verify,
- quiet, hash_only, abbrev;
+static int show_head, tags_only, heads_only, verify;
-static void show_one(const char *refname, const struct object_id *oid)
+struct show_one_options {
+ int quiet;
+ int hash_only;
+ int abbrev;
+ int deref_tags;
+};
+
+static void show_one(const struct show_one_options *opts,
+ const char *refname, const struct object_id *oid)
{
const char *hex;
struct object_id peeled;
@@ -30,25 +37,26 @@ static void show_one(const char *refname, const struct object_id *oid)
die("git show-ref: bad ref %s (%s)", refname,
oid_to_hex(oid));
- if (quiet)
+ if (opts->quiet)
return;
- hex = repo_find_unique_abbrev(the_repository, oid, abbrev);
- if (hash_only)
+ hex = repo_find_unique_abbrev(the_repository, oid, opts->abbrev);
+ if (opts->hash_only)
printf("%s\n", hex);
else
printf("%s %s\n", hex, refname);
- if (!deref_tags)
+ if (!opts->deref_tags)
return;
if (!peel_iterated_oid(oid, &peeled)) {
- hex = repo_find_unique_abbrev(the_repository, &peeled, abbrev);
+ hex = repo_find_unique_abbrev(the_repository, &peeled, opts->abbrev);
printf("%s %s^{}\n", hex, refname);
}
}
struct show_ref_data {
+ const struct show_one_options *show_one_opts;
const char **patterns;
int found_match;
};
@@ -81,7 +89,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
match:
data->found_match++;
- show_one(refname, oid);
+ show_one(data->show_one_opts, refname, oid);
return 0;
}
@@ -153,7 +161,8 @@ static int cmd_show_ref__exclude_existing(const struct exclude_existing_options
return 0;
}
-static int cmd_show_ref__verify(const char **refs)
+static int cmd_show_ref__verify(const struct show_one_options *show_one_opts,
+ const char **refs)
{
if (!refs || !*refs)
die("--verify requires a reference");
@@ -163,9 +172,9 @@ static int cmd_show_ref__verify(const char **refs)
if ((starts_with(*refs, "refs/") || !strcmp(*refs, "HEAD")) &&
!read_ref(*refs, &oid)) {
- show_one(*refs, &oid);
+ show_one(show_one_opts, *refs, &oid);
}
- else if (!quiet)
+ else if (!show_one_opts->quiet)
die("'%s' - not a valid ref", *refs);
else
return 1;
@@ -175,9 +184,12 @@ static int cmd_show_ref__verify(const char **refs)
return 0;
}
-static int cmd_show_ref__patterns(const char **patterns)
+static int cmd_show_ref__patterns(const struct show_one_options *show_one_opts,
+ const char **patterns)
{
- struct show_ref_data show_ref_data = {0};
+ struct show_ref_data show_ref_data = {
+ .show_one_opts = show_one_opts,
+ };
if (patterns && *patterns)
show_ref_data.patterns = patterns;
@@ -200,11 +212,16 @@ static int cmd_show_ref__patterns(const char **patterns)
static int hash_callback(const struct option *opt, const char *arg, int unset)
{
- hash_only = 1;
+ struct show_one_options *opts = opt->value;
+ struct option abbrev_opt = *opt;
+
+ opts->hash_only = 1;
/* Use full length SHA1 if no argument */
if (!arg)
return 0;
- return parse_opt_abbrev_cb(opt, arg, unset);
+
+ abbrev_opt.value = &opts->abbrev;
+ return parse_opt_abbrev_cb(&abbrev_opt, arg, unset);
}
static int exclude_existing_callback(const struct option *opt, const char *arg,
@@ -220,6 +237,7 @@ static int exclude_existing_callback(const struct option *opt, const char *arg,
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
struct exclude_existing_options exclude_existing_opts = {0};
+ struct show_one_options show_one_opts = {0};
const struct option show_ref_options[] = {
OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
@@ -229,13 +247,13 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
N_("show the HEAD reference, even if it would be filtered out")),
OPT_BOOL(0, "head", &show_head,
N_("show the HEAD reference, even if it would be filtered out")),
- OPT_BOOL('d', "dereference", &deref_tags,
+ OPT_BOOL('d', "dereference", &show_one_opts.deref_tags,
N_("dereference tags into object IDs")),
- OPT_CALLBACK_F('s', "hash", &abbrev, N_("n"),
+ OPT_CALLBACK_F('s', "hash", &show_one_opts, N_("n"),
N_("only show SHA1 hash using <n> digits"),
PARSE_OPT_OPTARG, &hash_callback),
- OPT__ABBREV(&abbrev),
- OPT__QUIET(&quiet,
+ OPT__ABBREV(&show_one_opts.abbrev),
+ OPT__QUIET(&show_one_opts.quiet,
N_("do not print results to stdout (useful with --verify)")),
OPT_CALLBACK_F(0, "exclude-existing", &exclude_existing_opts,
N_("pattern"), N_("show refs from stdin that aren't in local repository"),
@@ -251,7 +269,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
else if (verify)
- return cmd_show_ref__verify(argv);
+ return cmd_show_ref__verify(&show_one_opts, argv);
else
- return cmd_show_ref__patterns(argv);
+ return cmd_show_ref__patterns(&show_one_opts, argv);
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 08/12] builtin/show-ref: refactor options for patterns subcommand
From: Patrick Steinhardt @ 2023-10-31 8:16 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698739941.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3956 bytes --]
The patterns subcommand is the last command that still uses global
variables to track its options. Convert it to use a structure instead
with the same motivation as preceding commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index fb0960ac507..36ac10551da 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -18,8 +18,6 @@ static const char * const show_ref_usage[] = {
NULL
};
-static int show_head, tags_only, heads_only, verify;
-
struct show_one_options {
int quiet;
int hash_only;
@@ -59,6 +57,7 @@ struct show_ref_data {
const struct show_one_options *show_one_opts;
const char **patterns;
int found_match;
+ int show_head;
};
static int show_ref(const char *refname, const struct object_id *oid,
@@ -66,7 +65,7 @@ static int show_ref(const char *refname, const struct object_id *oid,
{
struct show_ref_data *data = cbdata;
- if (show_head && !strcmp(refname, "HEAD"))
+ if (data->show_head && !strcmp(refname, "HEAD"))
goto match;
if (data->patterns) {
@@ -184,22 +183,30 @@ static int cmd_show_ref__verify(const struct show_one_options *show_one_opts,
return 0;
}
-static int cmd_show_ref__patterns(const struct show_one_options *show_one_opts,
+struct patterns_options {
+ int show_head;
+ int heads_only;
+ int tags_only;
+};
+
+static int cmd_show_ref__patterns(const struct patterns_options *opts,
+ const struct show_one_options *show_one_opts,
const char **patterns)
{
struct show_ref_data show_ref_data = {
.show_one_opts = show_one_opts,
+ .show_head = opts->show_head,
};
if (patterns && *patterns)
show_ref_data.patterns = patterns;
- if (show_head)
+ if (opts->show_head)
head_ref(show_ref, &show_ref_data);
- if (heads_only || tags_only) {
- if (heads_only)
+ if (opts->heads_only || opts->tags_only) {
+ if (opts->heads_only)
for_each_fullref_in("refs/heads/", show_ref, &show_ref_data);
- if (tags_only)
+ if (opts->tags_only)
for_each_fullref_in("refs/tags/", show_ref, &show_ref_data);
} else {
for_each_ref(show_ref, &show_ref_data);
@@ -237,15 +244,17 @@ static int exclude_existing_callback(const struct option *opt, const char *arg,
int cmd_show_ref(int argc, const char **argv, const char *prefix)
{
struct exclude_existing_options exclude_existing_opts = {0};
+ struct patterns_options patterns_opts = {0};
struct show_one_options show_one_opts = {0};
+ int verify = 0;
const struct option show_ref_options[] = {
- OPT_BOOL(0, "tags", &tags_only, N_("only show tags (can be combined with heads)")),
- OPT_BOOL(0, "heads", &heads_only, N_("only show heads (can be combined with tags)")),
+ OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with heads)")),
+ OPT_BOOL(0, "heads", &patterns_opts.heads_only, N_("only show heads (can be combined with tags)")),
OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
"requires exact ref path")),
- OPT_HIDDEN_BOOL('h', NULL, &show_head,
+ OPT_HIDDEN_BOOL('h', NULL, &patterns_opts.show_head,
N_("show the HEAD reference, even if it would be filtered out")),
- OPT_BOOL(0, "head", &show_head,
+ OPT_BOOL(0, "head", &patterns_opts.show_head,
N_("show the HEAD reference, even if it would be filtered out")),
OPT_BOOL('d', "dereference", &show_one_opts.deref_tags,
N_("dereference tags into object IDs")),
@@ -271,5 +280,5 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
else if (verify)
return cmd_show_ref__verify(&show_one_opts, argv);
else
- return cmd_show_ref__patterns(&show_one_opts, argv);
+ return cmd_show_ref__patterns(&patterns_opts, &show_one_opts, argv);
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 09/12] builtin/show-ref: ensure mutual exclusiveness of subcommands
From: Patrick Steinhardt @ 2023-10-31 8:16 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698739941.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1883 bytes --]
The git-show-ref(1) command has three different modes, of which one is
implicit and the other two can be chosen explicitly by passing a flag.
But while these modes are standalone and cause us to execute completely
separate code paths, we gladly accept the case where a user asks for
both `--exclude-existing` and `--verify` at the same time even though it
is not obvious what will happen. Spoiler: we ignore `--verify` and
execute the `--exclude-existing` mode.
Let's explicitly detect this invalid usage and die in case both modes
were requested.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/show-ref.c | 4 ++++
t/t1403-show-ref.sh | 9 +++++++++
2 files changed, 13 insertions(+)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 36ac10551da..6685495dd2c 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -275,6 +275,10 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, show_ref_options,
show_ref_usage, 0);
+ if ((!!exclude_existing_opts.enabled + !!verify) > 1)
+ die(_("only one of '%s' or '%s' can be given"),
+ "--exclude-existing", "--verify");
+
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
else if (verify)
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 9252a581abf..f1e0388324e 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -196,4 +196,13 @@ test_expect_success 'show-ref --verify with dangling ref' '
)
'
+test_expect_success 'show-ref sub-modes are mutually exclusive' '
+ cat >expect <<-EOF &&
+ fatal: only one of ${SQ}--exclude-existing${SQ} or ${SQ}--verify${SQ} can be given
+ EOF
+
+ test_must_fail git show-ref --verify --exclude-existing 2>err &&
+ test_cmp expect err
+'
+
test_done
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 10/12] builtin/show-ref: explicitly spell out different modes in synopsis
From: Patrick Steinhardt @ 2023-10-31 8:16 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698739941.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2536 bytes --]
The synopsis treats the `--verify` and the implicit mode the same. They
are slightly different though:
- They accept different sets of flags.
- The implicit mode accepts patterns while the `--verify` mode
accepts references.
Split up the synopsis for these two modes such that we can disambiguate
those differences.
While at it, drop "--quiet" from the pattern mode's synopsis. It does
not make a lot of sense to list patterns, but squelch the listing output
itself. The description for "--quiet" is adapted accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-show-ref.txt | 9 ++++++---
builtin/show-ref.c | 5 ++++-
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index 2fe274b8faa..22f5ebc6a92 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -8,9 +8,12 @@ git-show-ref - List references in a local repository
SYNOPSIS
--------
[verse]
-'git show-ref' [-q | --quiet] [--verify] [--head] [-d | --dereference]
+'git show-ref' [--head] [-d | --dereference]
[-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]
[--heads] [--] [<pattern>...]
+'git show-ref' --verify [-q | --quiet] [-d | --dereference]
+ [-s | --hash[=<n>]] [--abbrev[=<n>]]
+ [--] [<ref>...]
'git show-ref' --exclude-existing[=<pattern>]
DESCRIPTION
@@ -70,8 +73,8 @@ OPTIONS
-q::
--quiet::
- Do not print any results to stdout. When combined with `--verify`, this
- can be used to silently check if a reference exists.
+ Do not print any results to stdout. Can be used with `--verify` to
+ silently check if a reference exists.
--exclude-existing[=<pattern>]::
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6685495dd2c..460f238a62d 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -11,9 +11,12 @@
#include "parse-options.h"
static const char * const show_ref_usage[] = {
- N_("git show-ref [-q | --quiet] [--verify] [--head] [-d | --dereference]\n"
+ N_("git show-ref [--head] [-d | --dereference]\n"
" [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]\n"
" [--heads] [--] [<pattern>...]"),
+ N_("git show-ref --verify [-q | --quiet] [-d | --dereference]\n"
+ " [-s | --hash[=<n>]] [--abbrev[=<n>]]\n"
+ " [--] [<ref>...]"),
N_("git show-ref --exclude-existing[=<pattern>]"),
NULL
};
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 11/12] builtin/show-ref: add new mode to check for reference existence
From: Patrick Steinhardt @ 2023-10-31 8:16 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698739941.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 9741 bytes --]
While we have multiple ways to show the value of a given reference, we
do not have any way to check whether a reference exists at all. While
commands like git-rev-parse(1) or git-show-ref(1) can be used to check
for reference existence in case the reference resolves to something
sane, neither of them can be used to check for existence in some other
scenarios where the reference does not resolve cleanly:
- References which have an invalid name cannot be resolved.
- References to nonexistent objects cannot be resolved.
- Dangling symrefs can be resolved via git-symbolic-ref(1), but this
requires the caller to special case existence checks depending on
whether or not a reference is symbolic or direct.
Furthermore, git-rev-list(1) and other commands do not let the caller
distinguish easily between an actually missing reference and a generic
error.
Taken together, this seems like sufficient motivation to introduce a
separate plumbing command to explicitly check for the existence of a
reference without trying to resolve its contents.
This new command comes in the form of `git show-ref --exists`. This
new mode will exit successfully when the reference exists, with a
specific exit code of 2 when it does not exist, or with 1 when there
has been a generic error.
Note that the only way to properly implement this command is by using
the internal `refs_read_raw_ref()` function. While the public function
`refs_resolve_ref_unsafe()` can be made to behave in the same way by
passing various flags, it does not provide any way to obtain the errno
with which the reference backend failed when reading the reference. As
such, it becomes impossible for us to distinguish generic errors from
the explicit case where the reference wasn't found.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-show-ref.txt | 11 ++++++
builtin/show-ref.c | 49 +++++++++++++++++++++++---
t/t1403-show-ref.sh | 63 +++++++++++++++++++++++++++++++++-
3 files changed, 117 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index 22f5ebc6a92..8fecc9e80f6 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -15,6 +15,7 @@ SYNOPSIS
[-s | --hash[=<n>]] [--abbrev[=<n>]]
[--] [<ref>...]
'git show-ref' --exclude-existing[=<pattern>]
+'git show-ref' --exists <ref>
DESCRIPTION
-----------
@@ -30,6 +31,10 @@ The `--exclude-existing` form is a filter that does the inverse. It reads
refs from stdin, one ref per line, and shows those that don't exist in
the local repository.
+The `--exists` form can be used to check for the existence of a single
+references. This form does not verify whether the reference resolves to an
+actual object.
+
Use of this utility is encouraged in favor of directly accessing files under
the `.git` directory.
@@ -65,6 +70,12 @@ OPTIONS
Aside from returning an error code of 1, it will also print an error
message if `--quiet` was not specified.
+--exists::
+
+ Check whether the given reference exists. Returns an exit code of 0 if
+ it does, 2 if it is missing, and 1 in case looking up the reference
+ failed with an error other than the reference being missing.
+
--abbrev[=<n>]::
Abbreviate the object name. When using `--hash`, you do
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 460f238a62d..7aac525a878 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -2,7 +2,7 @@
#include "config.h"
#include "gettext.h"
#include "hex.h"
-#include "refs.h"
+#include "refs/refs-internal.h"
#include "object-name.h"
#include "object-store-ll.h"
#include "object.h"
@@ -18,6 +18,7 @@ static const char * const show_ref_usage[] = {
" [-s | --hash[=<n>]] [--abbrev[=<n>]]\n"
" [--] [<ref>...]"),
N_("git show-ref --exclude-existing[=<pattern>]"),
+ N_("git show-ref --exists <ref>"),
NULL
};
@@ -220,6 +221,41 @@ static int cmd_show_ref__patterns(const struct patterns_options *opts,
return 0;
}
+static int cmd_show_ref__exists(const char **refs)
+{
+ struct strbuf unused_referent = STRBUF_INIT;
+ struct object_id unused_oid;
+ unsigned int unused_type;
+ int failure_errno = 0;
+ const char *ref;
+ int ret = 0;
+
+ if (!refs || !*refs)
+ die("--exists requires a reference");
+ ref = *refs++;
+ if (*refs)
+ die("--exists requires exactly one reference");
+
+ if (refs_read_raw_ref(get_main_ref_store(the_repository), ref,
+ &unused_oid, &unused_referent, &unused_type,
+ &failure_errno)) {
+ if (failure_errno == ENOENT) {
+ error(_("reference does not exist"));
+ ret = 2;
+ } else {
+ errno = failure_errno;
+ error_errno(_("failed to look up reference"));
+ ret = 1;
+ }
+
+ goto out;
+ }
+
+out:
+ strbuf_release(&unused_referent);
+ return ret;
+}
+
static int hash_callback(const struct option *opt, const char *arg, int unset)
{
struct show_one_options *opts = opt->value;
@@ -249,10 +285,11 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
struct exclude_existing_options exclude_existing_opts = {0};
struct patterns_options patterns_opts = {0};
struct show_one_options show_one_opts = {0};
- int verify = 0;
+ int verify = 0, exists = 0;
const struct option show_ref_options[] = {
OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with heads)")),
OPT_BOOL(0, "heads", &patterns_opts.heads_only, N_("only show heads (can be combined with tags)")),
+ OPT_BOOL(0, "exists", &exists, N_("check for reference existence without resolving")),
OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, "
"requires exact ref path")),
OPT_HIDDEN_BOOL('h', NULL, &patterns_opts.show_head,
@@ -278,14 +315,16 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, show_ref_options,
show_ref_usage, 0);
- if ((!!exclude_existing_opts.enabled + !!verify) > 1)
- die(_("only one of '%s' or '%s' can be given"),
- "--exclude-existing", "--verify");
+ if ((!!exclude_existing_opts.enabled + !!verify + !!exists) > 1)
+ die(_("only one of '%s', '%s' or '%s' can be given"),
+ "--exclude-existing", "--verify", "--exists");
if (exclude_existing_opts.enabled)
return cmd_show_ref__exclude_existing(&exclude_existing_opts);
else if (verify)
return cmd_show_ref__verify(&show_one_opts, argv);
+ else if (exists)
+ return cmd_show_ref__exists(argv);
else
return cmd_show_ref__patterns(&patterns_opts, &show_one_opts, argv);
}
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index f1e0388324e..b50ae6fcf11 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -198,10 +198,71 @@ test_expect_success 'show-ref --verify with dangling ref' '
test_expect_success 'show-ref sub-modes are mutually exclusive' '
cat >expect <<-EOF &&
- fatal: only one of ${SQ}--exclude-existing${SQ} or ${SQ}--verify${SQ} can be given
+ fatal: only one of ${SQ}--exclude-existing${SQ}, ${SQ}--verify${SQ} or ${SQ}--exists${SQ} can be given
EOF
test_must_fail git show-ref --verify --exclude-existing 2>err &&
+ test_cmp expect err &&
+
+ test_must_fail git show-ref --verify --exists 2>err &&
+ test_cmp expect err &&
+
+ test_must_fail git show-ref --exclude-existing --exists 2>err &&
+ test_cmp expect err
+'
+
+test_expect_success '--exists with existing reference' '
+ git show-ref --exists refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+'
+
+test_expect_success '--exists with missing reference' '
+ test_expect_code 2 git show-ref --exists refs/heads/does-not-exist
+'
+
+test_expect_success '--exists does not use DWIM' '
+ test_expect_code 2 git show-ref --exists $GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME 2>err &&
+ grep "reference does not exist" err
+'
+
+test_expect_success '--exists with HEAD' '
+ git show-ref --exists HEAD
+'
+
+test_expect_success '--exists with bad reference name' '
+ test_when_finished "git update-ref -d refs/heads/bad...name" &&
+ new_oid=$(git rev-parse HEAD) &&
+ test-tool ref-store main update-ref msg refs/heads/bad...name $new_oid $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+ git show-ref --exists refs/heads/bad...name
+'
+
+test_expect_success '--exists with arbitrary symref' '
+ test_when_finished "git symbolic-ref -d refs/symref" &&
+ git symbolic-ref refs/symref refs/heads/$GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME &&
+ git show-ref --exists refs/symref
+'
+
+test_expect_success '--exists with dangling symref' '
+ test_when_finished "git symbolic-ref -d refs/heads/dangling" &&
+ git symbolic-ref refs/heads/dangling refs/heads/does-not-exist &&
+ git show-ref --exists refs/heads/dangling
+'
+
+test_expect_success '--exists with nonexistent object ID' '
+ test-tool ref-store main update-ref msg refs/heads/missing-oid $(test_oid 001) $ZERO_OID REF_SKIP_OID_VERIFICATION &&
+ git show-ref --exists refs/heads/missing-oid
+'
+
+test_expect_success '--exists with non-commit object' '
+ tree_oid=$(git rev-parse HEAD^{tree}) &&
+ test-tool ref-store main update-ref msg refs/heads/tree ${tree_oid} $ZERO_OID REF_SKIP_OID_VERIFICATION &&
+ git show-ref --exists refs/heads/tree
+'
+
+test_expect_success '--exists with directory fails with generic error' '
+ cat >expect <<-EOF &&
+ error: failed to look up reference: Is a directory
+ EOF
+ test_expect_code 1 git show-ref --exists refs/heads 2>err &&
test_cmp expect err
'
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v3 12/12] t: use git-show-ref(1) to check for ref existence
From: Patrick Steinhardt @ 2023-10-31 8:16 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Eric Sunshine, Han-Wen Nienhuys
In-Reply-To: <cover.1698739941.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 14354 bytes --]
Convert tests that use `test_path_is_file` and `test_path_is_missing` to
instead use a set of helpers `test_ref_exists` and `test_ref_missing`.
These helpers are implemented via the newly introduced `git show-ref
--exists` command. Thus, we can avoid intimate knowledge of how the ref
backend stores references on disk.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
t/t1430-bad-ref-name.sh | 27 +++++++++++++-------
t/t3200-branch.sh | 33 ++++++++++++++-----------
t/t5521-pull-options.sh | 4 +--
t/t5605-clone-local.sh | 2 +-
t/test-lib-functions.sh | 55 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 94 insertions(+), 27 deletions(-)
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index ff1c967d550..7b7d6953c62 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -205,8 +205,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -216,8 +217,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git branch -d badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
test_must_be_empty error
'
@@ -225,8 +227,9 @@ test_expect_success 'branch -d can delete symref to broken name' '
test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -234,8 +237,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok
test_expect_success 'branch -d can delete dangling symref to broken name' '
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/badname &&
git branch -d badname >output 2>error &&
- test_path_is_missing .git/refs/heads/badname &&
+ test_ref_missing refs/heads/badname &&
test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
test_must_be_empty error
'
@@ -245,8 +249,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
+ test_ref_exists refs/heads/broken...ref &&
git update-ref -d refs/heads/badname >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...ref &&
+ test_ref_missing refs/heads/broken...ref &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -254,8 +259,9 @@ test_expect_success 'update-ref -d can delete broken name through symref' '
test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -263,8 +269,9 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name
test_expect_success 'branch -d can delete symref with broken name' '
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
test_must_be_empty error
'
@@ -272,8 +279,9 @@ test_expect_success 'branch -d can delete symref with broken name' '
test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_must_be_empty output &&
test_must_be_empty error
'
@@ -281,8 +289,9 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br
test_expect_success 'branch -d can delete dangling symref with broken name' '
printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
+ test_ref_exists refs/heads/broken...symref &&
git branch -d broken...symref >output 2>error &&
- test_path_is_missing .git/refs/heads/broken...symref &&
+ test_ref_missing refs/heads/broken...symref &&
test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
test_must_be_empty error
'
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 080e4f24a6e..bde4f1485b7 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -25,7 +25,7 @@ test_expect_success 'prepare a trivial repository' '
test_expect_success 'git branch --help should not have created a bogus branch' '
test_might_fail git branch --man --help </dev/null >/dev/null 2>&1 &&
- test_path_is_missing .git/refs/heads/--help
+ test_ref_missing refs/heads/--help
'
test_expect_success 'branch -h in broken repository' '
@@ -40,7 +40,8 @@ test_expect_success 'branch -h in broken repository' '
'
test_expect_success 'git branch abc should create a branch' '
- git branch abc && test_path_is_file .git/refs/heads/abc
+ git branch abc &&
+ test_ref_exists refs/heads/abc
'
test_expect_success 'git branch abc should fail when abc exists' '
@@ -61,11 +62,13 @@ test_expect_success 'git branch --force abc should succeed when abc exists' '
'
test_expect_success 'git branch a/b/c should create a branch' '
- git branch a/b/c && test_path_is_file .git/refs/heads/a/b/c
+ git branch a/b/c &&
+ test_ref_exists refs/heads/a/b/c
'
test_expect_success 'git branch mb main... should create a branch' '
- git branch mb main... && test_path_is_file .git/refs/heads/mb
+ git branch mb main... &&
+ test_ref_exists refs/heads/mb
'
test_expect_success 'git branch HEAD should fail' '
@@ -78,14 +81,14 @@ EOF
test_expect_success 'git branch --create-reflog d/e/f should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git -c core.logallrefupdates=false branch --create-reflog d/e/f &&
- test_path_is_file .git/refs/heads/d/e/f &&
+ test_ref_exists refs/heads/d/e/f &&
test_path_is_file .git/logs/refs/heads/d/e/f &&
test_cmp expect .git/logs/refs/heads/d/e/f
'
test_expect_success 'git branch -d d/e/f should delete a branch and a log' '
git branch -d d/e/f &&
- test_path_is_missing .git/refs/heads/d/e/f &&
+ test_ref_missing refs/heads/d/e/f &&
test_must_fail git reflog exists refs/heads/d/e/f
'
@@ -213,7 +216,7 @@ test_expect_success 'git branch -M should leave orphaned HEAD alone' '
test_commit initial &&
git checkout --orphan lonely &&
grep lonely .git/HEAD &&
- test_path_is_missing .git/refs/head/lonely &&
+ test_ref_missing refs/head/lonely &&
git branch -M main mistress &&
grep lonely .git/HEAD
)
@@ -799,8 +802,8 @@ test_expect_success 'deleting a symref' '
git symbolic-ref refs/heads/symref refs/heads/target &&
echo "Deleted branch symref (was refs/heads/target)." >expect &&
git branch -d symref >actual &&
- test_path_is_file .git/refs/heads/target &&
- test_path_is_missing .git/refs/heads/symref &&
+ test_ref_exists refs/heads/target &&
+ test_ref_missing refs/heads/symref &&
test_cmp expect actual
'
@@ -809,16 +812,16 @@ test_expect_success 'deleting a dangling symref' '
test_path_is_file .git/refs/heads/dangling-symref &&
echo "Deleted branch dangling-symref (was nowhere)." >expect &&
git branch -d dangling-symref >actual &&
- test_path_is_missing .git/refs/heads/dangling-symref &&
+ test_ref_missing refs/heads/dangling-symref &&
test_cmp expect actual
'
test_expect_success 'deleting a self-referential symref' '
git symbolic-ref refs/heads/self-reference refs/heads/self-reference &&
- test_path_is_file .git/refs/heads/self-reference &&
+ test_ref_exists refs/heads/self-reference &&
echo "Deleted branch self-reference (was refs/heads/self-reference)." >expect &&
git branch -d self-reference >actual &&
- test_path_is_missing .git/refs/heads/self-reference &&
+ test_ref_missing refs/heads/self-reference &&
test_cmp expect actual
'
@@ -826,8 +829,8 @@ test_expect_success 'renaming a symref is not allowed' '
git symbolic-ref refs/heads/topic refs/heads/main &&
test_must_fail git branch -m topic new-topic &&
git symbolic-ref refs/heads/topic &&
- test_path_is_file .git/refs/heads/main &&
- test_path_is_missing .git/refs/heads/new-topic
+ test_ref_exists refs/heads/main &&
+ test_ref_missing refs/heads/new-topic
'
test_expect_success SYMLINKS 'git branch -m u v should fail when the reflog for u is a symlink' '
@@ -1142,7 +1145,7 @@ EOF
test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
GIT_COMMITTER_DATE="2005-05-26 23:30" \
git checkout -b g/h/i -l main &&
- test_path_is_file .git/refs/heads/g/h/i &&
+ test_ref_exists refs/heads/g/h/i &&
test_path_is_file .git/logs/refs/heads/g/h/i &&
test_cmp expect .git/logs/refs/heads/g/h/i
'
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 079b2f2536e..3681859f983 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -143,7 +143,7 @@ test_expect_success 'git pull --dry-run' '
cd clonedry &&
git pull --dry-run ../parent &&
test_path_is_missing .git/FETCH_HEAD &&
- test_path_is_missing .git/refs/heads/main &&
+ test_ref_missing refs/heads/main &&
test_path_is_missing .git/index &&
test_path_is_missing file
)
@@ -157,7 +157,7 @@ test_expect_success 'git pull --all --dry-run' '
git remote add origin ../parent &&
git pull --all --dry-run &&
test_path_is_missing .git/FETCH_HEAD &&
- test_path_is_missing .git/refs/remotes/origin/main &&
+ test_ref_missing refs/remotes/origin/main &&
test_path_is_missing .git/index &&
test_path_is_missing file
)
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 1d7b1abda1a..946c5751885 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -69,7 +69,7 @@ test_expect_success 'local clone of repo with nonexistent ref in HEAD' '
git clone a d &&
(cd d &&
git fetch &&
- test ! -e .git/refs/remotes/origin/HEAD)
+ test_ref_missing refs/remotes/origin/HEAD)
'
test_expect_success 'bundle clone without .bundle suffix' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 2f8868caa17..56b33536ed1 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -251,6 +251,61 @@ debug () {
done
}
+# Usage: test_ref_exists [options] <ref>
+#
+# -C <dir>:
+# Run all git commands in directory <dir>
+#
+# This helper function checks whether a reference exists. Symrefs or object IDs
+# will not be resolved. Can be used to check references with bad names.
+test_ref_exists () {
+ local indir=
+
+ while test $# != 0
+ do
+ case "$1" in
+ -C)
+ indir="$2"
+ shift
+ ;;
+ *)
+ break
+ ;;
+ esac
+ shift
+ done &&
+
+ indir=${indir:+"$indir"/} &&
+
+ if test "$#" != 1
+ then
+ BUG "expected exactly one reference"
+ fi &&
+
+ git ${indir:+ -C "$indir"} show-ref --exists "$1"
+}
+
+# Behaves the same as test_ref_exists, except that it checks for the absence of
+# a reference. This is preferable to `! test_ref_exists` as this function is
+# able to distinguish actually-missing references from other, generic errors.
+test_ref_missing () {
+ test_ref_exists "$@"
+ case "$?" in
+ 2)
+ # This is the good case.
+ return 0
+ ;;
+ 0)
+ echo >&4 "test_ref_missing: reference exists"
+ return 1
+ ;;
+ *)
+ echo >&4 "test_ref_missing: generic error"
+ return 1
+ ;;
+ esac
+}
+
# Usage: test_commit [options] <message> [<file> [<contents> [<tag>]]]
# -C <dir>:
# Run all git commands in directory <dir>
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH] merge: --ff-one-only to apply FF if commit is one
From: Kristoffer Haugsbakk @ 2023-10-31 8:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Josh Soref, git, Ruslan Yakauleu, Taylor Blau
In-Reply-To: <xmqq1qdb8wzk.fsf@gitster.g>
On Tue, Oct 31, 2023, at 01:19, Junio C Hamano wrote:
> Another more fundamental objection is "Why do we special case only a
> singleton commit?"
>
> Why isn't a trivial two-patch series also OK to fast-forward?
> Three? There is no inherent reason to draw a line on one commit
> topic---given that a single commit could be a large and involved one
> that could have been a multi commmit series.
I think it’s about the `--first-parent` view. Then you get a one-commit
view of each pull request that was merged. For a merge commit it serves as
a summary of multiple commits. And a merge commit of one commit would
serve as a summary of one commit. So in that case you remove that extra
level of indirection.
> And you cannot decide if the "topic" is large enough to deserve a
> binding merge commit even if it is a single commit topic, or if is
> small enough and you want to allow fast-forward, without looking at
> it first.
But the pull request is already given: it either has one commit or
several. And you can for sure look at it and either argue that it should
be reduced (squashed) to one commit or maybe expanded (split out) into
several commits.
The point at which you use such a merge feature is when you are already
happy with the pull request (or patch series or whatever else). And then
you (according to this strategy) prefer to avoid merge commits for
single-commit pull requests.
— —
This is not an argument for making this a built-in strategy. Just an
explanation from my vantage point.
--
Kristoffer Haugsbakk
^ permalink raw reply
* [PATCH v4 0/8] ci: add GitLab CI definition
From: Patrick Steinhardt @ 2023-10-31 9:04 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698305961.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 7520 bytes --]
Hi,
this is the fourth version of my patch series that introduces support
for GitLab CI.
Changes compared to v3:
- Stopped using nproc(1) to figure out the number of builds jobs for
GitHub Actions and Azure Pipelines. Instead, we now continue to
use the hardcoded 10 jobs there, whereas on GitLab CI we now use
nproc. We can adapt GitHub/Azure at a later point as required, but
I don't feel comfortable doing changes there.
- Improved the linux-musl job. Namely, we now also install all
required Apache modules, which makes the Apache-based test setup
work. There is a packaging issue with the WebDAV module though, so
we now skip tests that depend on it on Alpine Linux.
I still didn't move `.gitlab-ci.yml` to `contrib/`. As Taylor argued
(and I don't disagree), moving it to `contrib/` would convey the spirit
that this is _not_ an authoritative CI pipeline setup. But I wanted to
hear other opinions first before moving it into `contrib/`.
Patrick
Patrick Steinhardt (8):
ci: reorder definitions for grouping functions
ci: make grouping setup more generic
ci: group installation of Docker dependencies
ci: split out logic to set up failed test artifacts
ci: unify setup of some environment variables
ci: squelch warnings when testing with unusable Git repo
ci: install test dependencies for linux-musl
ci: add support for GitLab CI
.gitlab-ci.yml | 53 +++++++++
ci/install-docker-dependencies.sh | 23 +++-
ci/lib.sh | 190 +++++++++++++++++++++---------
ci/print-test-failures.sh | 6 +
t/lib-httpd.sh | 17 ++-
5 files changed, 233 insertions(+), 56 deletions(-)
create mode 100644 .gitlab-ci.yml
Range-diff against v3:
1: ef44ed5c3b1 = 1: 8595fe5016a ci: reorder definitions for grouping functions
2: 77798fa7a7a = 2: 7358a943392 ci: make grouping setup more generic
3: 4542bd38dc2 = 3: 6d842592c6f ci: group installation of Docker dependencies
4: 5fdda7fd83f = 4: e15651b3f5d ci: split out logic to set up failed test artifacts
5: 6af0075fd87 ! 5: a64799b6e25 ci: unify setup of some environment variables
@@ Commit message
parts are separated. While at it, we also perform some additional small
improvements:
- - We use nproc instead of a hardcoded count of jobs for make and
- prove. This ensures that the number of concurrent processes adapts
- to the host automatically.
-
- We now always pass `--state=failed,slow,save` via GIT_PROVE_OPTS.
It doesn't hurt on platforms where we don't persist the state, so
this further reduces boilerplate.
@@ ci/lib.sh: then
exit 1
fi
-+MAKEFLAGS="$MAKEFLAGS --jobs=$(nproc)"
-+GIT_PROVE_OPTS="--timer --jobs $(nproc) --state=failed,slow,save"
++MAKEFLAGS="$MAKEFLAGS --jobs=10"
++GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
+
+GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
+if test windows = "$CI_OS_NAME"
6: 78d863bf24e = 6: f7d2a8666fe ci: squelch warnings when testing with unusable Git repo
7: f150d61a1ce ! 7: 9b43b0d90e3 ci: install test dependencies for linux-musl
@@ Commit message
available, all Subversion-related tests are skipped without the
SVN::Core Perl library anyway.
- Furthermore, in order to make the Apache-based tests work, this commit
- also adds the Alpine-specific modules path of it to the list of known
- paths.
+ The Apache2-based tests require a bit more care though. For one, the
+ module path is different on Alpine Linux, which requires us to add it to
+ the list of known module paths to detect it. But second, the WebDAV
+ module on Alpine Linux is broken because it does not bundle the default
+ database backend [1]. We thus need to skip the WebDAV-based tests on
+ Alpine Linux for now.
+
+ [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13112
Signed-off-by: Patrick Steinhardt <ps@pks.im>
@@ ci/install-docker-dependencies.sh: linux32)
apk add --update build-base curl-dev openssl-dev expat-dev gettext \
- pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
+ pcre2-dev python3 musl-libintl perl-utils ncurses \
-+ apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
++ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
++ bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
pedantic)
dnf -yq update >/dev/null &&
@@ t/lib-httpd.sh: for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
do
if test -d "$DEFAULT_HTTPD_MODULE_PATH"
then
+@@ t/lib-httpd.sh: else
+ "Could not identify web server at '$LIB_HTTPD_PATH'"
+ fi
+
++if test -n "$LIB_HTTPD_DAV" && test -f /etc/os-release
++then
++ case "$(grep "^ID=" /etc/os-release | cut -d= -f2-)" in
++ alpine)
++ # The WebDAV module in Alpine Linux is broken at least up to
++ # Alpine v3.16 as the default DBM driver is missing.
++ #
++ # https://gitlab.alpinelinux.org/alpine/aports/-/issues/13112
++ test_skip_or_die GIT_TEST_HTTPD \
++ "Apache WebDAV module does not have default DBM backend driver"
++ ;;
++ esac
++fi
++
+ install_script () {
+ write_script "$HTTPD_ROOT_PATH/$1" <"$TEST_PATH/$1"
+ }
8: 5272d66d9f1 ! 8: f3f2c98a0dc ci: add support for GitLab CI
@@ ci/install-docker-dependencies.sh: linux32)
- apk add --update build-base curl-dev openssl-dev expat-dev gettext \
+ apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
pcre2-dev python3 musl-libintl perl-utils ncurses \
- apache2 bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
+ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
+ bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
+linux-*)
+ apt update -q &&
@@ ci/lib.sh: then
else
begin_group () { :; }
end_group () { :; }
+@@ ci/lib.sh: then
+ cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
+
+ GIT_TEST_OPTS="--write-junit-xml"
++ JOBS=10
+ elif test true = "$GITHUB_ACTIONS"
+ then
+ CI_TYPE=github-actions
@@ ci/lib.sh: then
cache_dir="$HOME/none"
GIT_TEST_OPTS="--github-workflow-markup"
++ JOBS=10
+elif test true = "$GITLAB_CI"
+then
+ CI_TYPE=gitlab-ci
@@ ci/lib.sh: then
+ cache_dir="$HOME/none"
+
+ runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
++ JOBS=$(nproc)
else
echo "Could not identify CI type" >&2
env >&2
+ exit 1
+ fi
+
+-MAKEFLAGS="$MAKEFLAGS --jobs=10"
+-GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
++MAKEFLAGS="$MAKEFLAGS --jobs=${JOBS}"
++GIT_PROVE_OPTS="--timer --jobs ${JOBS} --state=failed,slow,save"
+
+ GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
+ if test windows = "$CI_OS_NAME"
## ci/print-test-failures.sh ##
@@ ci/print-test-failures.sh: do
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v4 1/8] ci: reorder definitions for grouping functions
From: Patrick Steinhardt @ 2023-10-31 9:04 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]
We define a set of grouping functions that are used to group together
output in our CI, where these groups then end up as collapsible sections
in the respective pipeline platform. The way these functions are defined
is not easily extensible though as we have an up front check for the CI
_not_ being GitHub Actions, where we define the non-stub logic in the
else branch.
Reorder the conditional branches such that we explicitly handle GitHub
Actions.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index 6fbb5bade12..eb384f4e952 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -1,16 +1,7 @@
# Library of functions shared by all CI scripts
-if test true != "$GITHUB_ACTIONS"
+if test true = "$GITHUB_ACTIONS"
then
- begin_group () { :; }
- end_group () { :; }
-
- group () {
- shift
- "$@"
- }
- set -x
-else
begin_group () {
need_to_end_group=t
echo "::group::$1" >&2
@@ -42,6 +33,15 @@ else
}
begin_group "CI setup"
+else
+ begin_group () { :; }
+ end_group () { :; }
+
+ group () {
+ shift
+ "$@"
+ }
+ set -x
fi
# Set 'exit on error' for all CI scripts to let the caller know that
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 2/8] ci: make grouping setup more generic
From: Patrick Steinhardt @ 2023-10-31 9:04 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]
Make the grouping setup more generic by always calling `begin_group ()`
and `end_group ()` regardless of whether we have stubbed those functions
or not. This ensures we can more readily add support for additional CI
platforms.
Furthermore, the `group ()` function is made generic so that it is the
same for both GitHub Actions and for other platforms. There is a
semantic conflict here though: GitHub Actions used to call `set +x` in
`group ()` whereas the non-GitHub case unconditionally uses `set -x`.
The latter would get overriden if we kept the `set +x` in the generic
version of `group ()`. To resolve this conflict, we simply drop the `set
+x` in the generic variant of this function. As `begin_group ()` calls
`set -x` anyway this is not much of a change though, as the only
commands that aren't printed anymore now are the ones between the
beginning of `group ()` and the end of `begin_group ()`.
Last, this commit changes `end_group ()` to also accept a parameter that
indicates _which_ group should end. This will be required by a later
commit that introduces support for GitLab CI.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 46 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index eb384f4e952..b3411afae8e 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -14,36 +14,34 @@ then
need_to_end_group=
echo '::endgroup::' >&2
}
- trap end_group EXIT
-
- group () {
- set +x
- begin_group "$1"
- shift
- # work around `dash` not supporting `set -o pipefail`
- (
- "$@" 2>&1
- echo $? >exit.status
- ) |
- sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
- res=$(cat exit.status)
- rm exit.status
- end_group
- return $res
- }
-
- begin_group "CI setup"
else
begin_group () { :; }
end_group () { :; }
- group () {
- shift
- "$@"
- }
set -x
fi
+group () {
+ group="$1"
+ shift
+ begin_group "$group"
+
+ # work around `dash` not supporting `set -o pipefail`
+ (
+ "$@" 2>&1
+ echo $? >exit.status
+ ) |
+ sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/'
+ res=$(cat exit.status)
+ rm exit.status
+
+ end_group "$group"
+ return $res
+}
+
+begin_group "CI setup"
+trap "end_group 'CI setup'" EXIT
+
# Set 'exit on error' for all CI scripts to let the caller know that
# something went wrong.
#
@@ -287,5 +285,5 @@ esac
MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
-end_group
+end_group "CI setup"
set -x
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 3/8] ci: group installation of Docker dependencies
From: Patrick Steinhardt @ 2023-10-31 9:04 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
The output of CI jobs tends to be quite long-winded and hard to digest.
To help with this, many CI systems provide the ability to group output
into collapsible sections, and we're also doing this in some of our
scripts.
One notable omission is the script to install Docker dependencies.
Address it to bring more structure to the output for Docker-based jobs.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/install-docker-dependencies.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 78b7e326da6..d0bc19d3bb3 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -3,6 +3,10 @@
# Install dependencies required to build and test Git inside container
#
+. ${0%/*}/lib.sh
+
+begin_group "Install dependencies"
+
case "$jobname" in
linux32)
linux32 --32bit i386 sh -c '
@@ -20,3 +24,5 @@ pedantic)
dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
;;
esac
+
+end_group "Install dependencies"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 4/8] ci: split out logic to set up failed test artifacts
From: Patrick Steinhardt @ 2023-10-31 9:04 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]
We have some logic in place to create a directory with the output from
failed tests, which will then subsequently be uploaded as CI artifacts.
We're about to add support for GitLab CI, which will want to reuse the
logic.
Split the logic into a separate function so that it is reusable.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index b3411afae8e..9ffdf743903 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -131,6 +131,27 @@ handle_failed_tests () {
return 1
}
+create_failed_test_artifacts () {
+ mkdir -p t/failed-test-artifacts
+
+ for test_exit in t/test-results/*.exit
+ do
+ test 0 != "$(cat "$test_exit")" || continue
+
+ test_name="${test_exit%.exit}"
+ test_name="${test_name##*/}"
+ printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
+ echo "The full logs are in the 'print test failures' step below."
+ echo "See also the 'failed-tests-*' artifacts attached to this run."
+ cat "t/test-results/$test_name.markup"
+
+ trash_dir="t/trash directory.$test_name"
+ cp "t/test-results/$test_name.out" t/failed-test-artifacts/
+ tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+ done
+ return 1
+}
+
# GitHub Action doesn't set TERM, which is required by tput
export TERM=${TERM:-dumb}
@@ -171,25 +192,8 @@ then
CC="${CC_PACKAGE:-${CC:-gcc}}"
DONT_SKIP_TAGS=t
handle_failed_tests () {
- mkdir -p t/failed-test-artifacts
echo "FAILED_TEST_ARTIFACTS=t/failed-test-artifacts" >>$GITHUB_ENV
-
- for test_exit in t/test-results/*.exit
- do
- test 0 != "$(cat "$test_exit")" || continue
-
- test_name="${test_exit%.exit}"
- test_name="${test_name##*/}"
- printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n"
- echo "The full logs are in the 'print test failures' step below."
- echo "See also the 'failed-tests-*' artifacts attached to this run."
- cat "t/test-results/$test_name.markup"
-
- trash_dir="t/trash directory.$test_name"
- cp "t/test-results/$test_name.out" t/failed-test-artifacts/
- tar czf t/failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
- done
- return 1
+ create_failed_test_artifacts
}
cache_dir="$HOME/none"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 5/8] ci: unify setup of some environment variables
From: Patrick Steinhardt @ 2023-10-31 9:04 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2626 bytes --]
Both GitHub Actions and Azue Pipelines set up the environment variables
GIT_TEST_OPTS, GIT_PROVE_OPTS and MAKEFLAGS. And while most values are
actually the same, the setup is completely duplicate. With the upcoming
support for GitLab CI this duplication would only extend even further.
Unify the setup of those environment variables so that only the uncommon
parts are separated. While at it, we also perform some additional small
improvements:
- We now always pass `--state=failed,slow,save` via GIT_PROVE_OPTS.
It doesn't hurt on platforms where we don't persist the state, so
this further reduces boilerplate.
- When running on Windows systems we set `--no-chain-lint` and
`--no-bin-wrappers`. Interestingly though, we did so _after_
already having exported the respective environment variables.
- We stop using `export VAR=value` syntax, which is a Bashism. It's
not quite worth it as we still use this syntax all over the place,
but it doesn't hurt readability either.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/ci/lib.sh b/ci/lib.sh
index 9ffdf743903..9a9b92c05b3 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -175,11 +175,7 @@ then
# among *all* phases)
cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
- export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
- export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
- MAKEFLAGS="$MAKEFLAGS --jobs=10"
- test windows_nt != "$CI_OS_NAME" ||
- GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
+ GIT_TEST_OPTS="--write-junit-xml"
elif test true = "$GITHUB_ACTIONS"
then
CI_TYPE=github-actions
@@ -198,17 +194,25 @@ then
cache_dir="$HOME/none"
- export GIT_PROVE_OPTS="--timer --jobs 10"
- export GIT_TEST_OPTS="--verbose-log -x --github-workflow-markup"
- MAKEFLAGS="$MAKEFLAGS --jobs=10"
- test windows != "$CI_OS_NAME" ||
- GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
+ GIT_TEST_OPTS="--github-workflow-markup"
else
echo "Could not identify CI type" >&2
env >&2
exit 1
fi
+MAKEFLAGS="$MAKEFLAGS --jobs=10"
+GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
+
+GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
+if test windows = "$CI_OS_NAME"
+then
+ GIT_TEST_OPTS="$GIT_TEST_OPTS --no-chain-lint --no-bin-wrappers"
+fi
+
+export GIT_TEST_OPTS
+export GIT_PROVE_OPTS
+
good_trees_file="$cache_dir/good-trees"
mkdir -p "$cache_dir"
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 6/8] ci: squelch warnings when testing with unusable Git repo
From: Patrick Steinhardt @ 2023-10-31 9:05 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]
Our CI jobs that run on Docker also use mostly the same architecture to
build and test Git via the "ci/run-build-and-tests.sh" script. These
scripts also provide some functionality to massage the Git repository
we're supposedly operating in.
In our Docker-based infrastructure we may not even have a Git repository
available though, which leads to warnings when those functions execute.
Make the helpers exit gracefully in case either there is no Git in our
PATH, or when not running in a Git repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/lib.sh | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/ci/lib.sh b/ci/lib.sh
index 9a9b92c05b3..e14b1029fad 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -69,10 +69,32 @@ skip_branch_tip_with_tag () {
fi
}
+# Check whether we can use the path passed via the first argument as Git
+# repository.
+is_usable_git_repository () {
+ # We require Git in our PATH, otherwise we cannot access repositories
+ # at all.
+ if ! command -v git >/dev/null
+ then
+ return 1
+ fi
+
+ # And the target directory needs to be a proper Git repository.
+ if ! git -C "$1" rev-parse 2>/dev/null
+ then
+ return 1
+ fi
+}
+
# Save some info about the current commit's tree, so we can skip the build
# job if we encounter the same tree again and can provide a useful info
# message.
save_good_tree () {
+ if ! is_usable_git_repository .
+ then
+ return
+ fi
+
echo "$(git rev-parse $CI_COMMIT^{tree}) $CI_COMMIT $CI_JOB_NUMBER $CI_JOB_ID" >>"$good_trees_file"
# limit the file size
tail -1000 "$good_trees_file" >"$good_trees_file".tmp
@@ -88,6 +110,11 @@ skip_good_tree () {
return
fi
+ if ! is_usable_git_repository .
+ then
+ return
+ fi
+
if ! good_tree_info="$(grep "^$(git rev-parse $CI_COMMIT^{tree}) " "$good_trees_file")"
then
# Haven't seen this tree yet, or no cached good trees file yet.
@@ -119,6 +146,11 @@ skip_good_tree () {
}
check_unignored_build_artifacts () {
+ if ! is_usable_git_repository .
+ then
+ return
+ fi
+
! git ls-files --other --exclude-standard --error-unmatch \
-- ':/*' 2>/dev/null ||
{
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 7/8] ci: install test dependencies for linux-musl
From: Patrick Steinhardt @ 2023-10-31 9:05 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 3180 bytes --]
The linux-musl CI job executes tests on Alpine Linux, which is based on
musl libc instead of glibc. We're missing some test dependencies though,
which causes us to skip a subset of tests.
Install these test dependencies to increase our test coverage on this
platform. There are still some missing test dependecies, but these do
not have a corresponding package in the Alpine repositories:
- p4 and p4d, both parts of the Perforce version control system.
- cvsps, which generates patch sets for CVS.
- Subversion and the SVN::Core Perl library, the latter of which is
not available in the Alpine repositories. While the tool itself is
available, all Subversion-related tests are skipped without the
SVN::Core Perl library anyway.
The Apache2-based tests require a bit more care though. For one, the
module path is different on Alpine Linux, which requires us to add it to
the list of known module paths to detect it. But second, the WebDAV
module on Alpine Linux is broken because it does not bundle the default
database backend [1]. We thus need to skip the WebDAV-based tests on
Alpine Linux for now.
[1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13112
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
ci/install-docker-dependencies.sh | 4 +++-
t/lib-httpd.sh | 17 ++++++++++++++++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index d0bc19d3bb3..6e845283680 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -17,7 +17,9 @@ linux32)
;;
linux-musl)
apk add --update build-base curl-dev openssl-dev expat-dev gettext \
- pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null
+ pcre2-dev python3 musl-libintl perl-utils ncurses \
+ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
+ bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
pedantic)
dnf -yq update >/dev/null &&
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 2fb1b2ae561..9ea74927c40 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -67,7 +67,8 @@ for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
'/usr/lib/apache2/modules' \
'/usr/lib64/httpd/modules' \
'/usr/lib/httpd/modules' \
- '/usr/libexec/httpd'
+ '/usr/libexec/httpd' \
+ '/usr/lib/apache2'
do
if test -d "$DEFAULT_HTTPD_MODULE_PATH"
then
@@ -127,6 +128,20 @@ else
"Could not identify web server at '$LIB_HTTPD_PATH'"
fi
+if test -n "$LIB_HTTPD_DAV" && test -f /etc/os-release
+then
+ case "$(grep "^ID=" /etc/os-release | cut -d= -f2-)" in
+ alpine)
+ # The WebDAV module in Alpine Linux is broken at least up to
+ # Alpine v3.16 as the default DBM driver is missing.
+ #
+ # https://gitlab.alpinelinux.org/alpine/aports/-/issues/13112
+ test_skip_or_die GIT_TEST_HTTPD \
+ "Apache WebDAV module does not have default DBM backend driver"
+ ;;
+ esac
+fi
+
install_script () {
write_script "$HTTPD_ROOT_PATH/$1" <"$TEST_PATH/$1"
}
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH v4 8/8] ci: add support for GitLab CI
From: Patrick Steinhardt @ 2023-10-31 9:05 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <cover.1698742590.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 7617 bytes --]
We already support Azure Pipelines and GitHub Workflows in the Git
project, but until now we do not have support for GitLab CI. While it is
arguably not in the interest of the Git project to maintain a ton of
different CI platforms, GitLab has recently ramped up its efforts and
tries to contribute to the Git project more regularly.
Part of a problem we hit at GitLab rather frequently is that our own,
custom CI setup we have is so different to the setup that the Git
project has. More esoteric jobs like "linux-TEST-vars" that also set a
couple of environment variables do not exist in GitLab's custom CI
setup, and maintaining them to keep up with what Git does feels like
wasted time. The result is that we regularly send patch series upstream
that fail to compile or pass tests in GitHub Workflows. We would thus
like to integrate the GitLab CI configuration into the Git project to
help us send better patch series upstream and thus reduce overhead for
the maintainer.
The integration does not necessarily have to be a first-class citizen,
which would in practice only add to the fallout that pipeline failures
have for the maintainer. That being said, we are happy to maintain this
alternative CI setup for the Git project and will make test results
available as part of our own mirror of the Git project at [1].
This commit introduces the integration into our regular CI scripts so
that most of the setup continues to be shared across all of the CI
solutions. Note that as the builds on GitLab CI run as unprivileged
user, we need to pull in both sudo and shadow packages to our Alpine
based job to set this up.
[1]: https://gitlab.com/gitlab-org/git
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
.gitlab-ci.yml | 53 +++++++++++++++++++++++++++++++
ci/install-docker-dependencies.sh | 13 +++++++-
ci/lib.sh | 50 +++++++++++++++++++++++++++--
ci/print-test-failures.sh | 6 ++++
4 files changed, 119 insertions(+), 3 deletions(-)
create mode 100644 .gitlab-ci.yml
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
new file mode 100644
index 00000000000..cd98bcb18aa
--- /dev/null
+++ b/.gitlab-ci.yml
@@ -0,0 +1,53 @@
+default:
+ timeout: 2h
+
+workflow:
+ rules:
+ - if: $CI_PIPELINE_SOURCE == "merge_request_event"
+ - if: $CI_COMMIT_TAG
+ - if: $CI_COMMIT_REF_PROTECTED == "true"
+
+test:
+ image: $image
+ before_script:
+ - ./ci/install-docker-dependencies.sh
+ script:
+ - useradd builder --create-home
+ - chown -R builder "${CI_PROJECT_DIR}"
+ - sudo --preserve-env --set-home --user=builder ./ci/run-build-and-tests.sh
+ after_script:
+ - |
+ if test "$CI_JOB_STATUS" != 'success'
+ then
+ sudo --preserve-env --set-home --user=builder ./ci/print-test-failures.sh
+ fi
+ parallel:
+ matrix:
+ - jobname: linux-sha256
+ image: ubuntu:latest
+ CC: clang
+ - jobname: linux-gcc
+ image: ubuntu:20.04
+ CC: gcc
+ CC_PACKAGE: gcc-8
+ - jobname: linux-TEST-vars
+ image: ubuntu:20.04
+ CC: gcc
+ CC_PACKAGE: gcc-8
+ - jobname: linux-gcc-default
+ image: ubuntu:latest
+ CC: gcc
+ - jobname: linux-leaks
+ image: ubuntu:latest
+ CC: gcc
+ - jobname: linux-asan-ubsan
+ image: ubuntu:latest
+ CC: clang
+ - jobname: pedantic
+ image: fedora:latest
+ - jobname: linux-musl
+ image: alpine:latest
+ artifacts:
+ paths:
+ - t/failed-test-artifacts
+ when: on_failure
diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
index 6e845283680..48cb2e735b5 100755
--- a/ci/install-docker-dependencies.sh
+++ b/ci/install-docker-dependencies.sh
@@ -7,6 +7,9 @@
begin_group "Install dependencies"
+# Required so that apt doesn't wait for user input on certain packages.
+export DEBIAN_FRONTEND=noninteractive
+
case "$jobname" in
linux32)
linux32 --32bit i386 sh -c '
@@ -16,11 +19,19 @@ linux32)
'
;;
linux-musl)
- apk add --update build-base curl-dev openssl-dev expat-dev gettext \
+ apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
pcre2-dev python3 musl-libintl perl-utils ncurses \
apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
;;
+linux-*)
+ apt update -q &&
+ apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
+ libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
+ perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
+ libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
+ apache2 cvs cvsps gnupg libcgi-pm-perl subversion
+ ;;
pedantic)
dnf -yq update >/dev/null &&
dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
diff --git a/ci/lib.sh b/ci/lib.sh
index e14b1029fad..6e3d64004ec 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -14,6 +14,22 @@ then
need_to_end_group=
echo '::endgroup::' >&2
}
+elif test true = "$GITLAB_CI"
+then
+ begin_group () {
+ need_to_end_group=t
+ printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1\n"
+ trap "end_group '$1'" EXIT
+ set -x
+ }
+
+ end_group () {
+ test -n "$need_to_end_group" || return 0
+ set +x
+ need_to_end_group=
+ printf "\e[0Ksection_end:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K\n"
+ trap - EXIT
+ }
else
begin_group () { :; }
end_group () { :; }
@@ -208,6 +224,7 @@ then
cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
GIT_TEST_OPTS="--write-junit-xml"
+ JOBS=10
elif test true = "$GITHUB_ACTIONS"
then
CI_TYPE=github-actions
@@ -227,14 +244,43 @@ then
cache_dir="$HOME/none"
GIT_TEST_OPTS="--github-workflow-markup"
+ JOBS=10
+elif test true = "$GITLAB_CI"
+then
+ CI_TYPE=gitlab-ci
+ CI_BRANCH="$CI_COMMIT_REF_NAME"
+ CI_COMMIT="$CI_COMMIT_SHA"
+ case "$CI_JOB_IMAGE" in
+ macos-*)
+ CI_OS_NAME=osx;;
+ alpine:*|fedora:*|ubuntu:*)
+ CI_OS_NAME=linux;;
+ *)
+ echo "Could not identify OS image" >&2
+ env >&2
+ exit 1
+ ;;
+ esac
+ CI_REPO_SLUG="$CI_PROJECT_PATH"
+ CI_JOB_ID="$CI_JOB_ID"
+ CC="${CC_PACKAGE:-${CC:-gcc}}"
+ DONT_SKIP_TAGS=t
+ handle_failed_tests () {
+ create_failed_test_artifacts
+ }
+
+ cache_dir="$HOME/none"
+
+ runs_on_pool=$(echo "$CI_JOB_IMAGE" | tr : -)
+ JOBS=$(nproc)
else
echo "Could not identify CI type" >&2
env >&2
exit 1
fi
-MAKEFLAGS="$MAKEFLAGS --jobs=10"
-GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
+MAKEFLAGS="$MAKEFLAGS --jobs=${JOBS}"
+GIT_PROVE_OPTS="--timer --jobs ${JOBS} --state=failed,slow,save"
GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
if test windows = "$CI_OS_NAME"
diff --git a/ci/print-test-failures.sh b/ci/print-test-failures.sh
index 57277eefcd0..c33ad4e3a22 100755
--- a/ci/print-test-failures.sh
+++ b/ci/print-test-failures.sh
@@ -51,6 +51,12 @@ do
tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
continue
;;
+ gitlab-ci)
+ mkdir -p failed-test-artifacts
+ cp "${TEST_EXIT%.exit}.out" failed-test-artifacts/
+ tar czf failed-test-artifacts/"$test_name".trash.tar.gz "$trash_dir"
+ continue
+ ;;
*)
echo "Unhandled CI type: $CI_TYPE" >&2
exit 1
--
2.42.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [EXT] Re: ls-remote bug
From: René Scharfe @ 2023-10-31 10:00 UTC (permalink / raw)
To: Lior Zeltzer, Git Mailing List
Cc: Andrzej Hunt, Ævar Arnfjörð Bjarmason,
Junio C Hamano, Bagas Sanjaya
In-Reply-To: <BL0PR18MB2130C503D3608186AFC516D2BAA1A@BL0PR18MB2130.namprd18.prod.outlook.com>
Am 30.10.23 um 09:17 schrieb Lior Zeltzer:
> But why when cancelling stderr redirect to stdout (2>&1) all works well ?
Good question. Maybe the output on stderr can give a hint? Any error
messages? I assume there is *some* output, otherwise the redirection
should have no effect.
René
^ permalink raw reply
* Re: [PATCH v2 0/1] Object ID support for git merge-file
From: Phillip Wood @ 2023-10-31 11:05 UTC (permalink / raw)
To: brian m. carlson, git
Cc: Junio C Hamano, Elijah Newren, Eric Sunshine, Taylor Blau
In-Reply-To: <20231030162658.567523-1-sandals@crustytoothpaste.net>
Hi brian
Thanks for the re-roll, this version looks good to me
Phillip
On 30/10/2023 16:26, brian m. carlson wrote:
> This series introduces an --object-id option to git merge-file such
> that, instead of reading and writing from files on the system, it reads
> from and writes to the object store using blobs.
>
> Changes from v1:
> * Improve error handling
> * Re-add `-p` argument for documentation
>
> brian m. carlson (1):
> merge-file: add an option to process object IDs
>
> Documentation/git-merge-file.txt | 20 +++++++++++
> builtin/merge-file.c | 62 +++++++++++++++++++++++---------
> t/t6403-merge-file.sh | 58 ++++++++++++++++++++++++++++++
> 3 files changed, 124 insertions(+), 16 deletions(-)
>
>
^ permalink raw reply
* Re: [PATCH v2 0/2] sequencer: remove use of hardcoded comment char
From: Phillip Wood @ 2023-10-31 11:18 UTC (permalink / raw)
To: Elijah Newren, Tony Tung via GitGitGadget; +Cc: git, Tony Tung
In-Reply-To: <CABPp-BEjV0H=waNQfKNNqibs3g_BU1CCrNjb8G8h_jXrt8kaiw@mail.gmail.com>
Hi Elijah
On 31/10/2023 06:55, Elijah Newren wrote:
> Hi,
>
> On Mon, Oct 30, 2023 at 10:09 PM Tony Tung via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> Instead of using the hardcoded # , use the user-defined comment_line_char.
>> Adds a test to prevent regressions.
>>
>> Tony Tung (2):
>> sequencer: remove use of comment character
>> sequencer: fix remaining hardcoded comment char
>
> The second commit message seems to suggest that the two commits should
> just be squashed; there's no explicit or even implicit reason provided
> for why the two small patches are logically independent. After
> reading them carefully, and digging through the particular changes
> being made and what part of the code they touch, I think I can guess
> at a potential reason, but I feel like I'm crossing into the territory
> of mind reading trying to articulate that reason. (Besides, my
> rationale would argue that the two patches should be split
> differently.) Perhaps a comment could be added, to either the second
> commit message or the cover letter, to explain that better?
>
> More importantly, though, I think the second commit message is
> actually wrong. Before and after applying this series:
>
> $ git grep -c -e '".*#' -e "'#'" -- sequencer.c
> sequencer.c:16
>
> $ b4 am c9f4ff34dbdb7ba221e4203bb6551b80948dc71d.1698728953.git.gitgitgadget@gmail.com
> $ git am ./v2_20231031_gitgitgadget_sequencer_remove_use_of_hardcoded_comment_char.mbx
>
> $ git grep -c -e '".*#' -e "'#'" -- sequencer.c
> sequencer.c:12
As far as I can see those remaining instances are all to do with the '#'
that separates a merge subject line from its parents. I don't think we
need to complicate things anymore by respecting core.commentchar there
as the '#' is not denoting a commented line, it is being used as an
intra-line separator instead.
> Granted, four of those lines are code comments, but that still leaves
> 8 hard coded references to '#' in the code at the end (i.e. the
> majority are still left), meaning your second patch doesn't do what
> its subject line claims.
>
> And, most important of all is still the first patch. As I stated
> elsewhere in this thread (at
> CABPp-BFY7m_g+sT131_Ubxqo5FsHGKOPMng7=90_0-+xCS9NEQ@mail.gmail.com):
>
> """
> I think supporting comment_line_char for the TODO file provides no
> value, and I think the easier fix would be undoing the uses of
> comment_line_char relative to the TODO file (perhaps also leaving
> in-code comments to the effect that comment_line_char just doesn't
> apply to the TODO file).
I agree that I don't see much point in respecting core.commentchar in
the TODO file as unlike a commit message a legitimate non-commented line
will never begin with '#'. Unfortunately I think we're committed to
respecting it - see 180bad3d10f (rebase -i: respect core.commentchar,
2013-02-11)
> [...]
> I feel quite differently about patches that make COMMIT_EDITMSG
> handling use comment_line_char more consistently since that code
> simply writes the file without re-parsing it; although fixing
> everything would be best, even fixing some of them to use
> comment_line_char would be welcome. I think the first two hunks of
> your second patch happen to fall into this category, so if those were
> split out, then I'd say those are good partial solutions.
I think splitting the changes so that we have one patch that fixes the
TODO file generation and another that fixes the commit message
generation for fixup commands would be best.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v2 2/2] sequencer: fix remaining hardcoded comment char
From: Phillip Wood @ 2023-10-31 11:27 UTC (permalink / raw)
To: Tony Tung via GitGitGadget, git; +Cc: Elijah Newren, Tony Tung
In-Reply-To: <c9f4ff34dbdb7ba221e4203bb6551b80948dc71d.1698728953.git.gitgitgadget@gmail.com>
Hi Tony
Thanks for working on this. When you're submitting patches please try
testing them locally and check the CI before doing '/submit'. In this
case all the jobs that try to compile git are failing - see
https://github.com/gitgitgadget/git/actions/runs/6702301267/job/18211090139?pr=1603#step:4:260
for example.
On 31/10/2023 05:09, Tony Tung via GitGitGadget wrote:
> From: Tony Tung <tonytung@merly.org>
>
> Signed-off-by: Tony Tung <tonytung@merly.org>
> ---
> sequencer.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 8c6666d5e43..bbadc3fb710 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1893,8 +1893,14 @@ static void update_squash_message_for_fixup(struct strbuf *msg)
> size_t orig_msg_len;
> int i = 1;
>
> - strbuf_addf(&buf1, "# %s\n", _(first_commit_msg_str));
> - strbuf_addf(&buf2, "# %s\n", _(skip_first_commit_msg_str));
> + strbuf_commented_addf(&buf1,
> + comment_line_char,
> + "%s\n",
> + _(first_commit_msg_str));
This is what is causing the compilation the fail. It should be
strbuf_commented_addf(&buf1, "%c $s\n", comment_char_line,
_(first_commit_msg_str));
> + strbuf_commented_addf(&buf2,
> + comment_line_char,
> + "%s\n",
> + _(skip_first_commit_msg_str));
This needs changing in the same way.
It would be nice to add a test for this. I think we can do that by adding
test_config core.commentchar :
To the beginning of the test '--skip after failed fixup cleans commit
message' in t3418-rebase-continue.sh and changing the expected message
so that it uses ':' instead of '#' for the comments.
> s = start = orig_msg = strbuf_detach(msg, &orig_msg_len);
> while (s) {
> const char *next;
> @@ -2269,8 +2275,9 @@ static int do_pick_commit(struct repository *r,
> next = parent;
> next_label = msg.parent_label;
> if (opts->commit_use_reference) {
> - strbuf_addstr(&msgbuf,
> - "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
> + strbuf_commented_addf(&msgbuf,
> + "%s",
> + "*** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
> } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) &&
> /*
> * We don't touch pre-existing repeated reverts, because
> @@ -6082,7 +6089,8 @@ static int add_decorations_to_list(const struct commit *commit,
> /* If the branch is checked out, then leave a comment instead. */
> if ((path = branch_checked_out(decoration->name))) {
> item->command = TODO_COMMENT;
> - strbuf_commented_addf(ctx->buf, comment_line_char,
> + strbuf_commented_addf(ctx->buf,
> + comment_line_char,
> "Ref %s checked out at '%s'\n",
> decoration->name, path);
This hunk is unnecessary - it is just changing the code you added in the
first patch.
Best Wishes
Phillip
^ permalink raw reply
* Re: [PATCH v2 1/2] sequencer: remove use of comment character
From: Phillip Wood @ 2023-10-31 11:43 UTC (permalink / raw)
To: Tony Tung via GitGitGadget, git; +Cc: Elijah Newren, Tony Tung
In-Reply-To: <10598a56d64f5c2b4d8d05d7e7b09a18ef254f88.1698728953.git.gitgitgadget@gmail.com>
Hi Tony
On 31/10/2023 05:09, Tony Tung via GitGitGadget wrote:
> From: Tony Tung <tonytung@merly.org>
>
> Instead of using the hardcoded `# `, use the
> user-defined comment_line_char. Adds a test
> to prevent regressions.
Well spotted and thanks for fixing this. Normally we wrap the commit
message at ~72 chars.
> Signed-off-by: Tony Tung <tonytung@merly.org>
> ---
> sequencer.c | 5 +++--
> t/t3404-rebase-interactive.sh | 39 +++++++++++++++++++++++++++++++++++
> 2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index d584cac8ed9..8c6666d5e43 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6082,8 +6082,9 @@ static int add_decorations_to_list(const struct commit *commit,
> /* If the branch is checked out, then leave a comment instead. */
> if ((path = branch_checked_out(decoration->name))) {
> item->command = TODO_COMMENT;
> - strbuf_addf(ctx->buf, "# Ref %s checked out at '%s'\n",
> - decoration->name, path);
> + strbuf_commented_addf(ctx->buf, comment_line_char,
> + "Ref %s checked out at '%s'\n",
> + decoration->name, path);
> } else {
> struct string_list_item *sti;
> item->command = TODO_UPDATE_REF;
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 8ea2bf13026..076dca87871 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1839,6 +1839,45 @@ test_expect_success '--update-refs adds label and update-ref commands' '
> )
> '
Thank you for taking the time to add a test. I think it could be
simplified though as all we really need to do is check that the expected
comment is present in the todo list. Something like (untested)
test_expect_success '--update-refs works with core.commentChar' '
git worktree add new-branch &&
test_when_finished "git worktree remove new-branch" &&
test_config core.commentchar : &&
write_script fake-editor.sh <<-\EOF &&
grep "^: Ref refs/heads/new-branch checked out at .*new-branch" "$1" &&
# no need to rebase
>"$1"
EOF
(
test_set_editor "$(pwd)/fake-editor.sh" &&
git rebase -i --update-refs HEAD^
)
'
Best Wishes
Phillip
> +test_expect_success '--update-refs works with core.commentChar' '
> + git checkout -b update-refs-with-commentchar no-conflict-branch &&
> + test_config core.commentChar : &&
> + git branch -f base HEAD~4 &&
> + git branch -f first HEAD~3 &&
> + git branch -f second HEAD~3 &&
> + git branch -f third HEAD~1 &&
> + git commit --allow-empty --fixup=third &&
> + git branch -f is-not-reordered &&
> + git commit --allow-empty --fixup=HEAD~4 &&
> + git branch -f shared-tip &&
> + git checkout update-refs &&
> + (
> + write_script fake-editor.sh <<-\EOF &&
> + grep "^[^:]" "$1"
> + exit 1
> + EOF
> + test_set_editor "$(pwd)/fake-editor.sh" &&
> +
> + cat >expect <<-EOF &&
> + pick $(git log -1 --format=%h J) J
> + fixup $(git log -1 --format=%h update-refs) fixup! J : empty
> + update-ref refs/heads/second
> + update-ref refs/heads/first
> + pick $(git log -1 --format=%h K) K
> + pick $(git log -1 --format=%h L) L
> + fixup $(git log -1 --format=%h is-not-reordered) fixup! L : empty
> + update-ref refs/heads/third
> + pick $(git log -1 --format=%h M) M
> + update-ref refs/heads/no-conflict-branch
> + update-ref refs/heads/is-not-reordered
> + update-ref refs/heads/update-refs-with-commentchar
> + EOF
> +
> + test_must_fail git rebase -i --autosquash --update-refs primary shared-tip >todo &&
> + test_cmp expect todo
> + )
> +'
> +
> test_expect_success '--update-refs adds commands with --rebase-merges' '
> git checkout -b update-refs-with-merge no-conflict-branch &&
> git branch -f base HEAD~4 &&
^ permalink raw reply
* [PATCH v2] clang-format: fix typo in comment
From: Aditya Neelamraju via GitGitGadget @ 2023-10-31 13:40 UTC (permalink / raw)
To: git; +Cc: Aditya Neelamraju, Aditya Neelamraju
In-Reply-To: <pull.1602.git.git.1698610987926.gitgitgadget@gmail.com>
From: Aditya Neelamraju <adityanv97@gmail.com>
Signed-off-by: Aditya Neelamraju <adityanv97@gmail.com>
---
clang-format: fix typo in comment for formatting binary operations.
cc: Taylor Blau me@ttaylorr.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1602%2Faneelamr%2Fclang-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1602/aneelamr/clang-v2
Pull-Request: https://github.com/git/git/pull/1602
Range-diff vs v1:
1: e3ff8aa76fe ! 1: dfb442ea338 chore: fix typo in .clang-format comment
@@ Metadata
Author: Aditya Neelamraju <adityanv97@gmail.com>
## Commit message ##
- chore: fix typo in .clang-format comment
+ clang-format: fix typo in comment
Signed-off-by: Aditya Neelamraju <adityanv97@gmail.com>
.clang-format | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/.clang-format b/.clang-format
index c592dda681f..3ed4fac753a 100644
--- a/.clang-format
+++ b/.clang-format
@@ -83,9 +83,9 @@ BinPackParameters: true
BreakBeforeBraces: Linux
# Break after operators
-# int valuve = aaaaaaaaaaaaa +
-# bbbbbb -
-# ccccccccccc;
+# int value = aaaaaaaaaaaaa +
+# bbbbbb -
+# ccccccccccc;
BreakBeforeBinaryOperators: None
BreakBeforeTernaryOperators: false
base-commit: 2e8e77cbac8ac17f94eee2087187fa1718e38b14
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v4 5/8] ci: unify setup of some environment variables
From: Victoria Dye @ 2023-10-31 17:06 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <a64799b6e25d05e5d5fc7fe3c5602b5ce256d8b9.1698742590.git.ps@pks.im>
Patrick Steinhardt wrote:
> Both GitHub Actions and Azue Pipelines set up the environment variables
s/Azue/Azure
> GIT_TEST_OPTS, GIT_PROVE_OPTS and MAKEFLAGS. And while most values are
> actually the same, the setup is completely duplicate. With the upcoming
> support for GitLab CI this duplication would only extend even further.
>
> Unify the setup of those environment variables so that only the uncommon
> parts are separated. While at it, we also perform some additional small
> improvements:
>
> - We now always pass `--state=failed,slow,save` via GIT_PROVE_OPTS.
> It doesn't hurt on platforms where we don't persist the state, so
> this further reduces boilerplate.
>
> - When running on Windows systems we set `--no-chain-lint` and
> `--no-bin-wrappers`. Interestingly though, we did so _after_
> already having exported the respective environment variables.
>
> - We stop using `export VAR=value` syntax, which is a Bashism. It's
> not quite worth it as we still use this syntax all over the place,
> but it doesn't hurt readability either.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> ci/lib.sh | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 9ffdf743903..9a9b92c05b3 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -175,11 +175,7 @@ then
> # among *all* phases)
> cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
>
> - export GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
> - export GIT_TEST_OPTS="--verbose-log -x --write-junit-xml"
> - MAKEFLAGS="$MAKEFLAGS --jobs=10"
> - test windows_nt != "$CI_OS_NAME" ||
> - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> + GIT_TEST_OPTS="--write-junit-xml"
> elif test true = "$GITHUB_ACTIONS"
> then
> CI_TYPE=github-actions
> @@ -198,17 +194,25 @@ then
>
> cache_dir="$HOME/none"
>
> - export GIT_PROVE_OPTS="--timer --jobs 10"
> - export GIT_TEST_OPTS="--verbose-log -x --github-workflow-markup"
> - MAKEFLAGS="$MAKEFLAGS --jobs=10"
> - test windows != "$CI_OS_NAME" ||
> - GIT_TEST_OPTS="--no-chain-lint --no-bin-wrappers $GIT_TEST_OPTS"
> + GIT_TEST_OPTS="--github-workflow-markup"
> else
> echo "Could not identify CI type" >&2
> env >&2
> exit 1
> fi
>
> +MAKEFLAGS="$MAKEFLAGS --jobs=10"
> +GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
> +
> +GIT_TEST_OPTS="$GIT_TEST_OPTS --verbose-log -x"
> +if test windows = "$CI_OS_NAME"
Based on the deleted lines above, I think this would need to be:
if test windows = "$CI_OS_NAME" || test windows_nt = "$CI_OS_NAME"
I believe these settings are required on all Windows builds, though, so you could
instead match on the first 7 characters of $CI_OS_NAME:
if test windows = "$(echo "$CI_OS_NAME" | cut -c1-7)"
(full disclosure: I'm not 100% confident in the correctness of that shell syntax)
> +then
> + GIT_TEST_OPTS="$GIT_TEST_OPTS --no-chain-lint --no-bin-wrappers"
> +fi
> +
> +export GIT_TEST_OPTS
> +export GIT_PROVE_OPTS
> +
> good_trees_file="$cache_dir/good-trees"
>
> mkdir -p "$cache_dir"
^ permalink raw reply
* Re: [PATCH v4 8/8] ci: add support for GitLab CI
From: Victoria Dye @ 2023-10-31 17:47 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Taylor Blau, Junio C Hamano, Phillip Wood, Oswald Buddenhagen
In-Reply-To: <f3f2c98a0dc6042b7ed5eab9c10bee4f64858f02.1698742590.git.ps@pks.im>
Patrick Steinhardt wrote:> diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh
> index 6e845283680..48cb2e735b5 100755
> --- a/ci/install-docker-dependencies.sh
> +++ b/ci/install-docker-dependencies.sh
> @@ -7,6 +7,9 @@
>
> begin_group "Install dependencies"
>
> +# Required so that apt doesn't wait for user input on certain packages.
> +export DEBIAN_FRONTEND=noninteractive
> +
> case "$jobname" in
> linux32)
> linux32 --32bit i386 sh -c '
> @@ -16,11 +19,19 @@ linux32)
> '
> ;;
> linux-musl)
> - apk add --update build-base curl-dev openssl-dev expat-dev gettext \
> + apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \
> pcre2-dev python3 musl-libintl perl-utils ncurses \
> apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \
> bash cvs gnupg perl-cgi perl-dbd-sqlite >/dev/null
> ;;
> +linux-*)
> + apt update -q &&
> + apt install -q -y sudo git make language-pack-is libsvn-perl apache2 libssl-dev \
> + libcurl4-openssl-dev libexpat-dev tcl tk gettext zlib1g-dev \
> + perl-modules liberror-perl libauthen-sasl-perl libemail-valid-perl \
> + libdbd-sqlite3-perl libio-socket-ssl-perl libnet-smtp-ssl-perl ${CC_PACKAGE:-${CC:-gcc}} \
> + apache2 cvs cvsps gnupg libcgi-pm-perl subversion
> + ;;
> pedantic)
> dnf -yq update >/dev/null &&
> dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null
...
> diff --git a/ci/lib.sh b/ci/lib.sh
> index e14b1029fad..6e3d64004ec 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -208,6 +224,7 @@ then
> cache_dir="$HOME/test-cache/$SYSTEM_PHASENAME"
>
> GIT_TEST_OPTS="--write-junit-xml"
> + JOBS=10
> elif test true = "$GITHUB_ACTIONS"
> then
> CI_TYPE=github-actions
...
> -MAKEFLAGS="$MAKEFLAGS --jobs=10"
> -GIT_PROVE_OPTS="--timer --jobs 10 --state=failed,slow,save"
> +MAKEFLAGS="$MAKEFLAGS --jobs=${JOBS}"
> +GIT_PROVE_OPTS="--timer --jobs ${JOBS} --state=failed,slow,save"
>
Organizationally, this commit seems to be doing two things at once:
- Adding GitLab-specific CI setup (either in the new .gitlab-ci.yml or in
conditions gated on "gitlab-ci").
- Updating the common CI scripts with things that are needed for GitLab CI,
but aren't conditioned on it (i.e. the patch excerpts I've included
above).
I'd prefer these being separated into two patches, mainly to isolate "things
that affect all CI" from "things that affect only GitLab CI". This is
ultimately a pretty minor nit, though; if you're not planning on re-rolling
(or just disagree with what I'm suggesting :) ), I'm okay with leaving it
as-is.
Otherwise, I can't comment on the correctness of the GitLab CI definition (I
assume you've tested it anyway), but AFAICT the changes above shouldn't break
GitHub CI.
^ permalink raw reply
* [bug] 2.39.0: error in help for ls-remote
From: Jeremy Hetzler @ 2023-10-31 18:11 UTC (permalink / raw)
To: git
In-Reply-To: <CAOh4nmk2KZBTuW9qn_ZgDY3yLRZ6NgGOWuBMLRRm1sU=pdmRoQ@mail.gmail.com>
All,
The short help for ls-remote advertises that '-h' is short for '--heads':
> usage: git ls-remote [--heads] [--tags] [--refs] [--upload-pack=<exec>]
> [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]
> [--symref] [<repository> [<refs>...]]
>
>
> -q, --quiet do not print remote URL
> --upload-pack <exec> path of git-upload-pack on the remote host
> -t, --tags limit to tags
> -h, --heads limit to heads
> --refs do not show peeled tags
> --get-url take url.<base>.insteadOf into account
> --sort <key> field name to sort on
> --exit-code exit with exit code 2 if no matching refs are found
> --symref show underlying ref in addition to the object pointed by it
> -o, --server-option <server-specific>
> option to transmit
However, 'git ls-remote -h' instead prints the help. So perhaps the
help message should be revised.
git version 2.39.0
Thanks,
Jeremy
^ 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