Re: [PATCH v4] clone: simplify string handling in guess_dir_name()
On Tue, Aug 4, 2015 at 6:34 AM, Lukas Fleischer wrote: > I am currently on vacation and cannot bisect or debug this but I am > pretty confident that this patch changes the behaviour of directory name > guessing. With Git 2.4.6, cloning http://foo.bar/foo.git/ results in a > directory named foo and with Git 2.5.0, the resulting directory is > called foo.git. > > Note how the end variable is decreased when the repository name ends > with a slash but that isn't taken into account when simply using > strip_suffix() later... > > Is this intended? I did not intend this change in behavior, and I can confirm that reverting my patch restores the original behavior. Thanks for bringing this to my attention, I'll work on a patch. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] clone: use computed length in guess_dir_name
On 8/5/2015 10:39, Jeff King wrote: > Commit 7e837c6 (clone: simplify string handling in > guess_dir_name(), 2015-07-09) changed clone to use > strip_suffix instead of hand-rolled pointer manipulation. > However, strip_suffix will strip from the end of a > NUL-terminated string, and we may have already stripped some > characters (like directory separators, or "/.git"). This > leads to commands like: > >git clone host:foo.git/ > > failing to strip the ".git". Thanks a lot Peff for fixing my bugs, I should have known that you'll be able to come up with something much sooner than I would ;-) This all looks good to me! Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v1] Add Travis CI support
On 25.09.2015 05:14, Dennis Kaarsemaker wrote: My idea is that the owner of "https://github.com/git/git"; enables this account for Travis (it's free!). Then we would automatically get the test state for all official branches. The last time I heard about this "it's free" thing, I thought I heard that it wants write access to the repository. It does not need write access to the git data, only to auxiliary GitHub data: commit status and deployment status (where it can put "this commit failed tests"), repository hooks (to set up build triggers), team membership (ro) and email addresses (ro). Also, as Roberto explained at [1], "If you set up the webhook yourself, you don't need to grant the [repository hooks] permissions". BTW, there's already an attempt at creating a .travis.yml file at [2]. [1] https://github.com/rtyley/submitgit/issues/16#issuecomment-120119634 [2] https://github.com/git/git/pull/154 -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/17] engine.pl: provide more debug print statements
On 25.06.2015 02:03, Philip Oakley wrote:
--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -41,6 +41,7 @@ EOM
# Parse command-line options
while (@ARGV) {
my $arg = shift @ARGV;
+ #print "Arg: $arg \n";
if ("$arg" eq "-h" || "$arg" eq "--help" || "$arg" eq "-?") {
showUsage();
exit(0);
@@ -129,6 +130,7 @@ sub parseMakeOutput
print "Parsing GNU Make output to figure out build structure...\n";
my $line = 0;
while (my $text = shift @makedry) {
+ #print "Make: $text\n"; # show the makedry line
Please never commit code that's been commented out. Also see
http://dev.solita.fi/2013/07/04/whats-in-a-good-commit.html
;-)
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Visualizing merge conflicts after the fact (using kdiff3)
On 16.06.2015 03:17, Eric Raible wrote: > So naturally I want to check each of them for correctness. Sorry for joining this thread so late, I only come to know about it from the draft of the upcoming Git Rev News 5 [1]. A while ago Robin Green was asking a very similar question on StackOverflow [2], and I came up with a script called "git-show-merge-resolution.sh" [3]. Maybe that's something you're interested in, too. [1] https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-5.md#support [2] stackoverflow.com/questions/24958182/kdiff3-to-code-review-merge-commit/24958228 [3] https://github.com/sschuberth/dev-scripts/blob/master/git/git-show-merge-resolution.sh -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] clone: Make use of the strip_suffix() helper method
Signed-off-by: Sebastian Schuberth
---
builtin/clone.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 00535d0..d35b2b9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -147,6 +147,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
const char *end = repo + strlen(repo), *start;
+ size_t len;
char *dir;
/*
@@ -174,19 +175,17 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
* Strip .{bundle,git}.
*/
if (is_bundle) {
- if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
- end -= 7;
+ strip_suffix(start, ".bundle", &len);
} else {
- if (end - start > 4 && !strncmp(end - 4, ".git", 4))
- end -= 4;
+ strip_suffix(start, ".git", &len);
}
if (is_bare) {
struct strbuf result = STRBUF_INIT;
- strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
+ strbuf_addf(&result, "%.*s.git", len, start);
dir = strbuf_detach(&result, NULL);
} else
- dir = xstrndup(start, end - start);
+ dir = xstrndup(start, len);
/*
* Replace sequences of 'control' characters and whitespace
* with one ascii space, remove leading and trailing spaces.
---
https://github.com/git/git/pull/160
Re: [PATCH] clone: Make use of the strip_suffix() helper method
On Thu, Jul 9, 2015 at 7:00 PM, Jeff King wrote: > If you wanted to get really fancy, I think you could put a ternary > operator in the middle of the strip_suffix call. That makes it clear > that "len" is set in all code paths, but I think some people find > ternary operators unreadable. :) I like the idea about the ternary operator, will do. > This one can also be simplified using xstrfmt to: Nice, will also do. > Do we still need to cast "len" to an int to use it with "%.*" (it is > defined by the standard as an int, not a size_t)? I think we're more on the safe side by keeping the cast, so I'll do that, too. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] clone: Simplify string handling in guess_dir_name()
Signed-off-by: Sebastian Schuberth
---
builtin/clone.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 00535d0..afdc004 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -147,6 +147,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
const char *end = repo + strlen(repo), *start;
+ size_t len;
char *dir;
/*
@@ -173,20 +174,9 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
/*
* Strip .{bundle,git}.
*/
- if (is_bundle) {
- if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
- end -= 7;
- } else {
- if (end - start > 4 && !strncmp(end - 4, ".git", 4))
- end -= 4;
- }
+ strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);
- if (is_bare) {
- struct strbuf result = STRBUF_INIT;
- strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
- dir = strbuf_detach(&result, NULL);
- } else
- dir = xstrndup(start, end - start);
+ dir = is_bare ? xstrfmt("%.*s.git", (int)len, start) : xstrndup(start,
len);
/*
* Replace sequences of 'control' characters and whitespace
* with one ascii space, remove leading and trailing spaces.
---
https://github.com/git/git/pull/160
Re: [PATCH v2] clone: Simplify string handling in guess_dir_name()
On Thu, Jul 9, 2015 at 8:05 PM, Junio C Hamano wrote:
>> Subject: Re: [PATCH v2] clone: Simplify string handling in guess_dir_name()
>
> We seem not to capitalize the first word on the subject line.
Will change that.
>> Content-Type: multipart/mixed;
>> boundary="=_Part_8_836493213.1436462597065"
>
> Please don't.
This seems to come from submitgit, I've filed an issue about it:
https://github.com/rtyley/submitgit/issues/17
What content type(s) would you accept? Only text/plain?
>> - if (is_bare) {
>> - struct strbuf result = STRBUF_INIT;
>> - strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
>> - dir = strbuf_detach(&result, NULL);
>> - } else
>> - dir = xstrndup(start, end - start);
>> + dir = is_bare ? xstrfmt("%.*s.git", (int)len, start) : xstrndup(start,
>> len);
>
> This however I had to read twice. I'd say
>
> if (is_bare)
> dir = xstrfmt(...);
> else
> dir = xstrndup(...);
>
> is much easier to read.
That's what I had locally before. Will revert to that.
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] clone: Simplify string handling in guess_dir_name()
Signed-off-by: Sebastian Schuberth
---
builtin/clone.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 00535d0..ebcb849 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -147,6 +147,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
const char *end = repo + strlen(repo), *start;
+ size_t len;
char *dir;
/*
@@ -173,19 +174,11 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
/*
* Strip .{bundle,git}.
*/
- if (is_bundle) {
- if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
- end -= 7;
- } else {
- if (end - start > 4 && !strncmp(end - 4, ".git", 4))
- end -= 4;
- }
+ strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);
- if (is_bare) {
- struct strbuf result = STRBUF_INIT;
- strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
- dir = strbuf_detach(&result, NULL);
- } else
+ if (is_bare)
+ dir = xstrfmt("%.*s.git", (int)len, start);
+ else
dir = xstrndup(start, end - start);
/*
* Replace sequences of 'control' characters and whitespace
---
https://github.com/git/git/pull/160
[PATCH v4] clone: simplify string handling in guess_dir_name()
Signed-off-by: Sebastian Schuberth
---
builtin/clone.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 00535d0..ebcb849 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -147,6 +147,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
const char *end = repo + strlen(repo), *start;
+ size_t len;
char *dir;
/*
@@ -173,19 +174,11 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
/*
* Strip .{bundle,git}.
*/
- if (is_bundle) {
- if (end - start > 7 && !strncmp(end - 7, ".bundle", 7))
- end -= 7;
- } else {
- if (end - start > 4 && !strncmp(end - 4, ".git", 4))
- end -= 4;
- }
+ strip_suffix(start, is_bundle ? ".bundle" : ".git" , &len);
- if (is_bare) {
- struct strbuf result = STRBUF_INIT;
- strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
- dir = strbuf_detach(&result, NULL);
- } else
+ if (is_bare)
+ dir = xstrfmt("%.*s.git", (int)len, start);
+ else
dir = xstrndup(start, end - start);
/*
* Replace sequences of 'control' characters and whitespace
---
https://github.com/git/git/pull/160
Re: [PATCH v4] clone: simplify string handling in guess_dir_name()
On Thu, Jul 9, 2015 at 11:21 PM, Junio C Hamano wrote:
>> - if (is_bare) {
>> - struct strbuf result = STRBUF_INIT;
>> - strbuf_addf(&result, "%.*s.git", (int)(end - start), start);
>> - dir = strbuf_detach(&result, NULL);
>> - } else
>> + if (is_bare)
>> + dir = xstrfmt("%.*s.git", (int)len, start);
>> + else
>> dir = xstrndup(start, end - start);
>
> The last one needs to be adjusted with s/end - start/len/. The
> last-minute rewrite without testing shows; your first two patches
> correctly used "len" ;-)
Doh, you're right, sorry for that.
> No need to resend. Will locally tweak before queuing.
Thanks!
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] ident: support per-path configs by matching the path against a pattern
Support per-path identities by configuring Git like
$ git config user..email
e.g.
$ git config user.github.email sschuberth+git...@gmail.com
In this example, the middle "github" pattern is searched for
case-insensitively in the absolute path name to the current git work tree.
If there is a match the configured email address is used instead of what
otherwise would be the default email address.
Note that repository-local identities still take precedence over global /
system ones even if the pattern configured in the global / system identity
matches the path to the local work tree.
This change avoids the need to use external tools like [1].
TODO: Once the community agrees that this is a feature worth having, add
tests and adjust the docs.
[1] https://github.com/prydonius/karn
Signed-off-by: Sebastian Schuberth
---
ident.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/ident.c b/ident.c
index 5ff1aad..2429ed8 100644
--- a/ident.c
+++ b/ident.c
@@ -390,7 +390,21 @@ int author_ident_sufficiently_given(void)
int git_ident_config(const char *var, const char *value, void *data)
{
- if (!strcmp(var, "user.name")) {
+ /* the caller has already checked that var starts with "user." */
+
+ const char *first_period = strchr(var, '.');
+ const char *last_period = strrchr(var, '.');
+
+ if (first_period < last_period) {
+ ++first_period;
+ char *pattern = xstrndup(first_period, last_period -
first_period);
+ const char *match = strcasestr(get_git_work_tree(), pattern);
+ free(pattern);
+ if (!match)
+ return 0;
+ }
+
+ if (ends_with(var, ".name")) {
if (!value)
return config_error_nonbool(var);
strbuf_reset(&git_default_name);
@@ -400,7 +414,7 @@ int git_ident_config(const char *var, const char *value,
void *data)
return 0;
}
- if (!strcmp(var, "user.email")) {
+ if (ends_with(var, ".email")) {
if (!value)
return config_error_nonbool(var);
strbuf_reset(&git_default_email);
---
https://github.com/git/git/pull/161
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Issues with git diff-tree --quiet --ignore-space-change
Hi, I believe there's something wrong with diff-tree's --ignore-space-change option in conjunction with --quiet. In Git's repo, I get $ git --version git version 2.4.5 $ git diff-tree --quiet --ignore-space-change c925fe23684455735c3bb1903803643a24a58d8f ; echo $? c925fe23684455735c3bb1903803643a24a58d8f 0 First question is, why do I see the SHA1 printed to the output, esp. as I specified --quiet? Secondly, the exit code of 0 indicates there's no diff. However, using $ git diff-tree --patch --ignore-space-change c925fe23684455735c3bb1903803643a24a58d8f I see the diff (which does not only consist of whitespaces). If I omit --ignore-space-change the return value is correct: $ git diff-tree --quiet c925fe23684455735c3bb1903803643a24a58d8f ; echo $? c925fe23684455735c3bb1903803643a24a58d8f 1 Am I missing something, or are these bugs? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] diff-tree: do not show the sha1 of the given head with --quiet
"--quite" is documented to "Disable all output of the program". Yet
calling diff-tree with a single commit like
$ git diff-tree --quiet c925fe2
was logging
c925fe23684455735c3bb1903803643a24a58d8f
to the console despite "--quite" being given. This is inconsistent with
both the docs and the behavior if more than a single commit is passed to
diff-tree. Moreover, the output of that single line seems to be documented
nowhere except in a comment for a test. Fix this inconsistency by making
diff-tree really output nothing if "--quiet" is given and fix the test
accordingly.
Signed-off-by: Sebastian Schuberth
---
log-tree.c| 3 ++-
t/t4035-diff-quiet.sh | 3 +--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 01beb11..3c98234 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -741,7 +741,8 @@ int log_tree_diff_flush(struct rev_info *opt)
}
if (opt->loginfo && !opt->no_commit_id) {
- show_log(opt);
+ if (!DIFF_OPT_TST(&opt->diffopt, QUICK))
+ show_log(opt);
if ((opt->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT) &&
opt->verbose_header &&
opt->commit_format != CMIT_FMT_ONELINE &&
diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh
index 461f4bb..9a8225f 100755
--- a/t/t4035-diff-quiet.sh
+++ b/t/t4035-diff-quiet.sh
@@ -40,11 +40,10 @@ test_expect_success 'git diff-tree HEAD^ HEAD -- b' '
test_expect_code 1 git diff-tree --quiet HEAD^ HEAD -- b >cnt &&
test_line_count = 0 cnt
'
-# this diff outputs one line: sha1 of the given head
test_expect_success 'echo HEAD | git diff-tree --stdin' '
echo $(git rev-parse HEAD) |
test_expect_code 1 git diff-tree --quiet --stdin >cnt &&
- test_line_count = 1 cnt
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-tree HEAD HEAD' '
test_expect_code 0 git diff-tree --quiet HEAD HEAD >cnt &&
---
https://github.com/git/git/pull/163
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] diff-tree: do not show the sha1 of the given head with --quiet
"--quiet" is documented to "Disable all output of the program". Yet
calling diff-tree with a single commit like
$ git diff-tree --quiet c925fe2
was logging
c925fe23684455735c3bb1903803643a24a58d8f
to the console despite "--quiet" being given. This is inconsistent with
both the docs and the behavior if more than a single commit is passed to
diff-tree. Moreover, the output of that single line seems to be documented
nowhere except in a comment for a test. Fix this inconsistency by making
diff-tree really output nothing if "--quiet" is given and fix the test
accordingly.
Signed-off-by: Sebastian Schuberth
---
log-tree.c| 3 ++-
t/t4035-diff-quiet.sh | 3 +--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 01beb11..3c98234 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -741,7 +741,8 @@ int log_tree_diff_flush(struct rev_info *opt)
}
if (opt->loginfo && !opt->no_commit_id) {
- show_log(opt);
+ if (!DIFF_OPT_TST(&opt->diffopt, QUICK))
+ show_log(opt);
if ((opt->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT) &&
opt->verbose_header &&
opt->commit_format != CMIT_FMT_ONELINE &&
diff --git a/t/t4035-diff-quiet.sh b/t/t4035-diff-quiet.sh
index 461f4bb..9a8225f 100755
--- a/t/t4035-diff-quiet.sh
+++ b/t/t4035-diff-quiet.sh
@@ -40,11 +40,10 @@ test_expect_success 'git diff-tree HEAD^ HEAD -- b' '
test_expect_code 1 git diff-tree --quiet HEAD^ HEAD -- b >cnt &&
test_line_count = 0 cnt
'
-# this diff outputs one line: sha1 of the given head
test_expect_success 'echo HEAD | git diff-tree --stdin' '
echo $(git rev-parse HEAD) |
test_expect_code 1 git diff-tree --quiet --stdin >cnt &&
- test_line_count = 1 cnt
+ test_line_count = 0 cnt
'
test_expect_success 'git diff-tree HEAD HEAD' '
test_expect_code 0 git diff-tree --quiet HEAD HEAD >cnt &&
---
https://github.com/git/git/pull/163
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Wed, Jul 22, 2015 at 10:32 PM, Junio C Hamano wrote: >> "--quite" is documented to "Disable all output of the program". Yet >> calling diff-tree with a single commit like >> >> $ git diff-tree --quiet c925fe2 >> >> was logging >> >> c925fe23684455735c3bb1903803643a24a58d8f > > At this point, unfortunately I think we need to call that a > documentation bug. The "output" it refers to is output from the > "diff" portion, not the "poor-man's log" portion, of the program, > where diff-tree was the workhorse behind scripted "git log" that > gave the commit object name as the preamble for each commit it > shows information about. Well, from a user's perspective it does not matter which part of the internal implementation of diff-tree is responsible for printing that single line, a user would just expect "--quiet" to really mean "quiet". As for almost any bug, we could turn it into a feature by "fixing" the docs and claiming it's documented behavior. To me the question simply is whether it makes sense for "--quiet" to not be quiet, and I think it does not make sense. If you run diff-tree this way there is no added value in the given output. My use-case (also see [1]) is that I wanted to checked whether some given commits change nothing but whitespace. So I did if git diff-tree --quiet --ignore-space-change $commit; then echo "$commit only changes whitespace." fi just to see those SHA1s being printed to the console. I probably could instead do if git diff-tree --exit-code --ignore-space-change $commit > /dev/null 2>&1; then echo "$commit only changes whitespace." fi but that defeats the purpose of having "--quiet" in the first place. [1] http://article.gmane.org/gmane.comp.version-control.git/273975 -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git doesn't detect change, if file modification time is restored to original one
On 7/23/2015 9:29, Konrád Lőrinczi wrote: > Interesting, that git status doesn't show replaced changes, if the > mtime is same as original. See the somewhat related FAQ entry at [1] and also the lengthy discussion at [2] about a similar issue. That said, deleting the .git/index file should make these files appear as modified. [1] https://git.wiki.kernel.org/index.php/Git_FAQ#Why_isn.27t_Git_preserving_modification_time_on_files.3F [2] https://github.com/msysgit/git/issues/312 Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 8:08 PM, Jeff King wrote: > mode. Actually asking for a two-endpoint tree diff: > > git diff-tree --quiet --ignore-space-change $commit^ $commit > > will do what you want. Yes, I know, thanks. But I deliberately wanted to specify only a single commit as an optimization, hoping that it would be slightly faster than computing a commit range. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 7:06 PM, Junio C Hamano wrote: > Existing scripts by definition would not be using a new option you > will invent that used not to be a valid one. So that would be one > way that you can shorten your script without breaking other people. True. If it was only for shortening my script, I still could do "> /dev/null 2>&1" which is just as short (or long) as a newly introduced "--really-quiet" option. But I'm also concerned about consistency and making options do what they sound they would do. > In "git rev-list ... | git diff-tree --stdin" output, the commit > object name is absolutely necessary, with or without --quiet, as it Why is printing the object name also necessary with "--quiet"? I'd argue that any script that uses diff-tree that way uses --stdin without --quiet, just like you do in your example, so suppressing the object name if "--quiet" is given probably would not break as many scripts as you think. > But we do not live in an ideal world. True, but we should never stop striving after making it one :-) -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 9:39 PM, Junio C Hamano wrote: > I haven't dug into why that happens, but possible ways to fix that > are to make "--quiet" output all (making it consistent with "-s") or > no (making the command totally silent) output at all ;-). Exactly, and I chose the latter to add some value to --quiet instead of making it an alias for -s. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions on passing --depth to git-clone vs. git-fetch
On 1/6/2016 13:41, Duy Nguyen wrote: I think the culprit is the "git remote add" line. "git clone --depth" by default will fetch only one branch (aka --single-branch option in git-clone). But I suspect when you add a new remote, the default Now that you mention it I see this being documented as part of --single-branch instead of --depth, which I think is confusing. I'll send a patch. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] docs: clarify that passing --depth to git-clone implies --single-branch
It is confusing to document how --depth behaves as part of the --single-branch docs. Better move that part to the --depth docs, saying that it implies --single-branch by default. Signed-off-by: Sebastian Schuberth --- Documentation/git-clone.txt | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index 6bf000d..943de8b 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -190,15 +190,14 @@ objects from the source repository into a pack in the cloned repository. --depth :: Create a 'shallow' clone with a history truncated to the - specified number of revisions. + specified number of revisions. Implies `--single-branch` unless + `--no-single-branch` is given to fetch the histories near the + tips of all branches. --[no-]single-branch:: Clone only the history leading to the tip of a single branch, either specified by the `--branch` option or the primary - branch remote's `HEAD` points at. When creating a shallow - clone with the `--depth` option, this is the default, unless - `--no-single-branch` is given to fetch the histories near the - tips of all branches. + branch remote's `HEAD` points at. Further fetches into the resulting repository will only update the remote-tracking branch for the branch this option was used for the initial cloning. If the HEAD at the remote did not point at any -- 2.7.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] contrib/git-candidate: Add README
On 10.11.2015 13:56, Richard Ipsum wrote: +Existing tools such as Github's pull-requests and Gerrit are already +in wide use, why bother with something new? + +We are concerned that whilst git is a distributed version control +system the systems used to store comments and reviews for content +under version control are usually centralised, I think it's a bit unjust to unconditionally mention Gerrit in this context as you seem to imply that Gerrit does not store *any* review data in Git. Even without Dave's upcoming notedb, Gerrit already stores refs/changes in Git, and with the reviewnotes plugin [1] also the outcome of a review in refs/notes/review. [1] https://gerrit.googlesource.com/plugins/reviewnotes/+/refs/heads/master/src/main/resources/Documentation/refs-notes-review.md -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Submodule Groups
On 20.01.2016 04:34, Stefan Beller wrote: > So you could have a .gitmodules file such as: > > [submodule "gcc"] > path = gcc > url = git://... > groups = default > groups = devel On the quick I was unable to find the rationale why entries are now stored as separated lines compared to v1. I liked the comma-separated approach better as it's more compact. Anyway, if it's only one group per line, I'd find it more fitting to call the entry "group" instead of "groups" as it will always refer to a single group only. Also that would better match the "--group" command line option naming for "submodule add". However, if I'd read the single line "group = default" in a .gitmodules file, it wouldn't be immediately clear to me that "group" can appear multiple times per submodule. "groups = default" would me more hinting is this regard because the plural is used, but without reading the docs I'd assume multiple groups would be specified like "groups = default,devel". Long story short, my personal favorite still would be [submodule "gcc"] groups = default,devel followed by [submodule "gcc"] group = default group = devel -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Submodule Groups
On Thu, Jan 21, 2016 at 10:56 PM, Stefan Beller wrote: >>> [submodule "gcc"] >>> path = gcc >>> url = git://... >>> groups = default >>> groups = devel >> >> On the quick I was unable to find the rationale why entries are now stored >> as separated lines compared to v1. I liked the comma-separated approach >> better as it's more compact. > > IIUC the line oriented way is preferred as it is our standard. Do we > have any other options stored as a comma separated list? Out of my head I cannot think of any. But that shouldn't mean we cannot introduce such comma separated list if it makes sense. > Makes sense to use singular then. However per discussion with Junio in > [PATCH 3/4] submodule update: Initialize all group-selected submodules > by default, we want to not name it "group", as it's unclear what a group is > supposed to mean. What does a group do? which operations are supported? How about calling it "label" instead of "group"? IMO with the word "label" it's more clean that a single submodule can have multiple labels, as the concept of labels is familiar to the user already from applications like Firefox (bookmarks), Google Mail, Mac OS X Finder (files) etc. > Instead of having a submodule -> set assignment, we could do it the > other way round: > > [submodule "gcc"] > ... > > [submodule-set "default"] > submodule = gcc > submodule = foo > submodule = by/path/* In your example you're now introducing "set" as a new term. Shouldn't this better be "submodule-group" then? I actually like this idea quite a bit as it completely solves the problem about being clear that a submodule can belong to mutiple groups. > but I'd assume this is less useful for the user. How often does a user ask: > "How many/Which submodules are in $GROUP" as opposed to "What about > submodule foo, > is that part of group $GROUP?" True, but for answering that question a user would not look at .gitmodules, but run a command, and the implementation of that command would completely hide that complexity from the user. > As asked above, how many comma separated things do we have in git configs? > I'd really not want to add more mental complexity to Git. As far as I I don't think it can get much worse anyway ;-) > remember we have > rather double configs than one long line separated somehow. > (The only thing that comes to mind is multiple remote urls for pushing) I believe so, too. But I'd see the introduction of comma-separated values as an exit-strategy to that. More settings could make use of that in the future, then. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] submodule-config: keep labels around
On Sat, Jan 23, 2016 at 1:31 AM, Stefan Beller wrote: > We need the submodule groups in a later patch. The commit message should now say "labels", too, I guess. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-gui--askpass: generalize the window title
From: Sebastian Schuberth
git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to have a window title of
"OpenSSH". So generalize the title so that it also says which parent
process, i.e. Git, requires authentication.
Signed-off-by: Sebastian Schuberth
---
git-gui/git-gui--askpass | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..1e5c325 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
}
-wm title . "OpenSSH"
+wm title . "Git Authentication"
tk::PlaceWindow .
vwait rc
exit $rc
--
https://github.com/git/git/pull/195
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-completion.bash: always swallow error output of for-each-ref
This avoids output like
warning: ignoring broken ref refs/remotes/origin/HEAD
while completing branch names.
Signed-off-by: Sebastian Schuberth
---
contrib/completion/git-completion.bash | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 15ebba5..7c0549d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -317,7 +317,7 @@ __git_heads ()
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
- refs/heads
+ refs/heads 2>/dev/null
return
fi
}
@@ -327,7 +327,7 @@ __git_tags ()
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
- refs/tags
+ refs/tags 2>/dev/null
return
fi
}
@@ -355,14 +355,14 @@ __git_refs ()
;;
esac
git --git-dir="$dir" for-each-ref --format="%($format)" \
- $refs
+ $refs 2>/dev/null
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the
completion word
# but only output if the branch name is unique
local ref entry
git --git-dir="$dir" for-each-ref --shell
--format="ref=%(refname:short)" \
- "refs/remotes/" | \
+ "refs/remotes/" 2>/dev/null | \
while read -r entry; do
eval "$entry"
ref="${ref#*/}"
@@ -1835,7 +1835,7 @@ _git_config ()
remote="${remote%.push}"
__gitcomp_nl "$(git --git-dir="$(__gitdir)" \
for-each-ref --format='%(refname):%(refname)' \
- refs/heads)"
+ refs/heads 2>/dev/null)"
return
;;
pull.twohead|pull.octopus)
--
2.7.0.windows.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] config: add '--sources' option to print the source of a config value
On 2/5/2016 9:42, larsxschnei...@gmail.com wrote: Teach 'git config' the '--sources' option to print the source configuration file for every printed value. Yay, not being able to see where a config setting originates from has bothered me in the past, too. So thanks for working on this. However, the naming of the '--sources' option sounds a bit misleading to me. It has nothing to do with source code. So maybe better name it '--origin', or even more verbose '--show-origin' or '--show-filename'? Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] config: add '--sources' option to print the source of a config value
On 2/5/2016 12:20, Jeff King wrote: Hmm. I had originally envisioned this only being used with "--list", but I guess it makes sense to say "--sources --get" to show where the value for a particular option is coming from. Being able to use "--sources --get" is a feature that I'd definitely like to see, too. I'm not sure returning here is the best idea. We won't have a config filename if we are reading from "-c", but if we return early from this function, it parses differently than every other line. E.g., with your patch, if I do: git config -c foo.bar=true config --sources --list I'll get: /home/peff/.gitconfig user.name=Jeff King /home/peff/.gitconfig user.email=p...@peff.net ...etc... foo.bar=true If somebody is parsing this as a tab-delimited list, then instead of the source field for that line being empty, it is missing (and it looks like "foo.bar=true" is the source file). I think it would be more friendly to consumers of the output to have a blank (i.e., set "fn" to the empty string and continue in the function). Or to come up with a special string to denote config values specified on the command line. Maybe somehting like foo.bar=true I acknowledge that "" would be a valid filename on some filesystems, but I think the risk is rather low that someone would actually be using that name for a Git config file. Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] config: add '--sources' option to print the source of a config value
On Sun, Feb 7, 2016 at 8:28 PM, Lars Schneider wrote: >>> However, the naming of the '--sources' option sounds a bit misleading to me. >>> It has nothing to do with source code. So maybe better name it '--origin', >>> or even more verbose '--show-origin' or '--show-filename'? >> >> I think he inherited the "--sources" name from me. :) I agree it could >> be better. I think "--show-filename" is not right as there are >> non-filename cases. Just "--origin" sounds funny to me, perhaps because >> of git's normal use of the word "origin". >> >> I like "--show-origin" the best of the ones suggested. > > I understand your reasoning and I agree that "--show-origin" is better than > "--sources". However, I think just the word "origin" could be misleading in > this context because people associate it with Git remotes. How about > "--show-config-origin" then? Or would that be too verbose? Well, "origin" just happens to be the name of the default remote. AFAIK all options that deal with remotes have "remote" and not "origin" in their name, so I think the risk of confusion is rather low. But I'd be fine with "--show-config-origin", too. Although it's verbose, it's probably not used very often, so personally I could live with typing the extra character. Esp. if you add Bash completion support for it :-) -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] config: add '--sources' option to print the source of a config value
On 2/5/2016 14:58, Jeff King wrote: Yeah, I agree it's unlikely. And the output is already ambiguous, as the first field could be a blob (though I guess the caller knows if they passed "--blob" or not). If we really wanted an unambiguous output, we could have something like "file:...", "blob:...", etc. But that's a bit less readable for humans, and I don't think solves any real-world problems. So I think it would be OK to use "" here, as long as the token is documented. Thinking about it again, I actually do like Peff's prefix solution better. It would solve the real-world problem that my proposed "line>" marker could in fact be a file name. Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] config: add '--sources' option to print the source of a config value
On Wed, Feb 10, 2016 at 1:54 PM, Jeff King wrote: >> +--sources:: >> + Augment the output of all queried config options with the >> + source type (file, stdin, blob, cmd) and the actual source >> + (config file path, ref, or blob id if applicable). > > I think something like "cmdline" might be more descriptive than "cmd". I agree "cmdline" is better than "cmd". > So the format here is like: > > file\t\t > blob\t\t > stdin\t\t > cmd\t\t > > where two of the prefixes have nothing in the second slot. I expected > something more like: > > file:\t > blob:\t > stdin\t > cmd\t > > with a single delimited slot for the source, which can then be broken > down further if desired. I can't think of any reason to prefer one over > the other rather than personal preference, though. They can both be > parsed unambiguously. I also would have expected sopme like the latter, except that I'd also expect a colon after "stdin" and "cmd" (or "cmdline", as said above). I.e. the colon should be part of the prefix to mark it as such. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] config: add '--sources' option to print the source of a config value
On Wed, Feb 10, 2016 at 1:47 PM, Ramsay Jones wrote: >> Sebastian suggested "--show-origin" as a better option name over "--sources". >> I still believe "--sources" might be slightly better as I fear that users >> could >> somehow related "origin" to "remote" kind of configs. However, I am happy to >> change that if a majority prefers "--show-origin". > > > As I have said before, I'm not very good at naming things, but ... > > Of the two, I *slightly* prefer --show-origin, since I don't think > there will be any confusion. However, I think --source may be OK too > (for some reason it sounds better than the plural). Another idea > may be --show-source. ;-) > > I agree that using --source sounds better than --sources, as the latter sounds even more like "source code". Here's another idea: How about --declaration or --show-declaration? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] config: add '--sources' option to print the source of a config value
On Wed, Feb 10, 2016 at 4:40 PM, Jeff King wrote: >> > file:\t >> > blob:\t >> > stdin\t >> > cmd\t >> > >> > with a single delimited slot for the source, which can then be broken >> > down further if desired. I can't think of any reason to prefer one over >> > the other rather than personal preference, though. They can both be >> > parsed unambiguously. >> >> I also would have expected sopme like the latter, except that I'd also >> expect a colon after "stdin" and "cmd" (or "cmdline", as said above). >> I.e. the colon should be part of the prefix to mark it as such. > > Yeah, I waffled on that. Having a colon means you can definitely parse > to the first ":" without looking at what the prefix is. But if you don't > know what the prefix is, I don't know what good that does you. IOW, I'd IMO that's asking the wrong question. The question should not be "what good does it do if we add the colons also there", but "what justification do we have to introduce an inconsistency by not adding them". > That's perl, but I think most languages make prefix-parsing like that > easy. I dunno. I doubt it matters all that much, and we are deep into > personal preference. There's already plenty to bikeshed on the option > name :) I agree the option wording mostly is personal preference. On the other hand, I find discussions like these often prematurely waved aside as bikeshedding, just because e.g. Perl can parse the one as good as the other. But it's not about Perl, it's about humans. IMO Git has historically made many mistakes by not caring enough about consistency in docs, command and command line option naming, so I don't see it as wasted time to discuss about things like this. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref
On 04.02.2016 11:34, Sebastian Schuberth wrote: This avoids output like warning: ignoring broken ref refs/remotes/origin/HEAD while completing branch names. Signed-off-by: Sebastian Schuberth The discussion got a bit off the point with the "short" vs. "strip=2" stuff, but I guess the patch itself if good to apply? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui--askpass: generalize the window title
On 01.02.2016 13:11, Sebastian Schuberth wrote: git-gui--askpass is not only used for SSH authentication, but also for HTTPS. In that context it is confusing to have a window title of "OpenSSH". So generalize the title so that it also says which parent process, i.e. Git, requires authentication. Signed-off-by: Sebastian Schuberth I haven't seen this being picked up so far. Any comments? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] config: add '--sources' option to print the source of a config value
On Sat, Feb 13, 2016 at 3:43 PM, Mike Rappazzo wrote: >> I renamed the flag from "--source" to "--show-origin" as I got the impression >> that Sebastian, Peff and Ramsay seem to be all OK with "--show-origin". > > I know that I am late to the party here, but why not make the option > `--verbose` or `-v`? `git config` doesn't have that now, and this > seems like a logical thing to include as verbose data. I would `--verbose` would be fine with me. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-completion.bash: always swallow error output of for-each-ref
On Wed, Feb 24, 2016 at 12:30 AM, Junio C Hamano wrote: > So, have we decided to wait, or we'd rather apply the band-aid in > the meantime? I can go either way, just double checking as I > noticed this thread while updating my leftover bits list. Thanks for the follow-up, I was about to ask for a status update on this. As my patch it ready now, and we don't know how long we'd have to wait for the other solution, I'd vote for applying my patch. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.8.0-rc0
On 2/27/2016 0:41, Junio C Hamano wrote: * Some calls to strcpy(3) triggers a false warning from static analysers that are less intelligent than humans, and reducing the number of these false hits helps us notice real issues. A few calls to strcpy(3) in test-path-utils that are already safe has been rewritten to avoid false wanings. * Some calls to strcpy(3) triggers a false warning from static analysers that are less intelligent than humans, and reducing the number of these false hits helps us notice real issues. A few calls to strcpy(3) in "git rerere" that are already safe has been rewritten to avoid false wanings. This is a duplicate. Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC_PATCHv4 4/7] submodule init: redirect stdout to stderr
On Tue, Mar 22, 2016 at 3:06 AM, Stefan Beller wrote: > Reroute the output of stdout to stderr as it is just informative > messages, not to be consumed by machines. Just wondering, what's Git's policy on this? This message is neither an error nor a warning, but just purely informational. As such it semantically does not belong to stderr, or? On the other hand I see multiple places in Git's code where printing to stderr is (mis-)used for informational messages, probably to separate output to be consumed by humans from output to be consumed by machines, like you do here. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC_PATCHv4 4/7] submodule init: redirect stdout to stderr
On Tue, Mar 22, 2016 at 5:47 PM, Stefan Beller wrote: > I think the stance of Git is to write only machine readable stuff to stdout, > and essentially all _(translated) stuff (i.e. human readable) goes to stderr > as > some sort of help or progress indication. Thanks for the clarification. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] (exit 1) is silly
On 3/22/2016 17:16, Junio C Hamano wrote: IMO, this is such a minor thing that once it _is_ in the tree, it's not really worth the patch noise to go and fix it up. IMO, instead of writing this you could have just accepted the patch, reducing the patch noise ;-) Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more
On Tue, Mar 29, 2016 at 6:25 PM, Sven Strickroth wrote: > In MSVC2015 the behavior of vsnprintf was changed. > W/o this fix there is one character missing at the end. How about adding a link to [1] in the commit message and quoting the central "Beginning with the UCRT in Visual Studio 2015 and Windows 10, vsnprintf is no longer identical to _vsnprintf. The vsnprintf function complies with the C99 standard; _vnsprintf is retained for backward compatibility" statement? [1] https://msdn.microsoft.com/en-us/library/1kt27hek.aspx -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more
On Tue, Mar 29, 2016 at 9:13 PM, Sven Strickroth wrote: > diff --git a/compat/snprintf.c b/compat/snprintf.c > index 42ea1ac..0b11688 100644 > --- a/compat/snprintf.c > +++ b/compat/snprintf.c > @@ -9,7 +9,7 @@ > * always have room for a trailing NUL byte. > */ > #ifndef SNPRINTF_SIZE_CORR > -#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) > +#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) && > (!defined(_MSC_VER) || _MSC_VER < 1900) > #define SNPRINTF_SIZE_CORR 1 > #else > #define SNPRINTF_SIZE_CORR 0 I wonder if the logic is (and was) sensible here. We assume that every non-__GNUC__ and non-_MSC_VER compiler on Windows requires the correction. Wouldn't it make sense to not assume requiring the correction unless we know the compiler has this bug? That is, shouldn't this better say #if defined(WIN32) && (defined(__GNUC__) && __GNUC__ < 4) || (defined(_MSC_VER) && _MSC_VER < 1900)) #define SNPRINTF_SIZE_CORR 1 #else #define SNPRINTF_SIZE_CORR 0 -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] MSVC: vsnprintf in Visual Studio 2015 doesn't need SNPRINTF_SIZE_CORR any more
On Wed, Mar 30, 2016 at 9:49 AM, Johannes Schindelin wrote: >> > #ifndef SNPRINTF_SIZE_CORR >> > -#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) >> > +#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4) && >> > (!defined(_MSC_VER) || _MSC_VER < 1900) >> > #define SNPRINTF_SIZE_CORR 1 >> > #else >> > #define SNPRINTF_SIZE_CORR 0 >> >> I wonder if the logic is (and was) sensible here. We assume that every >> non-__GNUC__ and non-_MSC_VER compiler on Windows requires the >> correction. Wouldn't it make sense to not assume requiring the >> correction unless we know the compiler has this bug? That is, >> shouldn't this better say >> >> #if defined(WIN32) && (defined(__GNUC__) && __GNUC__ < 4) || >> (defined(_MSC_VER) && _MSC_VER < 1900)) >> #define SNPRINTF_SIZE_CORR 1 >> #else >> #define SNPRINTF_SIZE_CORR 0 > > Since the standard on Windows always was MS Visual C, it should be assumed > that compilers *other* than GCC followed Visual C's lead. > > Of course, evidence speaks louder than assumptions. > > Therefore I would prefer to keep the current version, at least until we > encounter a case where it is incorrect. Fine with me. It's probably better not to change the logic as we wouldn't know whether this would break things for some exotic compiler currently in use to compile Git. Also ACK from my side on the path then. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 V2] MSVC: VS2013 comes with inttypes.h
On 3/29/2016 19:23, Sven Strickroth wrote: > --- a/compat/mingw.h > +++ b/compat/mingw.h > @@ -415,7 +415,7 @@ int mingw_offset_1st_component(const char *path); > extern void build_libgit_environment(void); > extern const char *program_data_config(void); > #define git_program_data_config program_data_config > -#ifndef __MINGW64_VERSION_MAJOR > +#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < > 1800) > #define PRIuMAX "I64u" > #define PRId64 "I64d" > #else ACK for this part. For reference see [1]. > diff --git a/compat/vcbuild/include/unistd.h b/compat/vcbuild/include/unistd.h > index c65c2cd..b7cc48c 100644 > --- a/compat/vcbuild/include/unistd.h > +++ b/compat/vcbuild/include/unistd.h > @@ -45,11 +45,15 @@ typedef unsigned long long uintmax_t; > > typedef int64_t off64_t; > > +#if !defined(_MSC_VER) || _MSC_VER < 1800 > #define INTMAX_MIN _I64_MIN > #define INTMAX_MAX _I64_MAX > #define UINTMAX_MAX _UI64_MAX > > #define UINT32_MAX 0x /* 4294967295U */ > +#else > +#include > +#endif If we would do "#include " here instead, we could lower the _MSC_VER requirement to at least 1700. According to the comment at [2] we could lower it even to 1600. Also the original code is missing a single space after "#include". [1] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ [2] https://stackoverflow.com/questions/126279/c99-stdint-h-header-and-ms-visual-studio#comment4620359_126279 Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 V3] MSVC: Use shipped headers instead of fallback definitions
On 3/30/2016 13:37, Sven Strickroth wrote: VS2010 comes with stdint.h [1] VS2013 comes with inttypes.h [2] [1] https://stackoverflow.com/a/2628014/3906760 [2] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/ Signed-off-by: Sven Strickroth ACK. Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Trouble with cat-file on tags
Hi, I was trying to use cat-file to get the hash of a tag object (not the hash of the commit object the tag points to), and I'm running into some issues. At the example of a cloned gerry [1] repository: ---8<--- $ git tag test-tag $ git tag -l test-tag v0.0.3 v0.0.4 v0.1.0 v0.1.1 v0.1.2 $ git cat-file tag refs/tags/test-tag fatal: git cat-file refs/tags/test-tag: bad file ---8<--- So for a newly created local tag, cat-file does not seem to work. However: ---8<--- $ git cat-file tag refs/tags/v0.1.2 object 91b0d21eba039e5ba0a90104c9c485735576dcbf type commit tag v0.1.2 tagger Travis Truman 1452693317 -0500 Version 0.1.2 ---8<--- For an existing tag, git-file suddenly *does* seem to work, although I'm puzzled why I'm getting info on the commit object here. I thought "cat-file tag" should explicitly make "cat-file" list information about the tag object itself, not about the commit object the tag points to. Thoughts? [1] https://github.com/trumant/gerry Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trouble with cat-file on tags
On 4/1/2016 11:26, Sebastian Schuberth wrote: ---8<--- $ git tag test-tag $ git tag -l test-tag v0.0.3 v0.0.4 v0.1.0 v0.1.1 v0.1.2 $ git cat-file tag refs/tags/test-tag fatal: git cat-file refs/tags/test-tag: bad file ---8<--- Alright, I just found out why that is: Lighweight tags are not stored as Git objects. As soon as I make it an annoted tag by specifying a message, "cat-file tag" do esnot display a fatal error anymore. However, I still get information about the commit oject iintsead of the tag object. Is this expected? Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trouble with cat-file on tags
On 4/1/2016 11:32, Sebastian Schuberth wrote: However, I still get information about the commit oject iintsead of the tag object. Is this expected? Solved this one, too: Yes it is. I was misreading the docs: "If is specified, the raw (though uncompressed) contents of the will be returned." This means $ git cat-file tag refs/tags/v0.1.2 displays the *contents* of the tag, not the tag itself. Which leads me to the next question: For a given name of an annotated tag, how to get the hash of the tag object? The solution I found for now: $ git show-ref --tags -- v0.1.2 92b67e2b0626519ef8cd4e9cacb2bdafba6d53f0 refs/tags/v0.1.2 Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] doc: clarify that notes can be attached to any type of stored object
Signed-off-by: Sebastian Schuberth --- Documentation/git-notes.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt index 8de3499..5375d98 100644 --- a/Documentation/git-notes.txt +++ b/Documentation/git-notes.txt @@ -234,8 +234,9 @@ which operation triggered the update, and the commit authorship is determined according to the usual rules (see linkgit:git-commit[1]). These details may change in the future. -It is also permitted for a notes ref to point directly to a tree -object, in which case the history of the notes can be read with +It is also permitted for a notes ref to point to any other object in +the object store besides commit objects, that is annotated tags, blobs +or trees. For the latter, the history of the notes can be read with `git log -p -g `. -- 2.8.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What is an efficient way to get all blobs / trees that have notes attached?
Hi, I'm curious whether there's a more efficient way to get a list of blobs / trees (and their names) that have notes attached than doing this: 1) Get all notes refs I'm interested in (git-for-each-ref). 2) For each notes ref, get the list of notes (git-notes list) and store them in a hash table that maps object hashes to notes. 3) Recursively list all blobs / trees (git-ls-tree) and look whether an object's hash is conatined in our table to get its notes. In particular 3) could be expensive for repos with a lot of files as we're looking at all of them just to see whether they have notes attached. Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is an efficient way to get all blobs / trees that have notes attached?
On Fri, Apr 1, 2016 at 2:16 PM, Johan Herland wrote: >> 3) Recursively list all blobs / trees (git-ls-tree) and look whether an >> object's hash is conatined in our table to get its notes. >> >> In particular 3) could be expensive for repos with a lot of files as we're >> looking at all of them just to see whether they have notes attached. > > In (3), why would you need to search through _all_ blobs/trees? Would > it not be cheaper to simply query the object type of each annotated > object from (2)? I.e. something like: > > for notes_ref in $(git for-each-ref refs/notes | cut -c 49-) > do > echo "--- $notes_ref ---" > for annotated_obj in $(git notes --ref=$notes_ref list | cut -c 41-) > do > type=$(git cat-file -t "$annotated_obj") > if test "$type" != "commit" > then > echo "$annotated_obj: $type" > fi > done > done Thanks for the idea. The problem is that I do want to list the notes by path of the object they belong to. As a blob could potentially belong to more than one path (copies of files in the repo), I do not see another way of getting that information other than iterating over all blobs and checking what path(s) they belong to. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: clarify that notes can be attached to any type of stored object
On Fri, Apr 1, 2016 at 5:31 PM, Junio C Hamano wrote: > Sebastian Schuberth writes: > >> Signed-off-by: Sebastian Schuberth >> --- >> Documentation/git-notes.txt | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt >> index 8de3499..5375d98 100644 >> --- a/Documentation/git-notes.txt >> +++ b/Documentation/git-notes.txt >> @@ -234,8 +234,9 @@ which operation triggered the update, and the commit >> authorship is >> determined according to the usual rules (see linkgit:git-commit[1]). >> These details may change in the future. >> >> -It is also permitted for a notes ref to point directly to a tree >> -object, in which case the history of the notes can be read with >> +It is also permitted for a notes ref to point to any other object in >> +the object store besides commit objects, that is annotated tags, blobs >> +or trees. For the latter, the history of the notes can be read with >> `git log -p -g `. > > I do not think this is correct place to patch. The original is not > talking about what objects can have notes attached at all. What it > explains is this. Thanks for the explanation, I was indeed misreading this. I'll try to clarify this section then, too. In order to do so, I think we should mention how to actually create a that directly points to a tree instead of a commit for the history of notes. Would you have an example how to do that? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: clarify that notes can be attached to any type of stored object
On Fri, Apr 1, 2016 at 6:47 PM, Junio C Hamano wrote: > DESCRIPTION > --- > Adds, removes, or reads notes attached to objects, without touching > the objects themselves. > > This says "notes attached to objects" and "the objects themselves". > They do not limit the target of attaching a note to "commits". > So this may be the place to add " Note that notes can be attached > to any kind of objects, not limited to commits" or something, if > we really wanted to. Done, I'll send a patch shortly. But I wanted to list the supported object types explicitly, in particular as many guide in the net are unclear that only annotated tags are object, and lightweight ones are not. > Notes can also be added to patches prepared with `git format-patch` by > using the `--notes` option. Such notes are added as a patch commentary > after a three dash separator line. > > This paragraph _might_ be confusing to new readers. The "added to" > sounds as if you are attaching a note to a non-object which is a > patch. But this "add" is about "inserting the contents of the note > attached to the commit being formatted" and corresponds to "can be > shown by" in the previous paragraph. We may want to avoid the verb > "add" when talking about the use of data stored in an existing note > to somewhere else like this. Done. > add:: > Add notes for a given object (defaults to HEAD). Abort if the > > And this "Add notes for " should probably be reworded to "Attach > notes to" to match the first sentence in the description. After a bit of thinking, I don't believe we should do this. All subcommand docs start with the verb the subcommand is named after. In that sense making the "add" docs start with "Attach" would be inconsistent and probably raise the question why the subcommand is not called "attach" after all. Also, in the description it says "Adds, removes, or reads notes attached to objects", so it includes "[...] removes [...] notes attached to objects", and if you read it like this the word "attach" is not specific to the "add" subcommand. So I left this as-is in my patch. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] doc: Clarify which objects notes can be attached to
Explicitly name the supported object types, and ensure patches cannot be misinterpreted as non-objects that can have notes attached. Signed-off-by: Sebastian Schuberth --- Documentation/git-notes.txt | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt index 8de3499..101e6ba 100644 --- a/Documentation/git-notes.txt +++ b/Documentation/git-notes.txt @@ -25,7 +25,8 @@ SYNOPSIS DESCRIPTION --- Adds, removes, or reads notes attached to objects, without touching -the objects themselves. +the objects themselves. Supported objects are commits, blobs, trees +and annotated tags. By default, notes are saved to and read from `refs/notes/commits`, but this default can be overridden. See the OPTIONS, CONFIGURATION, and @@ -39,9 +40,9 @@ message stored in the commit object, the notes are indented like the message, after an unindented line saying "Notes ():" (or "Notes:" for `refs/notes/commits`). -Notes can also be added to patches prepared with `git format-patch` by -using the `--notes` option. Such notes are added as a patch commentary -after a three dash separator line. +Notes contents can also be included in patches prepared with +`git format-patch` by using the `--notes` option. Such notes are added +as a patch commentary after a three dash separator line. To change which notes are shown by 'git log', see the "notes.displayRef" configuration in linkgit:git-log[1]. -- 2.8.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing
On Tue, Apr 19, 2016 at 11:04 PM, Junio C Hamano wrote: >> I dropped the support for the older version to keep the code as >> simple as possible (plus it would be cumbersome to test with an >> outdated Git LFS version). Since it is probably a niche feature I >> thought that might be acceptable. > > It is bad enough that clients need to be adjusted at all in the > first place, but I would have found it very troubling if the > problematic change to LFS thing were made in such a way that it > makes backward compatible adjustment on the client code impossible. If clients rely on output targeted at human consumption it's not surprising that these clients need to be adjusted from time to time. What's troubling is not the change to git-lfs, but the very un-generic way git-p4 is implemented. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing
On Wed, Apr 20, 2016 at 10:10 AM, wrote:
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1064,8 +1064,17 @@ class GitLFS(LargeFileSystem):
> if pointerProcess.wait():
> os.remove(contentFile)
> die('git-lfs pointer command failed. Did you install the
> extension?')
> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
> +
> +# Git LFS removed the preamble in the output of the 'pointer' command
git-lfs did not remove the output. It simply goes to stderr instead of
stdout now. That said, could a fix simply be to capture both stdout
and sterr? If the output to the streams remain interleaved it should
look exactly like before.
> +# starting from version 1.2.0. Check for the preamble here to support
> +# earlier versions.
> +# c.f.
> https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43
> +preamble = 'Git LFS pointer for ' + contentFile + '\n\n'
> +if pointerFile.startswith(preamble):
> +pointerFile = pointerFile[len(preamble):]
> +
> +oidEntry = [i for i in pointerFile.split('\n') if
> i.startswith('oid')]
> +oid = oidEntry[0].split(' ')[1].split(':')[1]
Why do we need to remove the preamble at all, if present? If all we
want is the oid, we should simply only look at the line that starts
with that keyword, which would skip any preamble. Which is what you
already do here. However, I'd probably use .splitlines() instead of
.split('\n') and .startswith('oid ') (note the trailing space) instead
of .startswith('oid') to ensure "oid" is a separate word.
But then again, I wonder why there's so much split() logic involved in
extracting the oid. Couldn't we replace all of that with a regexp like
oid = re.search(r"^oid \w+:(\w+)", pointerFile, re.MULTILINE).group(1)
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing
On Wed, Apr 20, 2016 at 5:30 PM, Junio C Hamano wrote: >> If clients rely on output targeted at human consumption it's not >> surprising that these clients need to be adjusted from time to time. >> What's troubling is not the change to git-lfs, but the very un-generic >> way git-p4 is implemented. > > Sounds like the subcommand they are using is not meant for > scripting? What is the kosher way to get at the information they > can use that is a supported interface for scripters? The "pointer" subcommand indeed is listed under "Low level commands" by "git lfs" (without any arguments), and as such it probably can be considered for scripting use. However, before my fix in [1] the subcommand was printing both output targeted at humans and output targeted at scripts to stdout. After my fix, only output targeted at script goes to stdout, and output targeted at humans goes to stderr. [1] https://github.com/github/git-lfs/pull/1105 -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing
On Fri, Apr 22, 2016 at 9:53 AM, Lars Schneider wrote: > What would be the best way forward? A v3 with a better commit message > mentioning the array -> string change? I'd vote for that, yes. Also v3 could then properly incorporate my regexp. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing
On Sun, Apr 24, 2016 at 8:58 PM, wrote:
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1064,8 +1064,15 @@ class GitLFS(LargeFileSystem):
> if pointerProcess.wait():
> os.remove(contentFile)
> die('git-lfs pointer command failed. Did you install the
> extension?')
> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
> +
> +# Git LFS removed the preamble in the output of the 'pointer' command
> +# starting from version 1.2.0. Check for the preamble here to support
> +# earlier versions.
> +# c.f.
> https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43
> +if pointerFile.startswith('Git LFS pointer for'):
> +re.sub(r'Git LFS pointer for.*\n\n', '', pointerFile)
I liked the code from v2 better. I know Ben said "there could be
expansions or other modifications applied by git-lfs between input and
output", but I believe it's better to be too strict than too lenient
if you're omitting lines from the output. Also, the regex matches
against the whole multi-line string. That is, if the file for some
reason was ending in '\n\n' instead of just '\n', the '.*' would match
almost all content of the pointer file, not just the remains of the
preamble. One way to fix this would be to use
re.sub(r'Git LFS pointer for [^\n]+\n\n', '', pointerFile)
instead.
Regards,
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing
On Mon, Apr 25, 2016 at 9:33 AM, Lars Schneider wrote: >> if you're omitting lines from the output. Also, the regex matches >> against the whole multi-line string. That is, if the file for some >> reason was ending in '\n\n' instead of just '\n', the '.*' would match >> almost all content of the pointer file, not just the remains of the >> preamble. One way to fix this would be to use >> >> re.sub(r'Git LFS pointer for [^\n]+\n\n', '', pointerFile) >> >> instead. > > In general you are right as "*" is greedy. However, in Python "." matches any > character except a newline [1]. Therefore I think the regex is correct. Ah, thanks for pointing that out. Looks ok to me then. > Nevertheless... thanks for making me read the line again. I forgot to > assign the pointerFile variable in the version I sent around :-( > > This is how it should be: > > pointerFile = re.sub(r'Git LFS pointer for.*\n\n', '', pointerFile) Right. Good you've catched that! -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] git-p4: fix Git LFS pointer parsing
On Thu, Apr 28, 2016 at 8:26 AM, wrote: > diff to v3: > * fix missing assignment of pointerFile variable > ($gmane/292454, thanks Sebastian for making me aware) > * fix s/brake/break/ in commit message > ($gmane/292451, thanks Eric) The series looks good to me now. Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v1] Add Travis CI support
On 10/4/2015 19:46, Junio C Hamano wrote: The very nice thing with Travis-CI is that it does not only test the repository's branches, but also all pull-requests. OK, that is the first real argument I heard for enabling it on git/git that is worth listening to. I was mentioning that very argument in the context of PRs filed for use with submitgit already back in July in the conversation at [1] in which you took part. [1] https://github.com/rtyley/submitgit/issues/16 Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] Add Travis CI support
On 10/11/2015 19:55, larsxschnei...@gmail.com wrote: + sudo apt-get update -qq + sudo apt-get install -y apt-transport-https + sudo apt-get install perforce-server git-lfs Why no "-y" also in this line, or append these to the previous line? Or maybe even better, like [1] does, also use "--qq" (which implies "-y") for "apt-get install"? +install: make configure && ./configure + +before_script: make + +script: make --quiet test Semantically, it does not seem correct to me that configuarion goes to the install step. As "make test" will build git anyway, I'd instead propose to get rid of "install" and just say: before_script: make configure && ./configure script: make --quiet test [1] https://github.com/git/git/pull/154/files Regards, Sebastian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] Add Travis CI support
On Mon, Oct 12, 2015 at 6:02 PM, Junio C Hamano wrote: >> Semantically, it does not seem correct to me that configuarion goes to >> the install step. As "make test" will build git anyway, I'd instead >> propose to get rid of "install" and just say: >> >> before_script: make configure && ./configure >> >> script: make --quiet test > > Very good point. Do we even need to do anything in the "install" > target? We aim to be able to testable without any installed Git, > and not running "make install" at all, ever, would be one way to > make sure that works. Note that Travis' "install" step is about installing dependencies for the application to build [1], not for installing the built application (i.e. what "make install" does). In any case, I still think configuring the application to built in this step is wrong. > we are to start using automated tests, I wonder if we want to build > (and optionally test) with various combinations of the customization > options (e.g. NO_CURL, NO_OPENSSL, NO_MMAP, NO_IPV6, NO_PERL etc.) I like that idea, but I think we should save that for a future improvement to .travis.yml. [1] http://docs.travis-ci.com/user/installing-dependencies/ -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] Add Travis CI support
On Mon, Oct 12, 2015 at 7:12 PM, Lars Schneider wrote: >>> +install: make configure && ./configure >>> + >>> +before_script: make >>> + >>> +script: make --quiet test >> >> Semantically, it does not seem correct to me that configuarion goes to the >> install step. As "make test" will build git anyway, I'd instead propose to >> get rid of "install" and just say: >> >> before_script: make configure && ./configure >> >> script: make --quiet test > > I understand your point. I did this to make the "make" logs easily accessible > (no option "--quite"). By default Travis CI automatically collapses the logs > from all stages prior to the "script" stage. You can uncollapse these logs by > clicking on the little triangle on the left border of the log. Therefore the > "make" logs are available without noise. To make this more clear, I guess what you're referring to is the visual difference between [1] and [2], correct? > Do you see value in "make" logs? > > If yes then we could also do: > before_script: make configure && ./configure && make Reading through Travis' docs [3] again, "before_script" is documented to "return a non-zero exit code, the build is errored and stops immediately", while "script" is documented as "returns a non-zero exit code, the build is failed, but continues to run before being marked as failed". As it does not make much sense to continue the build or even start testing if the build failed, maybe it's indeed best to do: before_script: make configure && ./configure && make script: make --quiet test [1] https://travis-ci.org/larsxschneider/git/jobs/84805733 [2] https://travis-ci.org/larsxschneider/git/jobs/84955658 [3] http://docs.travis-ci.com/user/customizing-the-build/ -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] Add Travis CI support
On Mon, Oct 12, 2015 at 9:40 PM, Lars Schneider
wrote:
>> This is a slightly related tangent, but we saw a few build issues
>> reported recently on customized configurations like NO_PTHREAD. If
>> we are to start using automated tests, I wonder if we want to build
>> (and optionally test) with various combinations of the customization
>> options (e.g. NO_CURL, NO_OPENSSL, NO_MMAP, NO_IPV6, NO_PERL etc.)
> This easy to do. However, the more we environment settings we define the
> longer the build runs. I created a test matrix that runs the following
> combinations:
> {Linux | OSX} * {gcc | clang} * {Default, NO_PTHREAD, NO_CURL, NO_OPENSSL,
> NO_MMAP, NO_IPV6, NO_PERL}
>
> These result in 28 (= 2*2*7) combinations. I created a build without the
> "Default" environment (=24 combination) here:
> https://travis-ci.org/larsxschneider/git/builds/84978673
>
> Should I add them them to the Travis CI patch?
I'd say it depends on how long such a matrix build would take in
average. Personally, I'd prefer to not wait more than, say, 30 minutes
for testing a PR. From your Travis build history it looks to me as if
we already exceed that limit many fold, so I'd prefer to not use
matrix builds unless we find ways to speed up the build in general,
for example by using ccache [1].
[1] http://docs.travis-ci.com/user/caching/#ccache-cache
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/3] Add Travis CI support
On Mon, Oct 12, 2015 at 9:43 PM, Lars Schneider wrote: >> Reading through Travis' docs [3] again, "before_script" is documented >> to "return a non-zero exit code, the build is errored and stops >> immediately", while "script" is documented as "returns a non-zero exit >> code, the build is failed, but continues to run before being marked as >> failed". As it does not make much sense to continue the build or even >> start testing if the build failed, maybe it's indeed best to do: >> >> before_script: make configure && ./configure && make >> >> script: make --quiet test > > Ok, then I will make it so :-) Also, this has the added benefit of being quickly able to see how much time building vs testing took. As these two are the big blocks, we'd want to optimize both steps for time, and it's easier to see what we gain e.g. from a possible build-time improvement if these steps are listed individually in the Travis log. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] clone: Allow an explicit argument for parallel submodule clones
On 23.10.2015 20:44, Stefan Beller wrote: [...] which may pick reasonable defaults if you don't specify an explicit number. IMO the above should also be mentioned ini the docs: +-j:: +--jobs:: + The number of submodules fetched at the same time. Otherwise, from reading the docs, my immediate question would be "What's the default for n if not specified?" -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ls-files: Add eol diagnostics
On 31.10.2015 11:25, Matthieu Moy wrote: >> ca:text-no-eol wt:text-no-eol t/t5100/empty >> ca:binarywt:binaryt/test-binary-2.png >> ca:text-lf wt:text-lf t/t5100/rfc2047-info-0007 >> ca:text-lf wt:text-crlf doit.bat >> ca:text-crlf-lf wt:text-crlf-lf locale/XX.po > > I would spell the first "in" or "idx" (for "index"), not "ca" (for > "cache"). I think we avoid talking about "the cache" these days even > though the doc sometimes says "cached in the index" (i.e. use "cache" as > a verb, not a noun). Good point, I'd prefer "idx" over ca", too. However, the commit message says "to check if text files are stored normalized in the *repository*", yet the output refers to the index / cache. Is there a (potential) difference between line endings in the index and repo? AFAIK there is not. Any I find it a bit confusing to refer to the index where, as e.g. for a freshly cloned repo the index should be empty, yet you do have specific line endings in the repo. Long story short, how about consistently talking about line endings in the repo, and also using "repo" instead of "ca" here? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ls-files: Add eol diagnostics
On Sun, Nov 1, 2015 at 7:40 PM, Matthieu Moy wrote: >> Any I find it a bit confusing to refer to the index where, as e.g. for >> a freshly cloned repo the index should be empty, > > No it is not. The index is a complete snapshot of your working tree. > When you have no uncommited staged changes, the index contains all files > that are in HEAD. Most commands show you _changes_ in the index (wrt > HEAD or wrt the working tree), but the index itself contain all files. Thanks for the info. > At stage 4), you really want to see the content of the index, because > your HEAD is still broken. Ok, I'm convinced. Thanks again! -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] Add Travis CI support
On Fri, Nov 6, 2015 at 2:11 PM, Lars Schneider wrote:
> Per platform/compiler (Linux&Mac/clang&gcc) we run two configurations. One
> normal configuration (see the lonely "-" right under "matrix:") and one
> configuration with all sorts of things are disabled ("NO...").
>
> You can see all 8 configurations ([linux, mac] * [clang, gcc] * [normal,
> NO_...]) here:
> https://travis-ci.org/larsxschneider/git/builds/89598194
Aren't these 8 configurations a bit too much? I see the total running
time is about 2 hours. For my taste, this is way to much to give
developers feedback about the status of their PR. It should be
something < 30 minutes.
IMO, the purpose of the Travis CI configuration mainly is to 1) save
developers work by not requiring to build manually, 2) build on other
platforms than the developer has access to. I doubt that the average
developer manually builds anything close to these 8 configurations
before we had this job, so I wouldn't make it a requirement for Travis
to do much more than a developer could / would to manually.
On the other hand, I see the point in letting a CI system test more
configurations than manually feasible. But that should be done as part
of a different job then. E.g. we could have a "fast" PR validation
job, and a "slow" nightly build job.
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] Add Travis CI support
On Fri, Nov 6, 2015 at 2:28 PM, Lars Schneider wrote: > Well, I partly agree. Right now the running time is ~20 min (that means less > than your 30min target!). After ~10min you even have all Linux results, Mac > takes a bit longer. Travis shows you 2h because that is the time that would > be required if all builds where run one after another (we run builds in > parallel). Are you sure about than? I mean, what sense does it make to show how long it *would* have taken *if* the builds were running serially? I can see that the longest of the jobs took 21 minutes, which is ok. But that does not mean that all jobs completed in within 21 minutes. It could be that not all jobs started at (about) the same time due to a lack of resources, and that the last job did not compete before the 2 hours were over because it only started to run 1 hours and 40 minutes befor ethe first job was started. > That being said, I see your point of to avoiding to burn Travis CI resources > meaningless. If I am not mistaken then you can configure Travis in a way that > it runs different configurations for different branches. E.g. I would like to > run all 8 configurations on maint, master, next and maybe pu. All other > branches on peoples own forks should be fine with the default Linux build > (~10min). > > What do you think? I think running different configuration per branch makes sense, yes. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] Add Travis CI support
On Fri, Nov 6, 2015 at 2:55 PM, Lars Schneider wrote: >> I think running different configuration per branch makes sense, yes. > > If the list decides to accept this patch. Do you think that would be a > necessary requirement for the first iteration? No. I think this could be addressed later as an improvements. To me it's more important to finally get *some* sane Travis CI configuration in. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] ls-files: Add eol diagnostics
On 21.11.2015 08:36, Torsten Bögershausen wrote:
> git ls-files --eol gives an output like this:
>
> i/text-no-eol w/text-no-eol attr/text=auto t/t5100/empty
I'm sorry if this has been discussed before, but hav you considered to use a
header line and omit the prefixed from the columns instead? Like
index working tree attributesfile
binarybinary -text t/test-binary-2.png
text-lf text-lf eol=lft/t5100/rfc2047-info-0007
text-lf text-crlfeol=crlf doit.bat
text-crlf-lf text-crlf-lf locale/XX.po
I believe this would be both easier to read for humans, and easier to parse for
scripts that e.g. want to compare line endings in the index and working tree.
> +stats_ascii () {
> + case "$1" in
[...]
> + *)
> + echo huh $1
> + ;;
Personally, I'm not a big fan of supposedly funny output like this. How about
printing a proper message rather than "huh", even for cases that should not
happen?
--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] ls-files: Add eol diagnostics
On Mon, Nov 23, 2015 at 6:05 PM, Torsten Bögershausen wrote: > or, as another example: > git ls-files -o --eol > i/ w/binaryattr/ zlib.o I see, somewhat convincing I have to agree. On another note, how about making the prefix either all just one letter (i.e. "attr/" becomes "a/"), or all multi-letter abbreviations (i.e. "i/" becomes "idx/" and "w/" becomes "wt/" or "wtree/" or "tree/")? -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/subtree: Remove --annotate
On Sat, Jan 2, 2016 at 9:36 PM, David Greene wrote: > commit messages. git has other tools more suited to rewriting > commit messages and it's easy enough to use them after a subtree > split. For completeness, it probably would be a good idea to name examples for such more suitable tools as part of the commit message. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Questions on passing --depth to git-clone vs. git-fetch
Hi, I recently compared the results of doing $ git clone --depth=1 https://github.com/git/git.git git-clone-depth-1 versus $ mkdir git-fetch-depth-1 $ cd git-fetch-depth-1 $ git init $ git remote add origin https://github.com/git/git.git $ git fetch --depth=1 and noticed a few things: 1. The docs of clone [1] say about --depth "Create a shallow clone with a history truncated to the specified number of revisions" while for fetch the docs [2] say "[...] to the specified number of commits [...]". As in this particular case revision are always commits, I think the clone docs should also say "commits". 2. In the fetch docs --depth is described to "Deepen or shorten the history of a shallow repository created by git clone". That sounds as if my example from above where I initialze a repo manually would not allow fetch to be called with --depth as I did not clone before. But in fact my example works fine. I guess we need some clarfication in the wording here. 3. When running "git log --all -oneline" in the two working trees I get different results, which is not what I'd expect: $ cd git-clone-depth-1 $ git log --all --oneline 7548842 Git 2.7 versus $ cd git-fetch-depth-1 $ git log --all --oneline b819526 Merge branch 'jk/notes-merge-from-anywhere' into pu e2281f4 What's cooking (2016/01 #01) ef7b32d Sync with 2.7 7548842 Git 2.7 833e482 Git 2.6.5 So in the clone case only the specified number of commits from the tip of the default branch (master in this case) is fetched, not of each remote branch history. fetch in the other hand really gets the specified number of commits from the tip of each remote branch history. I don't know whether this behavior is inded or not as I cannot find any docs on it either way. But it seems inconsistent to me that clone with --depth only gets the history for the default branch, as clone without --depth would give me the history of all branches. For completeness, I'm using Git for Windows 2.7. Any comments? [1] https://git-scm.com/docs/git-clone [1] https://git-scm.com/docs/git-fetch -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Issues with gitattributes pattern matching
Hi, after reading though [1] and [2] again, I believe a pattern in .gitattributes like */src/*/assets/**/*-expected-* text eol=lf should match a committed file at reporter/src/funTest/assets/NPM-is-windows-1.0.2-expected-NOTICE In other words, "**" should be able to match "nothing", but it doesn't seem to do in my case. To cross-check, assuming that ls-files supports the same patterns, running git ls-files "*/src/*/assets/**/*-expected-*" indeed does not list the committed file at reporter/src/funTest/assets/NPM-is-windows-1.0.2-expected-NOTICE for me. Tested with Git 2.20 on Windows and Git 2.19.2 on Linux. Is it a documentation error or a bug in Git? [1] https://git-scm.com/docs/gitattributes [2] https://git-scm.com/docs/gitignore#_pattern_format -- Sebastian Schuberth
Re: [PATCH 2/9] ci/lib.sh: encapsulate Travis-specific things
On 9/3/2018 11:10 PM, Johannes Schindelin via GitGitGadget wrote: +if test -n "$TRAVIS_COMMIT" +then + # We are running within Travis CI Personally, I'd find a check like if test "$TRAVIS" = "true" more speaking (also see [1]). [1] https://docs.travis-ci.com/user/environment-variables/ -- Sebastian Schuberth
Re: [PATCH 0/9] Offer to run CI/PR builds in Visual Studio Team Services
On 9/3/2018 11:10 PM, Johannes Schindelin via GitGitGadget wrote: The one sad part about this is the Windows support. Travis lacks it, and we work around that by using Visual Studio Team Services (VSTS) indirectly: one phase in Travis would trigger a build, wait for its log, and then paste that log. I'm sorry if this has been discussed before, but as this recap doesn't mention it: Has AppVeyor been considered as an option? It seems to be the defacto standard for Windows CI for projects on GitHub. -- Sebastian Schuberth
Re: [PATCH 1/2] t0051: test GIT_TRACE to a windows named pipe
On 9/7/2018 8:19 PM, Jeff Hostetler via GitGitGadget wrote: +test_expect_success MINGW 'o_append write to named pipe' ' Shouldn't this be "test_expect_failure" here, and then be changed to "test_expect_success" by your second patch? -- Sebastian Schuberth
Re: [PATCH v1] travis-ci: add static analysis build job to run coccicheck
On 2017-04-11 09:26, Lars Schneider wrote: Add a dedicated build job for static analysis. As a starter we only run coccicheck but in the future we could run Clang Static Analyzer or similar tools, too. Just FYI, some time ago someone (I don't recall who) signed up Git with Coverity's free scan service for OSS projects: https://scan.coverity.com/projects/git?tab=overview Maybe it makes sense to at least link to this page, too? -- Sebastian Schuberth
[PATCH] sha1_file: remove an used fd variable
Signed-off-by: Sebastian Schuberth
---
sha1_file.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 7106389..9ecf71f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3970,7 +3970,6 @@ int read_loose_object(const char *path,
void **contents)
{
int ret = -1;
- int fd = -1;
void *map = NULL;
unsigned long mapsize;
git_zstream stream;
@@ -4020,7 +4019,5 @@ int read_loose_object(const char *path,
out:
if (map)
munmap(map, mapsize);
- if (fd >= 0)
- close(fd);
return ret;
}
--
https://github.com/git/git/pull/344
[PATCH] submodule: remove a superfluous second check for the "new" variable
Signed-off-by: Sebastian Schuberth
---
submodule.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/submodule.c b/submodule.c
index c52d663..68623bd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1396,8 +1396,7 @@ int submodule_move_head(const char *path,
cp1.no_stdin = 1;
cp1.dir = path;
- argv_array_pushl(&cp1.args, "update-ref", "HEAD",
-new ? new : EMPTY_TREE_SHA1_HEX, NULL);
+ argv_array_pushl(&cp1.args, "update-ref", "HEAD", new,
NULL);
if (run_command(&cp1)) {
ret = -1;
--
https://github.com/git/git/pull/345
Re: Feature request: --format=json
On 2017-04-17 14:44, Fred .Flintstone wrote: However, if I want something more suitable for machine parsing, is there any way to get that output? Instead of machine parsing, why not directly get what you want via libgit2 (or one of its language bindings), or jgit? [1] https://github.com/libgit2/libgit2 [2] https://github.com/eclipse/jgit -- Sebastian Schuberth
Re: [PATCH] submodule: remove a superfluous second check for the "new" variable
On 2017-04-17 20:02, Stefan Beller wrote:
>> diff --git a/submodule.c b/submodule.c
>> index c52d663..68623bd 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -1396,8 +1396,7 @@ int submodule_move_head(const char *path,
>> cp1.no_stdin = 1;
>> cp1.dir = path;
>>
>> - argv_array_pushl(&cp1.args, "update-ref", "HEAD",
>> -new ? new : EMPTY_TREE_SHA1_HEX,
>> NULL);
>> + argv_array_pushl(&cp1.args, "update-ref", "HEAD",
>> new, NULL);
>
> EMPTY_TREE_SHA1_HEX != NULL?
>
> Can you clarify the intent in the commit message?
Sure. A few lines up (3 lines out of the diff) we have "if (new) {" [1], thus
there's no need to check "new != NULL" here again.
[1]
https://github.com/git/git/pull/345/files#diff-471db3ea6697763218bb8335a95ece57R1392
--
Sebastian Schuberth
Failed shallow submodule clone for fixed SHA1 falls back to checking out HEAD
Hi, this is using "git version 2.12.2.windows.2" on Windows / "git version 2.12.2-639-g584f897" on Linux. I have configured my superproject with .gitmodules saying ---8<--- [submodule "src/funTest/resources/projects/external/jgnash"] path = src/funTest/resources/projects/external/jgnash url = https://github.com/ccavanaugh/jgnash.git shallow = true ---8<--- and configured the submodule to checkout commit 2aa4cce7d7fd46164030f2b4d244c4b89e77f722 [1]. When doing a fresh clone of my superproject via "git clone --recursive" I get ---8<--- error: Server does not allow request for unadvertised object 2aa4cce7d7fd46164030f2b4d244c4b89e77f722 Fetched in submodule path 'src/funTest/resources/projects/external/jgnash', but it did not contain 2aa4cce7d7fd464030f2b4d244c4b89e77f722. Direct fetching of that commit failed. ---8<--- So far so good, it simply seems that GitHub does not support allowReachableSHA1InWant [2]. The interesting bit is that my submodule checkout still ends up being shallow, but poiting to HEAD: ---8<--- $ cd src/funTest/resources/projects/external/jgnash $ git log commit 12036fffb6c620515edd96416363fd1749b5d003 (grafted, HEAD -> master, origin/master, origin/HEAD) Author: Craig Cavanaugh Date: Tue Apr 18 05:33:06 2017 -0400 Fix typos ---8<--- Wouldn't it make more sense to unshallow the submodule clone in this case and checkout the configured commit 2aa4cce7d7fd46164030f2b4d244c4b89e77f722 afterwards? At least I'd be getting what I asked for in that case, even if at the cost of additional network traffic. Because if I read [3] correctly, the command line option belonging to "submodule..shallow" is called "--[no-]recommend-shallow", i.e. it's only a recommendation, to falling back to a full clone should be fine. [1] https://github.com/ccavanaugh/jgnash/commit/2aa4cce7d7fd46164030f2b4d244c4b89e77f722 [2] https://git-scm.com/docs/git-config#git-config-uploadpackallowReachableSHA1InWant [3] https://git-scm.com/docs/git-submodule Regards, Sebastian
Re: Fwd: Failed shallow submodule clone for fixed SHA1 falls back to checking out HEAD
On 2017-04-18 21:12, Stefan Beller wrote: Wouldn't it make more sense to unshallow the submodule clone in this case and checkout the configured commit 2aa4cce7d7fd46164030f2b4d244c4b89e77f722 afterwards? If I remember correctly the conclusion of the discussion at the time was that people are more interested in "less data" rather than "correct data" because otherwise you would not have asked for shallow. I do believe this is an awkward choice. What does it help to get less data if it's the wrong data? -- Sebastian Schuberth
[PATCH] gitmodules: clarify what history depth a shallow clone has
Signed-off-by: Sebastian Schuberth --- Documentation/gitmodules.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 8f7c50f..6f39f24 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -84,8 +84,8 @@ submodule..ignore:: submodule..shallow:: When set to true, a clone of this submodule will be performed as a - shallow clone unless the user explicitly asks for a non-shallow - clone. + shallow clone (with a history depth of 1) unless the user explicitly + asks for a non-shallow clone. EXAMPLES -- https://github.com/git/git/pull/347
[PATCH] gitmodules: clarify the ignore option values
Add more structure and describe each possible option in a self-contained way, not referring to any of the previously described options. Signed-off-by: Sebastian Schuberth --- Documentation/gitmodules.txt | 31 --- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 8f7c50f..4700624 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -66,17 +66,26 @@ submodule..fetchRecurseSubmodules:: submodule..ignore:: Defines under what circumstances "git status" and the diff family show - a submodule as modified. When set to "all", it will never be considered - modified (but will nonetheless show up in the output of status and - commit when it has been staged), "dirty" will ignore all changes - to the submodules work tree and - takes only differences between the HEAD of the submodule and the commit - recorded in the superproject into account. "untracked" will additionally - let submodules with modified tracked files in their work tree show up. - Using "none" (the default when this option is not set) also shows - submodules that have untracked files in their work tree as changed. - If this option is also present in the submodules entry in .git/config of - the superproject, the setting there will override the one found in + a submodule as modified. The following values are supported: + + all;; The submodule will never be considered modified (but will + nonetheless show up in the output of status and commit when it has + been staged). + + dirty;; All changes to the submodule's work tree will be ignored, only + committed differences between the HEAD of the submodule and its + recorded state in the superproject are taken into account. + + untracked;; Only untracked files in submodules will be ignored. + Committed differences and modifications to tracked files will show + up. + + none;; No modifiations to submodules are ignored, all of committed + differences, and modifications to tracked and untracked files are + shown. This is the default option. + + If this option is also present in the submodules entry in .git/config + of the superproject, the setting there will override the one found in .gitmodules. Both settings can be overridden on the command line by using the "--ignore-submodule" option. The 'git submodule' commands are not -- https://github.com/git/git/pull/348
[PATCH v2] git-gui--askpass: generalize the wording
git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to only rfer to "OpenSSH", also
because another SSH client like PuTTY might be in use. So generalize
wording and also say which parent process, i.e. Git, requires
authentication.
Signed-off-by: Sebastian Schuberth
---
git-gui/git-gui--askpass | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..4e3f00d 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -2,7 +2,7 @@
# Tcl ignores the next line -*- tcl -*- \
exec wish "$0" -- "$@"
-# This is a trivial implementation of an SSH_ASKPASS handler.
+# This is a trivial implementation of an GIT_ASKPASS / SSH_ASKPASS handler.
# Git-gui uses this script if none are already configured.
package require Tk
@@ -12,7 +12,7 @@ set yesno 0
set rc 255
if {$argc < 1} {
- set prompt "Enter your OpenSSH passphrase:"
+ set prompt "Enter your password / passphrase:"
} else {
set prompt [join $argv " "]
if {[regexp -nocase {\(yes\/no\)\?\s*$} $prompt]} {
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
}
-wm title . "OpenSSH"
+wm title . "Git Authentication"
tk::PlaceWindow .
vwait rc
exit $rc
--
https://github.com/git/git/pull/195
Re: [PATCH v2] git-gui--askpass: generalize the wording
+ Pat
On 2017-04-27 08:38, Sebastian Schuberth wrote:
git-gui--askpass is not only used for SSH authentication, but also for
HTTPS. In that context it is confusing to only rfer to "OpenSSH", also
because another SSH client like PuTTY might be in use. So generalize
wording and also say which parent process, i.e. Git, requires
authentication.
Signed-off-by: Sebastian Schuberth
---
git-gui/git-gui--askpass | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-gui/git-gui--askpass b/git-gui/git-gui--askpass
index 4277f30..4e3f00d 100755
--- a/git-gui/git-gui--askpass
+++ b/git-gui/git-gui--askpass
@@ -2,7 +2,7 @@
# Tcl ignores the next line -*- tcl -*- \
exec wish "$0" -- "$@"
-# This is a trivial implementation of an SSH_ASKPASS handler.
+# This is a trivial implementation of an GIT_ASKPASS / SSH_ASKPASS handler.
# Git-gui uses this script if none are already configured.
package require Tk
@@ -12,7 +12,7 @@ set yesno 0
set rc 255
if {$argc < 1} {
- set prompt "Enter your OpenSSH passphrase:"
+ set prompt "Enter your password / passphrase:"
} else {
set prompt [join $argv " "]
if {[regexp -nocase {\(yes\/no\)\?\s*$} $prompt]} {
@@ -60,7 +60,7 @@ proc finish {} {
set ::rc 0
}
-wm title . "OpenSSH"
+wm title . "Git Authentication"
tk::PlaceWindow .
vwait rc
exit $rc
--
Sebastian Schuberth
Cloning a custom refspec does not work as expected
Hi, I'm trying to clone a custom ref, avoiding the need to init && fetch manually. From reading [1] which says: "Set a configuration variable in the newly-created repository; this takes effect immediately after the repository is initialized, but before the remote history is fetched or any files checked out. [...] This makes it safe, for example, to add additional fetch refspecs to the origin remote." I'd assume that this would work: $ git clone -c remote.origin.fetch=+refs/changes/*:refs/remotes/origin/changes/* https://gerrit.googlesource.com/git-repo In fact, this *does* add the custom refs correct to .git/config, but they are not fetched during the clone. I can manually fix that by doing $ cd git-repo $ git fetch afterwards, but the whole point is to avoid exactly that separate step. Is this a code bug or documentation bug? I'm using git version 2.12.2.windows.2. [1] https://git-scm.com/docs/git-clone#git-clone---configltkeygtltvaluegt -- Sebastian Schuberth
Re: git-clone --config order & fetching extra refs during initial clone
On 5/3/2017 22:22, Jeff King wrote: >> My patch deals with 'remote..refspec', i.e. 'remote->fetch'. >> Apparently some extra care is necessary for 'remote..tagOpt' and >> 'remote->fetch_tags', too. Perhaps there are more, I haven't checked >> again, and maybe we'll add similar config variables in the future. So >> I don't think that dealing with such config variables one by one in >> 'git clone', too, is the right long-term solution... but perhaps it's >> sufficient for the time being? > > I think your patch is a strict improvement and we don't need to hold up > waiting for a perfect fix (and because of the --single-branch thing you > mentioned, this may be the best we can do anyway). I agree. Let's fix one thing at a time, and not make this a overarching "account for all previously ignored config variables during clone" fix. Esp. as specifying the refspec is explicitly documented to work durig clone [1] I think we should get this in ASAP. Thanks for your work so far! [1] https://git-scm.com/docs/git-clone#git-clone---configltkeygtltvaluegt Regards, Sebastian
Re: [PATCH] fetch-pack: always allow fetching of literal SHA1s
On 2017-05-10 19:00, Jonathan Nieder wrote: Right, makes sense. I wondered if GitHub should be turning on allowTipSHA1InWant, but it really doesn't make sense to. We _do_ hide some internal refs[1], and they're things that users wouldn't want to fetch. The problem for your case really is just on the client side, and this patch fixes it. [...] [1] The reachability checks from upload-pack don't actually do much on GitHub, because you can generally access the objects via the API or the web site anyway. So I'm not really opposed to turning on allowTipSHA1InWant if it would be useful for users, but after Jonathan's patch I don't see how it would be. Given that, what would make me really happy is if github enables uploadpack.allowAnySHA1InWant. That would be useful for me, at least. Me too, BTW. If you're only interested in building / analyzing a specific SHA1 it's really a waste of network resources to fetch all of the history. -- Sebastian Schuberth
Re: Possible bug in includeIf / conditional includes
On 2017-05-10 19:00, raphael.st...@gmail.com wrote: Current configuration which finds the conditional configuration. ~/.gitconfig [includeIf "gitdir:~/Work/git-repos/oss/"] path = ~/Work/git-repos/oss/.oss-gitconfig Expected configuration which doesn't find the conditional configuration: ~/.gitconfig [includeIf "gitdir:~/Work/git-repos/oss/"] path = .oss-gitconfig My guess is, because includeIf might contain other conditionals than "gitdir", the generic convention is to always use an absolute path for "path". -- Sebastian Schuberth
