[PATCH v5] commit: add a commit.verbose config variable

[Pranit Bauva]

Add commit.verbose configuration variable as a convenience for those
who always prefer --verbose.

Signed-off-by: Pranit Bauva 
Mentored-by: Eric Sunshine 

---
The previous versions of this patch are:
 - [v4] $gmane/288652
 - [v3] $gmane/288634
 - [v2] $gmane/288569
 - [v1] $gmane/287540

The changes with respect to the last version are :
 - Use the in built check-for-diff "editor" instead of cat and breaking
   the code unnecessarily.
 - Add test to check whether the verbose option did not break status.
---
 Documentation/config.txt |  4 
 Documentation/git-commit.txt |  3 ++-
 builtin/commit.c |  4 
 t/t7507-commit-verbose.sh| 18 ++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 01cca0a..9b93f6c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1110,6 +1110,10 @@ commit.template::
"`~/`" is expanded to the value of `$HOME` and "`~user/`" to the
specified user's home directory.
 
+commit.verbose::
+   A boolean to specify whether to always include the verbose option
+   with `git commit`. See linkgit:git-commit[1].
+
 credential.helper::
Specify an external helper to be called when a username or
password credential is needed; the helper may consult external
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 9ec6b3c..d474226 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -290,7 +290,8 @@ configuration variable documented in linkgit:git-config[1].
what changes the commit has.
Note that this diff output doesn't have its
lines prefixed with '#'. This diff will not be a part
-   of the commit message.
+   of the commit message. See the `commit.verbose` configuration
+   variable in linkgit:git-config[1].
 +
 If specified twice, show in addition the unified diff between
 what would be committed and the worktree files, i.e. the unstaged
diff --git a/builtin/commit.c b/builtin/commit.c
index b3bd2d4..e0b96231 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1505,6 +1505,10 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
}
+   if (!strcmp(k, "commit.verbose")) {
+   verbose = git_config_bool(k, v);
+   return 0;
+   }
 
status = git_gpg_config(k, v, NULL);
if (status)
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 2ddf28c..bb7ce7c 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -96,4 +96,22 @@ test_expect_success 'verbose diff is stripped out with set 
core.commentChar' '
test_i18ngrep "Aborting commit due to empty commit message." err
 '
 
+test_expect_success 'commit.verbose true and --verbose omitted' '
+   test_config commit.verbose true &&
+   ! git status | grep "*diff --git" &&
+   git commit --amend
+'
+
+test_expect_success 'commit.verbose true and --no-verbose' '
+   test_must_fail git -c commit.verbose=true commit --amend --no-verbose
+'
+
+test_expect_success 'commit.verbose false and --verbose' '
+   git -c commit.verbose=false commit --amend --verbose
+'
+
+test_expect_success 'commit.verbose false and --verbose omitted' '
+   test_must_fail git -c commit.verbose=false commit --amend
+'
+
 test_done

--
https://github.com/git/git/pull/205
--
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/GSoC] Introduction

[Sidhant Sharma]

Hi everyone!

I am Sidhant Sharma, from Delhi, India. I'm a third year Software Engineering
student at Delhi Technological University. I am looking to contribute to
Git via GSoC 2016. I have also worked on one of the microprojects [1]. I've
been using git for nearly two years now, and continue to be surprised by the
vast number of features this powerful DVCS possesses. I want to contribute to
Git because it has become a daily-use tool for me and it feels exciting to
be a part of the community that makes effective collaborative development
possible.

I would like to work on the project titled 'Git Beginner mode', and have been
reading up the discussions that took place regarding this [2]. The reason I wish
to take this project in particular is that when I initially started out with
Git, and was still discovering how things really worked, I sometimes felt the
need for some sort of safety-latch to keep me from making destructive and/or
irreversible changes. So, this project gives me the opportunity to implement
something on these lines for the future beginners. I believe a lot of discussion
on the idea is due. I'm reading up on the commands that were mentioned on the
project page to better understand what the project entails, and trying to design
a solution for this, without making git harder to use or getting in the user's
learning. I would really appreciate your comments, suggestions and critique on
this.

Thanks and regards,
Sidhant Sharma

[1]: http://thread.gmane.org/gmane.comp.version-control.git/288035
[2]: http://thread.gmane.org/gmane.comp.version-control.git/285893/focus=286613
--
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 v7 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 11, 2016 at 10:21 AM, Paul Tan  wrote:
> Stepping back a bit, the only reason why we introduced opt_autostash =
> -1 as a possible value is because we need to detect if
> --[no-]autostash is being used with git-merge. I consider that a
> kludge, because if git-merge supports --autostash as well (possibly in
> the future), then we will not need this -1 value.

No, there is one more reason for which opt_autostash = -1 is required.
When user calls "git pull --rebase" then config_autostash value will
be used to perform --[no-]autostash task but if user calls "git pull
--rebase --[no-]autostash" then config_autostash value should not be
read at all as this option is supposed to override config_autostash
value. So if opt_autostash defaults to 0 then how will the code
understand if "--[no-]autostash" flag is passed or not?

As per the current patch, the value opt_autostash = 0 or 1  tells us
that the user has explicitly asked for --no-autostash or --autostash
respectively, and -1 value tells us that user has not specified
anything and thus we should read config_autostash value to perform
--[no-]autostash.

One way to do this was to read rebase.autoStash before parse_options(),
but now  as we have introduced a callback function git_pull_config(),
reading this config variable before parse_option() will now require
calling git_config(git_pull_config, NULL) before parse_option() and
doing opt_autostash = config_autostash there only.This may lead to
some problems (I'm not sure of that), as git_config() reads many other
config variables too.

Thanks,
Mehul
--
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: handle "-" as abbreviation of '@{-1}'

[Javier Domingo Cansino]

dash is usually used for representing stdin / stdout as a file. I
think this could drive to error... but I would agree with transforming
-h1 to @{-1} or -h2 to @{-2} (-h representing head).

I do agree however that all those signs are thought with american
keyboards in mind. All those punctuation marks are usually hard to
type in other keyboards, and -h1 is way simpler than HEAD~ or @{-1}

This links provides an example of my worry:
http://stackoverflow.com/questions/15270970/is-it-possible-to-git-diff-a-file-against-standard-input

On Sat, Mar 12, 2016 at 2:11 AM, Senorsen  wrote:
>
> Currently it just replace "-" in argv[] into "@{-1}".
>
> For example,
>
> git diff -
>
> equals to
>
> git diff @{-1}
>
> Signed-off-by: Senorsen 
> ---
> Notes:
> Hello everyone, I'm Zhang Sen, a college student from Zhejiang University
> in China, and this is a patch for the microproject of GSoC 2016. I'm
> looking forward to contributing to Git and participating in GSoC 2016.
>
> I have learnt some rules and guides from the documents, and carefully
> wrote this small patch, according to other code from git.
>
> Thanks a lot!
>
>  builtin/diff.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index 52c98a9..c110141 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -389,6 +389,11 @@ int cmd_diff(int argc, const char **argv, const char 
> *prefix)
> }
> }
>
> +   for (i = 0; i < argc; i++) {
> +   if (!strcmp(argv[i], "-"))
> +   argv[i] = "@{-1}";
> +   }
> +
> for (i = 0; i < rev.pending.nr; i++) {
> struct object_array_entry *entry = [i];
> struct object *obj = entry->item;
> --
> 2.7.0
>
> --
> 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




-- 
Javier Domingo Cansino
--
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: handle "-" as abbreviation of '@{-1}'

[Senorsen]

Currently it just replace "-" in argv[] into "@{-1}".

For example,

git diff -

equals to

git diff @{-1}

Signed-off-by: Senorsen 
---
Notes:
Hello everyone, I'm Zhang Sen, a college student from Zhejiang University 
in China, and this is a patch for the microproject of GSoC 2016. I'm 
looking forward to contributing to Git and participating in GSoC 2016.

I have learnt some rules and guides from the documents, and carefully 
wrote this small patch, according to other code from git.

Thanks a lot!

 builtin/diff.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/builtin/diff.c b/builtin/diff.c
index 52c98a9..c110141 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -389,6 +389,11 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
}
}
 
+   for (i = 0; i < argc; i++) {
+   if (!strcmp(argv[i], "-"))
+   argv[i] = "@{-1}";
+   }
+
for (i = 0; i < rev.pending.nr; i++) {
struct object_array_entry *entry = [i];
struct object *obj = entry->item;
-- 
2.7.0

--
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[2]: Possible bug: --ext-diff ignored with --cc in git log

[Vadim Zeitlin]

On Fri, 11 Mar 2016 10:20:42 -0800 Junio C Hamano  wrote:

JCH> Vadim Zeitlin  writes:
JCH> 
JCH> >  Thank you for your reply, Junio, I hadn't realized that --cc was 
dependent
JCH> > on textual diff output format before, but now I understand why it can't
JCH> > respect --ext-diff.
JCH> 
JCH> Having established that, I should also add that "--cc fundamentally
JCH> is incompatible with --ext-diff" does not justify that
JCH> "--cc when given with --ext-diff just ignores and uses the usual
JCH> diff".
JCH> 
JCH> An equally (or even more) valid consequence could have been to
JCH> disable "--cc" processing for paths that would trigger an external
JCH> diff driver.

 FWIW I agree that this would make more sense than the current behaviour.
But it still wouldn't be ideal if disabling "--cc" meant not showing any
output for these files at all, we still want to know that the file has been
modified as part of the commit, even if we don't care about its contents.

JCH> After all, the user told us that the contents would not compare well
JCH> with the usual "diff"; we know that "--cc" output that summarizes the
JCH> usual diff output is useless.

 This is so logical that it made me check how did "--cc" behave with the
binary files because this argument seems to apply perfectly well to them
too. And (unsurprisingly?) it already works just fine with them:

# I have alias g=git and I also suppress all successful output
$ g init
$ echo 'Binary\0file' > binary
$ g add binary
$ g commit -m 'Added'
$ echo '2nd line' >> binary
$ g commit -a -m 'Added 2nd line'
$ g checkout -b another HEAD~
$ echo 'another line' >> binary
$ g commit -a -m 'Added another line'
$ g checkout master
$ g merge
warning: Cannot merge binary files: binary (HEAD vs. another)
Auto-merging binary
CONFLICT (content): Merge conflict in binary
Automatic merge failed; fix conflicts and then commit the result.
$ vi binary # combine both versions
$ g commit
$ g show # finally I can show what all this is about
commit d30ae002cb52974228d50723fc8c9d7077e760da
Merge: ae542d2 3204f35
Author: Vadim Zeitlin 
Date:   Sat Mar 12 01:57:55 2016 +0100

Merge branch 'another'

diff --cc binary
index 31499e2,1730dfd..1eda50a
Binary files differ

So it looks like it shouldn't be too difficult to make it also output
"Files using custom diff viewer differ", what do you think?

JCH> For example, we could also ignore what external diff driver
JCH> produces in this case (as we know it won't be producing an
JCH> appropriate input to the "--cc" post-processing), and pretend
JCH> as if comparing an old version of foo.sln with a new version of
JCH> foo.sln produced a diff like this:
JCH> 
JCH> diff --git a/foo.sln b/foo.sln
JCH> index d7ff46e,b829410
JCH> --- a/foo.sln
JCH> +++ b/foo.sln
JCH> @@ 1,1 @@
JCH> -d7ff46ec4a016c6ab7d233b9d4a196ecde623528  - generated file
JCH> +b829410f6da0afc14353b4621d2fdf874181a9f7  - generated file
JCH> 
JCH> then you might see in a merge that merges two versions of foo.sln
JCH> and result in another version of foo.sln in your "--cc" output a
JCH> hunk that is like this:
JCH> 
JCH> diff --cc foo.sln
JCH> index d7ff46e,6c9aaa1..b829410
JCH> --- a/foo.sln
JCH> +++ b/foo.sln
JCH> @@@ 1,1 @@@
JCH> - d7ff46ec4a016c6ab7d233b9d4a196ecde623528  - generated file
JCH>  -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3  - generated file
JCH> ++b829410f6da0afc14353b4621d2fdf874181a9f7  - generated file
JCH> 
JCH> which would at least tell you that there was a merge, and if the
JCH> merge took the full contents of the file from one of the commits and
JCH> recorded as the result of the merge, then you wouldn't see them in
JCH> the "--cc" output.

 Interesting, but I admit I don't really see any advantage of showing the
SHA-1s here compared to what already happens with the binary files. Is
there anything I'm missing?

JCH> It happens that the above is fairly easily doable with today's Git
JCH> without any modification.  Here is how.
JCH> 
JCH> (1) Have this in your .git/config
JCH> 
JCH> [diff "uninteresting"]
JCH>textconv = /path/to/uninteresting-textconv-script
JCH> 
JCH> (2) Mark your .sln paths as uninteresting in your .gitattributes
JCH> 
JCH> *.sln  diff=uninteresting
JCH> 
JCH> (3) Have this textconv filter in /path/to/uninteresting-textconv-script
JCH> 
JCH> #!/bin/sh
JCH> printf "%s generated file\n" "$(sha1sum <"$1")"

 This is really ingenious, thanks! I'm probably indeed going to put this in
place at least for now for our mail notification script because it's just
too annoying to receive emails with thousands of lines of diffs to the
generated files.

 But I still think that it would make sense for 

Re: [RFC/PATCH] clone: add `--shallow-submodules` flag

[Stefan Beller]

On Fri, Mar 11, 2016 at 4:41 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> When creating a shallow clone of a repository with submodules, the depth
>> argument does not influence the submodules, i.e. the submodules are done
>> as non-shallow clones. It is unclear what the best default is for the
>> depth of submodules of a shallow clone, so we need to have the possibility
>> to do all kinds of combinations:
>>
>> * shallow super project with shallow submodules
>>   e.g. build bots starting always from scratch. They want to transmit
>>   the least amount of network data as well as using the least amount
>>   of space on their hard drive.
>> * shallow super project with unshallow submodules
>>   e.g. The superproject is just there to track a collection of repositories
>>   and it is not important to have the relationship between the repositories
>>   intact. However the history of the individual submodules matter.
>> * unshallow super project with shallow submodules
>>   e.g. The superproject is the actual project and the submodule is a
>>   library which is rarely touched.
>>
>> The new switch to select submodules to be shallow or unshallow supports
>> all of these three cases.
>
> I think something like this is necessary to prime the well, but the
> more important (and intereseting) bit is how this shallowness is
> going to be maintained and carried forward across the future updates
> to the top-level supermodule.  A submodule that was cloned at depth=1
> initially along with its supermodule when the latter was initially
> cloned does not have to be indefinitely kept at depth=1, and there
> would be a lot of creative ways to make it useful, but the creative
> and useful logic would need a piece of information to tell the
> future "submodule update" why the submodule repository is shallow to
> take into account, I would imagine.
>
> It is somewhat curious that there is no hint left in the submodule
> repositories (e.g. their configfile) that they are originally
> created with an explicit user request "I said that I want these
> submodules to be cloned with depth=1", from that point of view.

Why is it interesting for submodules but not for standard repositories?
If I clone a repository without submodules, it is also not recorded
that I cloned with an explicit depth=1. If you fetch, you may end up with
a deeper history as git fetch doesn't do a "reshallow" to the configured
depth.

As the depth can easily change I view depth as a measure which is
only valid at moment in time, after the operation succeeded we rather
want to talk about the cut off points which were introduced by the
shallow operation? And these are kept as is by default which is sane.
--
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] clone: add `--shallow-submodules` flag

[Junio C Hamano]

Stefan Beller  writes:

> When creating a shallow clone of a repository with submodules, the depth
> argument does not influence the submodules, i.e. the submodules are done
> as non-shallow clones. It is unclear what the best default is for the
> depth of submodules of a shallow clone, so we need to have the possibility
> to do all kinds of combinations:
>
> * shallow super project with shallow submodules
>   e.g. build bots starting always from scratch. They want to transmit
>   the least amount of network data as well as using the least amount
>   of space on their hard drive.
> * shallow super project with unshallow submodules
>   e.g. The superproject is just there to track a collection of repositories
>   and it is not important to have the relationship between the repositories
>   intact. However the history of the individual submodules matter.
> * unshallow super project with shallow submodules
>   e.g. The superproject is the actual project and the submodule is a
>   library which is rarely touched.
>
> The new switch to select submodules to be shallow or unshallow supports
> all of these three cases.

I think something like this is necessary to prime the well, but the
more important (and intereseting) bit is how this shallowness is
going to be maintained and carried forward across the future updates
to the top-level supermodule.  A submodule that was cloned at depth=1
initially along with its supermodule when the latter was initially
cloned does not have to be indefinitely kept at depth=1, and there
would be a lot of creative ways to make it useful, but the creative
and useful logic would need a piece of information to tell the
future "submodule update" why the submodule repository is shallow to
take into account, I would imagine.

It is somewhat curious that there is no hint left in the submodule
repositories (e.g. their configfile) that they are originally
created with an explicit user request "I said that I want these
submodules to be cloned with depth=1", from that point of view.
--
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/PATCH] clone: add `--shallow-submodules` flag

[Stefan Beller]

When creating a shallow clone of a repository with submodules, the depth
argument does not influence the submodules, i.e. the submodules are done
as non-shallow clones. It is unclear what the best default is for the
depth of submodules of a shallow clone, so we need to have the possibility
to do all kinds of combinations:

* shallow super project with shallow submodules
  e.g. build bots starting always from scratch. They want to transmit
  the least amount of network data as well as using the least amount
  of space on their hard drive.
* shallow super project with unshallow submodules
  e.g. The superproject is just there to track a collection of repositories
  and it is not important to have the relationship between the repositories
  intact. However the history of the individual submodules matter.
* unshallow super project with shallow submodules
  e.g. The superproject is the actual project and the submodule is a
  library which is rarely touched.

The new switch to select submodules to be shallow or unshallow supports
all of these three cases.

It is easy to transition from the first to the second case by just
unshallowing the submodules (`git submodule foreach git fetch
--unshallow`), but it is not possible to transition from the second to the
first case (as we wouldd have already transmitted the non shallow over
the network). That is why we want to make the first case the default in
case of a shallow super project. This leads to the inconvenience in the
second case with the shallow super project and unshallow submodules,
as you need to pass `--no-shallow-submodules`.

Signed-off-by: Stefan Beller 
---
A few notes:
 * This applies on top of sb/submodule-parallel-update
 * I am aware of the current release cycle, and I ought to not add shiny
   new features. But scanning the list revealed no bugs I could jump at to fix.
 * Lars made some unit tests for a very similar case a few weeks ago, but they
   were not applied as there was no intention to fix it. So I am hoping we can
   reuse some of these tests for this patch.
 * Currently I have the opinion that thinking about (un)shallow projects should
   be rather binary.  Either you want the full history or you want as least as
   possible. (Who wants to have a --depth 42? Some people have argued that you
   can use the depth to avoid large binaries which were part of the history in
   the past. But I'd counter that argument by pointing out the --depth argument
   is a workaround for such a use case. What you really want in that situation
   is a setting to clone history since a certain point in time/DAG, or a setting
   to clone up to a certain depth such that the filesize of the packed objects
   is smaller than some threshold.)
   
   If we were to allow specifying the depth for submodules, we'd need to discuss
   how to specify them for individual submodules i.e. clone submodule A with
   depth 4 and submodule B with depth 10. but that problem is solved easier
   by first doing a shallow clone with depth 1 of all submodules and then deepen
   them individually.
   
   So binary shallowness (depth = 1 or infinity) it is.

Thanks,
Stefan

 Documentation/git-clone.txt | 13 ++---
 builtin/clone.c |  6 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6db7b6d..20a4577 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,8 +14,8 @@ SYNOPSIS
  [-o ] [-b ] [-u ] [--reference ]
  [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--jobs ] [--] 
- []
+ [--recursive | --recurse-submodules] [--[no-]shallow-submodules]
+ [--jobs ] [--]  []
 
 DESCRIPTION
 ---
@@ -190,7 +190,11 @@ 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. This implies `--shallow-submodules`. If
+   you want to have a shallow clone, but full submodules, also pass
+   `--no-shallow-submodules`.
 
 --[no-]single-branch::
Clone only the history leading to the tip of a single branch,
@@ -214,6 +218,9 @@ objects from the source repository into a pack in the 
cloned repository.
repository does not have a worktree/checkout (i.e. if any of
`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
 
+--shallow-submodules::
+   All submodules which are cloned, will be shallow.
+
 --separate-git-dir=::
Instead of placing the cloned repository where it is supposed
to be, place the cloned repository at the specified directory,
diff --git a/builtin/clone.c 

Re: [PATCH v2 05/10] config: drop git_config_early

[Junio C Hamano]

Jeff King  writes:

> There are no more callers, and it's a rather confusing
> interface. This could just be folded into
> git_config_with_options(), but for the sake of readability,
> we'll leave it as a separate (static) helper function.

Yay!

;-)
--
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 06/10] setup: refactor repo format reading and verification

[Jeff King]

When we want to know if we're in a git repository of
reasonable vintage, we can call check_repository_format_gently(),
which does three things:

  1. Reads the config from the .git/config file.

  2. Verifies that the version info we read is sane.

  3. Writes some global variables based on this.

There are a few things we could improve here.

One is that steps 1 and 3 happen together. So if the
verification in step 2 fails, we still clobber the global
variables. This is especially bad if we go on to try another
repository directory; we may end up with a state of mixed
config variables.

The second is there's no way to ask about the repository
version for anything besides the main repository we're in.
git-init wants to do this, and it's possible that we would
want to start doing so for submodules (e.g., to find out
which ref backend they're using).

We can improve both by splitting the first two steps into
separate functions. Now check_repository_format_gently()
calls out to steps 1 and 2, and does 3 only if step 2
succeeds.

Note that the public interface for read_repository_format()
and what check_repository_format_gently() needs from it are
not quite the same, leading us to have an extra
read_repository_format_1() helper. The extra needs from
check_repository_format_gently() will go away in a future
patch, and we can simplify this then to just the public
interface.

Signed-off-by: Jeff King 
---
 cache.h |  24 +
 setup.c | 118 +++-
 2 files changed, 103 insertions(+), 39 deletions(-)

diff --git a/cache.h b/cache.h
index 867e73e..4fc42b5 100644
--- a/cache.h
+++ b/cache.h
@@ -750,6 +750,30 @@ extern int grafts_replace_parents;
 extern int repository_format_version;
 extern int repository_format_precious_objects;
 
+struct repository_format {
+   int version;
+   int precious_objects;
+   int is_bare;
+   char *work_tree;
+   struct string_list unknown_extensions;
+};
+
+/*
+ * Read the repository format characteristics from the config file "path" into
+ * "format" struct. Returns the numeric version. On error, -1 is returned,
+ * format->version is set to -1, and all other fields in the struct are
+ * undefined.
+ */
+int read_repository_format(struct repository_format *format, const char *path);
+
+/*
+ * Verify that the repository described by repository_format is something we
+ * can read. If it is, return 0. Otherwise, return -1, and "err" will describe
+ * any errors encountered.
+ */
+int verify_repository_format(const struct repository_format *format,
+struct strbuf *err);
+
 /*
  * Check the repository format version in the path found in get_git_dir(),
  * and die if it is a version we don't understand. Generally one would
diff --git a/setup.c b/setup.c
index a6013e6..8aa49a9 100644
--- a/setup.c
+++ b/setup.c
@@ -5,7 +5,6 @@
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
 static int work_tree_config_is_bogus;
-static struct string_list unknown_extensions = STRING_LIST_INIT_DUP;
 
 /*
  * The input parameter must contain an absolute path, and it must already be
@@ -370,12 +369,13 @@ void setup_work_tree(void)
initialized = 1;
 }
 
-static int check_repo_format(const char *var, const char *value, void *cb)
+static int check_repo_format(const char *var, const char *value, void *vdata)
 {
+   struct repository_format *data = vdata;
const char *ext;
 
if (strcmp(var, "core.repositoryformatversion") == 0)
-   repository_format_version = git_config_int(var, value);
+   data->version = git_config_int(var, value);
else if (skip_prefix(var, "extensions.", )) {
/*
 * record any known extensions here; otherwise,
@@ -385,61 +385,104 @@ static int check_repo_format(const char *var, const char 
*value, void *cb)
if (!strcmp(ext, "noop"))
;
else if (!strcmp(ext, "preciousobjects"))
-   repository_format_precious_objects = 
git_config_bool(var, value);
+   data->precious_objects = git_config_bool(var, value);
else
-   string_list_append(_extensions, ext);
+   string_list_append(>unknown_extensions, ext);
}
return 0;
 }
 
+static int read_repository_format_1(struct repository_format *, config_fn_t,
+   const char *);
+
 static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 {
struct strbuf sb = STRBUF_INIT;
-   const char *repo_config;
+   struct strbuf err = STRBUF_INIT;
+   struct repository_format candidate;
config_fn_t fn;
-   int ret = 0;
-
-   string_list_clear(_extensions, 0);
 
if (get_common_dir(, gitdir))
fn = check_repo_format;
else
fn = 

[PATCH v2 04/10] check_repository_format_gently: stop using git_config_early

[Jeff King]

There's a chicken-and-egg problem with using the regular
git_config during the repository setup process. We get
around it here by using a special interface that lets us
specify the per-repo config, and avoid calling
git_pathdup().

But this interface doesn't actually make sense. It will look
in the system and per-user config, too; we definitely would
not want to accept a core.repositoryformatversion from
there.

The git_config_from_file interface is a better match, as it
lets us look at a single file.

Signed-off-by: Jeff King 
---
 setup.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/setup.c b/setup.c
index a02932b..a6013e6 100644
--- a/setup.c
+++ b/setup.c
@@ -409,15 +409,10 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
repo_config = sb.buf;
 
/*
-* git_config() can't be used here because it calls git_pathdup()
-* to get $GIT_CONFIG/config. That call will make setup_git_env()
-* set git_dir to ".git".
-*
-* We are in gitdir setup, no git dir has been found useable yet.
-* Use a gentler version of git_config() to check if this repo
-* is a good one.
+* Ignore return value; for historical reasons, we must treat a missing
+* config file as a noop (git-init relies on this).
 */
-   git_config_early(fn, NULL, repo_config);
+   git_config_from_file(fn, repo_config, NULL);
if (GIT_REPO_VERSION_READ < repository_format_version) {
if (!nongit_ok)
die ("Expected git repo version <= %d, found %d",
-- 
2.8.0.rc2.328.g39e2a47

--
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 05/10] config: drop git_config_early

[Jeff King]

There are no more callers, and it's a rather confusing
interface. This could just be folded into
git_config_with_options(), but for the sake of readability,
we'll leave it as a separate (static) helper function.

Signed-off-by: Jeff King 
---
 Documentation/technical/api-config.txt |  7 ---
 cache.h|  1 -
 config.c   | 12 
 3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index 0d8b99b..20741f3 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -63,13 +63,6 @@ parse for configuration, rather than looking in the usual 
files. Regular
 Specify whether include directives should be followed in parsed files.
 Regular `git_config` defaults to `1`.
 
-There is a special version of `git_config` called `git_config_early`.
-This version takes an additional parameter to specify the repository
-config, instead of having it looked up via `git_path`. This is useful
-early in a Git program before the repository has been found. Unless
-you're working with early setup code, you probably don't want to use
-this.
-
 Reading Specific Files
 --
 
diff --git a/cache.h b/cache.h
index fdb9583..867e73e 100644
--- a/cache.h
+++ b/cache.h
@@ -1535,7 +1535,6 @@ extern void git_config(config_fn_t fn, void *);
 extern int git_config_with_options(config_fn_t fn, void *,
   struct git_config_source *config_source,
   int respect_includes);
-extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
 extern int git_parse_ulong(const char *, unsigned long *);
 extern int git_parse_maybe_bool(const char *);
 extern int git_config_int(const char *, const char *);
diff --git a/config.c b/config.c
index 9ba40bc..7ddb287 100644
--- a/config.c
+++ b/config.c
@@ -1188,11 +1188,12 @@ int git_config_system(void)
return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
 }
 
-int git_config_early(config_fn_t fn, void *data, const char *repo_config)
+static int do_git_config_sequence(config_fn_t fn, void *data)
 {
int ret = 0, found = 0;
char *xdg_config = xdg_config_home("config");
char *user_config = expand_user_path("~/.gitconfig");
+   char *repo_config = git_pathdup("config");
 
if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 
0)) {
ret += git_config_from_file(fn, git_etc_gitconfig(),
@@ -1228,6 +1229,7 @@ int git_config_early(config_fn_t fn, void *data, const 
char *repo_config)
 
free(xdg_config);
free(user_config);
+   free(repo_config);
return ret == 0 ? found : ret;
 }
 
@@ -1235,8 +1237,6 @@ int git_config_with_options(config_fn_t fn, void *data,
struct git_config_source *config_source,
int respect_includes)
 {
-   char *repo_config = NULL;
-   int ret;
struct config_include_data inc = CONFIG_INCLUDE_INIT;
 
if (respect_includes) {
@@ -1257,11 +1257,7 @@ int git_config_with_options(config_fn_t fn, void *data,
else if (config_source && config_source->blob)
return git_config_from_blob_ref(fn, config_source->blob, data);
 
-   repo_config = git_pathdup("config");
-   ret = git_config_early(fn, data, repo_config);
-   if (repo_config)
-   free(repo_config);
-   return ret;
+   return do_git_config_sequence(fn, data);
 }
 
 static void git_config_raw(config_fn_t fn, void *data)
-- 
2.8.0.rc2.328.g39e2a47

--
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 08/10] setup: unify repository version callbacks

[Jeff King]

Once upon a time, check_repository_format_gently would parse
the config with a single callback, and that callback would
set up a bunch of global variables. But now that we have
separate workdirs, we have to be more careful. Commit
31e26eb (setup.c: support multi-checkout repo setup,
2014-11-30) introduced a reduced callback which omits some
values like core.worktree. In the "main" callback we call
the reduced one, and then add back in the missing variables.

Now that we have split the config-parsing from the munging
of the global variables, we can do it all with a single
callback, and keep all of the "are we in a separate workdir"
logic together.

Signed-off-by: Jeff King 
---
 cache.h |  1 -
 setup.c | 65 +++--
 2 files changed, 23 insertions(+), 43 deletions(-)

diff --git a/cache.h b/cache.h
index 4fc42b5..7704bc6 100644
--- a/cache.h
+++ b/cache.h
@@ -1582,7 +1582,6 @@ extern void git_config_set_multivar_in_file(const char *, 
const char *, const ch
 extern int git_config_rename_section(const char *, const char *);
 extern int git_config_rename_section_in_file(const char *, const char *, const 
char *);
 extern const char *git_etc_gitconfig(void);
-extern int check_repository_format_version(const char *var, const char *value, 
void *cb);
 extern int git_env_bool(const char *, int);
 extern unsigned long git_env_ulong(const char *, unsigned long);
 extern int git_config_system(void);
diff --git a/setup.c b/setup.c
index 8aa49a9..fbe7ec1 100644
--- a/setup.c
+++ b/setup.c
@@ -388,27 +388,26 @@ static int check_repo_format(const char *var, const char 
*value, void *vdata)
data->precious_objects = git_config_bool(var, value);
else
string_list_append(>unknown_extensions, ext);
+   } else if (strcmp(var, "core.bare") == 0) {
+   data->is_bare = git_config_bool(var, value);
+   } else if (strcmp(var, "core.worktree") == 0) {
+   if (!value)
+   return config_error_nonbool(var);
+   data->work_tree = xstrdup(value);
}
return 0;
 }
 
-static int read_repository_format_1(struct repository_format *, config_fn_t,
-   const char *);
-
 static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 {
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
struct repository_format candidate;
-   config_fn_t fn;
-
-   if (get_common_dir(, gitdir))
-   fn = check_repo_format;
-   else
-   fn = check_repository_format_version;
+   int has_common;
 
+   has_common = get_common_dir(, gitdir);
strbuf_addstr(, "/config");
-   read_repository_format_1(, fn, sb.buf);
+   read_repository_format(, sb.buf);
strbuf_release();
 
/*
@@ -432,36 +431,34 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
repository_format_version = candidate.version;
repository_format_precious_objects = candidate.precious_objects;
string_list_clear(_extensions, 0);
-   if (candidate.is_bare != -1) {
-   is_bare_repository_cfg = candidate.is_bare;
-   if (is_bare_repository_cfg == 1)
+   if (!has_common) {
+   if (candidate.is_bare != -1) {
+   is_bare_repository_cfg = candidate.is_bare;
+   if (is_bare_repository_cfg == 1)
+   inside_work_tree = -1;
+   }
+   if (candidate.work_tree) {
+   free(git_work_tree_cfg);
+   git_work_tree_cfg = candidate.work_tree;
inside_work_tree = -1;
-   }
-   if (candidate.work_tree) {
-   free(git_work_tree_cfg);
-   git_work_tree_cfg = candidate.work_tree;
-   inside_work_tree = -1;
+   }
+   } else {
+   free(candidate.work_tree);
}
 
return 0;
 }
 
-static int read_repository_format_1(struct repository_format *format,
-   config_fn_t fn, const char *path)
+int read_repository_format(struct repository_format *format, const char *path)
 {
memset(format, 0, sizeof(*format));
format->version = -1;
format->is_bare = -1;
string_list_init(>unknown_extensions, 1);
-   git_config_from_file(fn, path, format);
+   git_config_from_file(check_repo_format, path, format);
return format->version;
 }
 
-int read_repository_format(struct repository_format *format, const char *path)
-{
-   return read_repository_format_1(format, 
check_repository_format_version, path);
-}
-
 int verify_repository_format(const struct repository_format *format,
 struct strbuf *err)
 {
@@ -999,22 +996,6 @@ int git_config_perm(const char 

[PATCH v2 09/10] setup: drop repository_format_version global

[Jeff King]

Nobody reads this anymore, and they're not likely to; the
interesting thing is whether or not we passed
check_repository_format(), and possibly the individual
"extension" variables.

Signed-off-by: Jeff King 
---
 cache.h   | 1 -
 environment.c | 1 -
 setup.c   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/cache.h b/cache.h
index 7704bc6..bec6db5 100644
--- a/cache.h
+++ b/cache.h
@@ -747,7 +747,6 @@ extern int grafts_replace_parents;
  */
 #define GIT_REPO_VERSION 0
 #define GIT_REPO_VERSION_READ 1
-extern int repository_format_version;
 extern int repository_format_precious_objects;
 
 struct repository_format {
diff --git a/environment.c b/environment.c
index 8b8c8e8..d9e3861 100644
--- a/environment.c
+++ b/environment.c
@@ -25,7 +25,6 @@ int log_all_ref_updates = -1; /* unspecified */
 int warn_ambiguous_refs = 1;
 int warn_on_object_refname_ambiguity = 1;
 int ref_paranoia = -1;
-int repository_format_version;
 int repository_format_precious_objects;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
diff --git a/setup.c b/setup.c
index fbe7ec1..1314c17 100644
--- a/setup.c
+++ b/setup.c
@@ -428,7 +428,6 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
die("%s", err.buf);
}
 
-   repository_format_version = candidate.version;
repository_format_precious_objects = candidate.precious_objects;
string_list_clear(_extensions, 0);
if (!has_common) {
-- 
2.8.0.rc2.328.g39e2a47

--
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 10/10] verify_repository_format: mark messages for translation

[Jeff King]

These messages are human-readable and should be translated.

Signed-off-by: Jeff King 
---
 setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index 1314c17..09d3720 100644
--- a/setup.c
+++ b/setup.c
@@ -462,7 +462,7 @@ int verify_repository_format(const struct repository_format 
*format,
 struct strbuf *err)
 {
if (GIT_REPO_VERSION_READ < format->version) {
-   strbuf_addf(err, "Expected git repo version <= %d, found %d",
+   strbuf_addf(err, _("Expected git repo version <= %d, found %d"),
GIT_REPO_VERSION_READ, format->version);
return -1;
}
@@ -470,7 +470,7 @@ int verify_repository_format(const struct repository_format 
*format,
if (format->version >= 1 && format->unknown_extensions.nr) {
int i;
 
-   strbuf_addstr(err, "unknown repository extensions found:");
+   strbuf_addstr(err, _("unknown repository extensions found:"));
 
for (i = 0; i < format->unknown_extensions.nr; i++)
strbuf_addf(err, "\n\t%s",
-- 
2.8.0.rc2.328.g39e2a47
--
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 07/10] init: use setup.c's repo version verification

[Jeff King]

We check our templates to make sure they are from a
version of git we understand (otherwise we would init a
repository we cannot ourselves run in!). But our simple
integer check has fallen behind the times. Let's use the
helpers that setup.c provides to do it right.

Signed-off-by: Jeff King 
---
 builtin/init-db.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index e9b2256..d9934f3 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -95,6 +95,8 @@ static void copy_templates(const char *template_dir)
struct strbuf path = STRBUF_INIT;
struct strbuf template_path = STRBUF_INIT;
size_t template_len;
+   struct repository_format template_format;
+   struct strbuf err = STRBUF_INIT;
DIR *dir;
char *to_free = NULL;
 
@@ -121,17 +123,18 @@ static void copy_templates(const char *template_dir)
 
/* Make sure that template is from the correct vintage */
strbuf_addstr(_path, "config");
-   repository_format_version = 0;
-   git_config_from_file(check_repository_format_version,
-template_path.buf, NULL);
+   read_repository_format(_format, template_path.buf);
strbuf_setlen(_path, template_len);
 
-   if (repository_format_version &&
-   repository_format_version != GIT_REPO_VERSION) {
-   warning(_("not copying templates of "
-   "a wrong format version %d from '%s'"),
-   repository_format_version,
-   template_dir);
+   /*
+* No mention of version at all is OK, but anything else should be
+* verified.
+*/
+   if (template_format.version >= 0 &&
+   verify_repository_format(_format, ) < 0) {
+   warning(_("not copying templates from '%s': %s"),
+ template_dir, err.buf);
+   strbuf_release();
goto close_free_return;
}
 
-- 
2.8.0.rc2.328.g39e2a47

--
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 0/10] cleaning up check_repository_format_gently

[Jeff King]

This is a re-roll of:

  http://article.gmane.org/gmane.comp.version-control.git/288017

The changes from v1 are:

  - dropped original 10/10 that got rid of GIT_REPO_VERSION defines
(David mentioned that he's going to be expanding their use, so the
argument that they are not used does not make sense)

  - added new 10/10 marking messages for translation (suggested by Duy)

  - added int return value to new read_repository_format(); this is
redundant with the error value returned in the struct, but allows a
more idiomatic:

  if (read_repository_format(, file) < 0)
 ...

  - drop confusing comment from 06/10, in favor of a better explanation
in the commit message

  - fixed newline regression from v1 when printing out unknown
extensions. Note that because we switch from looping over warning()
or die() to sticking errors into a strbuf (which the caller then
feeds to warning/die), the format changed a bit.  Naively, it would
become:

  warning: unknown extension: foo
  unknown:extension: bar

but I turned it into the more pleasant:

  warning: unknown repository extensions found:
  foo
  bar

I think it would be better still if warning() was smart enough to
stick its prefix in front of all lines (like advise() does). But
that's outside the scope of this series. And it probably doesn't
matter much either way; this is not a message we'd expect anyone to
see routinely.

  [01/10]: setup: document check_repository_format()
  [02/10]: wrap shared_repository global in get/set accessors
  [03/10]: lazily load core.sharedrepository
  [04/10]: check_repository_format_gently: stop using git_config_early
  [05/10]: config: drop git_config_early
  [06/10]: setup: refactor repo format reading and verification
  [07/10]: init: use setup.c's repo version verification
  [08/10]: setup: unify repository version callbacks
  [09/10]: setup: drop repository_format_version global
  [10/10]: verify_repository_format: mark messages for translation

-Peff
--
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 01/10] setup: document check_repository_format()

[Jeff King]

This function's interface is rather enigmatic, so let's
document it further.

While we're here, let's also drop the return value. It will
always either be "0" or the function will die (consequently,
neither of its two callers bothered to check the return).

Signed-off-by: Jeff King 
---
 cache.h | 9 -
 setup.c | 4 ++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index b829410..02e38d1 100644
--- a/cache.h
+++ b/cache.h
@@ -747,7 +747,14 @@ extern int grafts_replace_parents;
 #define GIT_REPO_VERSION_READ 1
 extern int repository_format_version;
 extern int repository_format_precious_objects;
-extern int check_repository_format(void);
+
+/*
+ * Check the repository format version in the path found in get_git_dir(),
+ * and die if it is a version we don't understand. Generally one would
+ * set_git_dir() before calling this, and use it only for "are we in a valid
+ * repo?".
+ */
+extern void check_repository_format(void);
 
 #define MTIME_CHANGED  0x0001
 #define CTIME_CHANGED  0x0002
diff --git a/setup.c b/setup.c
index de1a2a7..b2f2e69 100644
--- a/setup.c
+++ b/setup.c
@@ -982,9 +982,9 @@ int check_repository_format_version(const char *var, const 
char *value, void *cb
return 0;
 }
 
-int check_repository_format(void)
+void check_repository_format(void)
 {
-   return check_repository_format_gently(get_git_dir(), NULL);
+   check_repository_format_gently(get_git_dir(), NULL);
 }
 
 /*
-- 
2.8.0.rc2.328.g39e2a47

--
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 02/10] wrap shared_repository global in get/set accessors

[Jeff King]

It would be useful to control access to the global
shared_repository, so that we can lazily load its config.
The first step to doing so is to make sure all access
goes through a set of functions.

This step is purely mechanical, and should result in no
change of behavior.

Signed-off-by: Jeff King 
---
 builtin/init-db.c | 24 
 cache.h   |  4 +++-
 environment.c | 13 -
 path.c| 10 +-
 setup.c   |  2 +-
 5 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 6223b7d..e9b2256 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -199,13 +199,13 @@ static int create_default_files(const char *template_path)
 
/* reading existing config may have overwrote it */
if (init_shared_repository != -1)
-   shared_repository = init_shared_repository;
+   set_shared_repository(init_shared_repository);
 
/*
 * We would have created the above under user's umask -- under
 * shared-repository settings, we would need to fix them up.
 */
-   if (shared_repository) {
+   if (get_shared_repository()) {
adjust_shared_perm(get_git_dir());
adjust_shared_perm(git_path_buf(, "refs"));
adjust_shared_perm(git_path_buf(, "refs/heads"));
@@ -369,7 +369,7 @@ int init_db(const char *template_dir, unsigned int flags)
 
create_object_directory();
 
-   if (shared_repository) {
+   if (get_shared_repository()) {
char buf[10];
/* We do not spell "group" and such, so that
 * the configuration can be read by older version
@@ -377,12 +377,12 @@ int init_db(const char *template_dir, unsigned int flags)
 * and compatibility values for PERM_GROUP and
 * PERM_EVERYBODY.
 */
-   if (shared_repository < 0)
+   if (get_shared_repository() < 0)
/* force to the mode value */
-   xsnprintf(buf, sizeof(buf), "0%o", -shared_repository);
-   else if (shared_repository == PERM_GROUP)
+   xsnprintf(buf, sizeof(buf), "0%o", 
-get_shared_repository());
+   else if (get_shared_repository() == PERM_GROUP)
xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_GROUP);
-   else if (shared_repository == PERM_EVERYBODY)
+   else if (get_shared_repository() == PERM_EVERYBODY)
xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY);
else
die("BUG: invalid value for shared_repository");
@@ -398,7 +398,7 @@ int init_db(const char *template_dir, unsigned int flags)
   "", and the last '%s%s' is the verbatim directory name. */
printf(_("%s%s Git repository in %s%s\n"),
   reinit ? _("Reinitialized existing") : _("Initialized 
empty"),
-  shared_repository ? _(" shared") : "",
+  get_shared_repository() ? _(" shared") : "",
   git_dir, len && git_dir[len-1] != '/' ? "/" : "");
}
 
@@ -493,8 +493,8 @@ int cmd_init_db(int argc, const char **argv, const char 
*prefix)
 * and we know shared_repository should always 
be 0;
 * but just in case we play safe.
 */
-   saved = shared_repository;
-   shared_repository = 0;
+   saved = get_shared_repository();
+   set_shared_repository(0);
switch 
(safe_create_leading_directories_const(argv[0])) {
case SCLD_OK:
case SCLD_PERMS:
@@ -506,7 +506,7 @@ int cmd_init_db(int argc, const char **argv, const char 
*prefix)
die_errno(_("cannot mkdir %s"), 
argv[0]);
break;
}
-   shared_repository = saved;
+   set_shared_repository(saved);
if (mkdir(argv[0], 0777) < 0)
die_errno(_("cannot mkdir %s"), 
argv[0]);
mkdir_tried = 1;
@@ -524,7 +524,7 @@ int cmd_init_db(int argc, const char **argv, const char 
*prefix)
}
 
if (init_shared_repository != -1)
-   shared_repository = init_shared_repository;
+   set_shared_repository(init_shared_repository);
 
/*
 * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR
diff --git a/cache.h b/cache.h
index 02e38d1..fdb9583 100644
--- a/cache.h
+++ b/cache.h
@@ -651,7 +651,6 @@ 

Re: Ability to remember last known good build

[Junio C Hamano]

Stefan Beller  writes:

>>> Any better way of accomplishing this?
>>
>> "test && git branch -f last-good"?
>
> Travis-CI enabled, tells me they're using Github and are distributed,
> so one contributor would want to know the last known good state of
> a remote, that others push to, without testing all commits locally.
>
> So maybe the question is better rephrased as: "How do we keep track of
> the last good state using the distributed nature of Git?"

I think Travis integration with GitHub lets the commits tested to be
annotated in the test status, so scanning the history from the tip
to find the latest one with the "good" test result would be how you
would find "the last passing one" if your workflow relies on the
Travis-GitHub combination.

I am not sure about "using the distributed nature" part.  That
depends on the way how the result of the Travis testing is reflected
on the GitHub side.  If it is done by adding a note or something
that can natively exported over "git fetch", that would be one way
to do so.
--
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: Letter of Intent!!

[Mr. Young Chio]

Esteem Partner,

I am Mr.Young Chio, I work with the China Merchants Bank, I wish to place your 
name as the beneficiary to Ten Million Three Hundred Thousand Dollars only due 
to the death of the depositor who died years ago along with her family.

However, I assure you that this transaction will be executed under a legitimate 
arrangement that will protect you from any breach of law both in your country 
and here in China once the fund is transferred to your bank account. The funds 
I am referring to is right here in my bank floating in suspense account, 
therefore I am solicit for your cooperation to be in collaboration with me to 
have this done. It will be transferred into an account you will provide any 
where in the world.

In acceptance to my proffer, I will need you to provide the following 
information urgently to enable the Attorney start the processing of all the 
relevant legal documents for the fund remittance.

1) Name:
2) Address:
3) Tel:
4) Age:
5) Sex:
6) Occupation
7) Status:

Waiting for your reply soon.

Yours Faithfully,

Mr. Young Chio
--
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: Sample pre-push hook can crash

[Stephen Morton]

Thanks for the information Junio. It is just interesting that although
the pre-push hook receives the remote_sha value from the server, it
does not get 'git merge-base $remote_sha $local_sha' which is what a
check that iterated over all outgoing commits would really need. (I'm
sure this is a simple-minded assessment. I don't pretend to have spent
even an order of magnitude less time working on git than you have. I'm
not trying to be foolishly arrogant.)

I do think it would be worth replacing the existing example with yours
because the existing example will crash anytime somebody's workspace
is up to date.

Stephen



On Fri, Mar 11, 2016 at 3:04 PM, Junio C Hamano  wrote:
> Stephen Morton  writes:
>
>> That is interesting, so in the case of a non-ff push, there is no way
>> for a pre-push hook to know what is being pushed in order to run?
>
> If you were up-to-date from the other side once:
>
> ---A---B---C
>
> and built one new commit on top:
>
> ---A---B---C---D
>
> when you attempt to push it out, there are various possibilities.
>
> An ff situation is simple.  They didn't do anything, so the hook
> gets "we'd be updating their ref from C to D", and "rev-list C..D"
> would tell you that you would need to make sure D is sane.
>
> If your push does not fast-forward, that would mean something has
> happened on the other side while you were working on D.  Perhaps
> they accepted another commit that you haven't seen:
>
> ---A---B---C---E
>
> and because you haven't fetched from them, even though the hook may
> say "we'd be updating their ref from E to D", you haven't heard of
> E, you do not know where it would be relative to the history you
> have:
>
>   E???
>
> ---A---B---C---D
>
> Or perhaps they themselves rewound their history and they now have
> this E at the tip:
>
> ---A---B---C
> \
>  E
>
> But again, because you do not yet know where E is relative to your
> history, you cannot say C is where you can stop checking your side
> of the history.
>
> Or perhaps they somehow obtained your D without you pushing into
> them (e.g. you pushed to the "next" tree and they fixed your commit
> and the result was merged there) and have something like this:
>
> ---A---B---C---D---E
>
> In this case, D is not even a new commit from their point of view,
> and updating their tip E with your old D would lose the fixup E they
> made for D, but again, you do not know what E is yet, you cannot say
> "they have this already, so there is no check I need to do".
>
> In summary, you cannot even say which ones you have need to be
> checked.
>
> If you _are_ using @{u} tracking, then you would know at least they
> used to have up to C in their repository to limit your check, but
> you cannot unconditionally say ref@{u}.. without making sure ref@{u}
> makes sense in the first place.
>
> An obvious alternative for the sample script would be to instead let
> the non-ff push pass the pre-push check (as you may have other
> arrangements to forbid non-ff pushes) without actually checking
> anything.  As this sample script is to serve as a sample, I think
> such an advanced consideration of loosening checks depending on the
> situation should be left to the end users.
>
--
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: Sample pre-push hook can crash

[Junio C Hamano]

Stephen Morton  writes:

> That is interesting, so in the case of a non-ff push, there is no way
> for a pre-push hook to know what is being pushed in order to run?

If you were up-to-date from the other side once:

---A---B---C

and built one new commit on top:

---A---B---C---D

when you attempt to push it out, there are various possibilities.

An ff situation is simple.  They didn't do anything, so the hook
gets "we'd be updating their ref from C to D", and "rev-list C..D"
would tell you that you would need to make sure D is sane.

If your push does not fast-forward, that would mean something has
happened on the other side while you were working on D.  Perhaps
they accepted another commit that you haven't seen:

---A---B---C---E

and because you haven't fetched from them, even though the hook may
say "we'd be updating their ref from E to D", you haven't heard of
E, you do not know where it would be relative to the history you
have:

  E???

---A---B---C---D

Or perhaps they themselves rewound their history and they now have
this E at the tip:

---A---B---C
\
 E

But again, because you do not yet know where E is relative to your
history, you cannot say C is where you can stop checking your side
of the history.

Or perhaps they somehow obtained your D without you pushing into
them (e.g. you pushed to the "next" tree and they fixed your commit
and the result was merged there) and have something like this:

---A---B---C---D---E

In this case, D is not even a new commit from their point of view,
and updating their tip E with your old D would lose the fixup E they
made for D, but again, you do not know what E is yet, you cannot say
"they have this already, so there is no check I need to do".

In summary, you cannot even say which ones you have need to be
checked.

If you _are_ using @{u} tracking, then you would know at least they
used to have up to C in their repository to limit your check, but
you cannot unconditionally say ref@{u}.. without making sure ref@{u}
makes sense in the first place.

An obvious alternative for the sample script would be to instead let
the non-ff push pass the pre-push check (as you may have other
arrangements to forbid non-ff pushes) without actually checking
anything.  As this sample script is to serve as a sample, I think
such an advanced consideration of loosening checks depending on the
situation should be left to the end users.

--
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: Ability to remember last known good build

[Randall S. Becker]

On March 11, 2016 1:08 PM Junio C Hamano wrote:
> "Pedroso, Osiris"  writes:
> 
> > I participate in an open source project that any pull merge is accepted,
no
> matter what.
> >
> > This makes for lots of broken builds, even though we do have Travis-CI
> enabled on the project, because people will merge a request before even
the
> build is complete.
> >
> > Therefore, I would like to remember the id of the commit of the last
> successful build. This would be updated by the Travis-CI script itself
upon a
> successful build.
> >
> > I imagine best option would be to merge master to a certain branch named
> "Last_known_Linux_build" or "Last_known_Windows_build" or even
> "Last_known_build_all_tests_passing".
> >
> > I am new to git, but some other experienced co-volunteers tell me that
it
> may not be possible due to authentication issues.
> >
> > Any better way of accomplishing this?
> 
> "test && git branch -f last-good"?

I think semantically a last-good tag might be another option, unless you are
applying build fixes to a last-good topic branch. You also have the option
of adding content to the tag describing the build reason, engine used, etc.
See git tag --help. I have used that in a Jenkins environment putting the
tag move in the step following a build (failure does not execute the step so
the last-good build tag stays where it is).

Cheers,
Randall

-- Brief whoami: NonStop developer since approximately
UNIX(421664400)/NonStop(2112884442)
-- In my real life, I talk too much.




--
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: Possible bug: --ext-diff ignored with --cc in git log

[Jeff King]

On Fri, Mar 11, 2016 at 11:08:32AM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> >> It happens that the above is fairly easily doable with today's Git
> >> without any modification.  Here is how.
> >> [...]
> >
> > I think an even easier way is:
> >
> >   git log --cc --raw
> >
> > I know that is somewhat beside the point you are making, which is how we
> > should handle "--cc" with ext-diff. But I would much rather have us
> > show nothing for that case, and let the user turn on "--raw", than to
> > invent a diff-looking format that does not actually represent the file
> > contents.
> 
> Sorry, but I am not sure where you are trying to go with this.
> 
> I understand that the original issue was that Vadim wants to
> suppress reams of differences for _some_ paths but still wants to
> benefit from the textual summarized diff for all the other paths.
> Giving "--raw" would be global, and would affect other paths, no?

Ah, sorry, I thought the problem was the opposite: that there was no
output for ext-diff paths, and we needed to add something back in. Doing
"--raw" is a much easier way than textconv of the "add back in" part,
but it does not suppress the ordinary combined diff.

-Peff
--
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: Possible bug: --ext-diff ignored with --cc in git log

[Junio C Hamano]

Jeff King  writes:

>> It happens that the above is fairly easily doable with today's Git
>> without any modification.  Here is how.
>> [...]
>
> I think an even easier way is:
>
>   git log --cc --raw
>
> I know that is somewhat beside the point you are making, which is how we
> should handle "--cc" with ext-diff. But I would much rather have us
> show nothing for that case, and let the user turn on "--raw", than to
> invent a diff-looking format that does not actually represent the file
> contents.

Sorry, but I am not sure where you are trying to go with this.

I understand that the original issue was that Vadim wants to
suppress reams of differences for _some_ paths but still wants to
benefit from the textual summarized diff for all the other paths.
Giving "--raw" would be global, and would affect other paths, no?
--
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: Possible bug: --ext-diff ignored with --cc in git log

[Jeff King]

On Fri, Mar 11, 2016 at 10:20:42AM -0800, Junio C Hamano wrote:

> diff --cc foo.sln
> index d7ff46e,6c9aaa1..b829410
> --- a/foo.sln
> +++ b/foo.sln
> @@@ 1,1 @@@
> - d7ff46ec4a016c6ab7d233b9d4a196ecde623528  - generated file
>  -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3  - generated file
> ++b829410f6da0afc14353b4621d2fdf874181a9f7  - generated file
> 
> which would at least tell you that there was a merge, and if the
> merge took the full contents of the file from one of the commits and
> recorded as the result of the merge, then you wouldn't see them in
> the "--cc" output.
> 
> It happens that the above is fairly easily doable with today's Git
> without any modification.  Here is how.
> [...]

I think an even easier way is:

  git log --cc --raw

I know that is somewhat beside the point you are making, which is how we
should handle "--cc" with ext-diff. But I would much rather have us
show nothing for that case, and let the user turn on "--raw", than to
invent a diff-looking format that does not actually represent the file
contents.

-Peff
--
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] commit: add a commit.verbose config variable

[Philip Oakley]

From: "Pranit Bauva" 
On Fri, Mar 11, 2016 at 4:31 AM, Eric Sunshine  
wrote:



Add commit.verbose configuration variable as a convenience
for those who always prefer --verbose.

or something.


Sure!


As a convenience to reviewers, please use this area below the "---"
line to provide links and explain what changed since the previous
round rather than doing so in a separate email.


Actually I am sending the patches with submitGit herokuapp because my
institute proxy does not allow IMAP/POP3 connections.


You can still include the 'three dashes' and a commentary at the end of your 
(local) regular commit message, and then when it is sent as a patch it will 
have the right format. There are carried through rebases as well.


There is a similar feature for attaching notes (though I haven't used it).

Either should get around your institute's proxy issue.



The "permanently" bit sounds scary. A more concise way to state this 
might be:


See the `commit.verbose` configuration variable in
linkgit:git-config[1].

which doesn't bother spelling out what the intelligent reader should
infer from the reference.
Style: space before {


Sure!

+test_expect_success 'commit with commit.verbose true and no arguments' 
'


"no arguments" doesn't convey much; how about "--verbose omitted" or
something? Ditto for the titles of other tests.


Will change the language construct.

+   echo content >file &&
+   git add file &&
+   test_config commit.verbose true &&
+   (
+   GIT_EDITOR=cat &&
+   export GIT_EDITOR &&
+   test_must_fail git commit >output
+   ) &&
+   test_i18ngrep "diff --git" output
+'


Making git-commit fail unconditionally with "aborting due to empty
commit message" is a rather sneaky way to perform this test. I would
have expected to see these new tests re-use the existing machinery
provided by this script (the check-for-diff "editor") rather than
inventing an entirely new and unintuitive mechanism. Doing so would
also reduce the size of each new test.


I agree on the fact that making git-commit fail unconditionally is not
a good way to perform the test. "check-for-diff" is not really an
"editor" and it checks for the commit message after it has been
written to the history. The verbose output is stripped when it is
written to the history so we won't be able to test whether this patch
works. This is where purposely breaking the code is required as when
the commit fails, it gives the output of the contents present at that
time (which will contain the verbose output). More over the
'check-for-diff' uses grep which is not preferred. Many tests are now
using test_i18ngrep (eg. f79ce8db). I had planned on using
'check-for-diff' before but it took me some time to figure out this
behavior and thus I began searching for another mechanism (breaking
code).


Some additional tests[1][2] are probably warranted.

[1]: http://article.gmane.org/gmane.comp.version-control.git/288648
[2]: http://article.gmane.org/gmane.comp.version-control.git/288657


I think these tests also are better included in this file as this
patch triggers it and it would not make much of a difference between
t7507 and t7502 but in fact improve its readability.

 test_done


--
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] t/t7502 : drop duplicate test

[Junio C Hamano]

Pranit Bauva  writes:

> This extra test was introduced erroneously by
> f9c0181 (t7502: test commit.status, --status and
> --no-status, 2010-01-13)
>
> Signed-off-by: Pranit Bauva 
> ---

Thanks.  I briefly thought that this might be checking that doing
this twice would give different results, but that is not what is
happening.  Also the remainder of this does cover all the
combinations, so we are OK after applying this patch.

Thanks, will queue.

>  t/t7502-commit.sh | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index b39e313..725687d 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -527,11 +527,6 @@ try_commit_status_combo () {
>   test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG
>   '
>  
> - test_expect_success 'commit' '
> - try_commit "" &&
> - test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG
> - '
> -
>   test_expect_success 'commit --status' '
>   try_commit --status &&
>   test_i18ngrep "^# Changes to be committed:" .git/COMMIT_EDITMSG
>
> --
> https://github.com/git/git/pull/208
--
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: Possible bug: --ext-diff ignored with --cc in git log

[Junio C Hamano]

Vadim Zeitlin  writes:

> On Thu, 10 Mar 2016 14:33:55 -0800 Junio C Hamano  wrote:
>
> JCH> Vadim Zeitlin  writes:
> JCH> 
> JCH> > I.e. the
> JCH> > command "git log --ext-diff -p --cc" still outputs the real diff even 
> for
> JCH> > the generated files, as if "--ext-diff" were not given. ...
> JCH> > Is the current behaviour intentional? I see it with all the git 
> versions I
> JCH> > tried (1.7.10, 2.1.0, 2.7.0 and v2.8.0-rc1), but I don't really see why
> JCH> > would it need to work like this, so I hope it's an oversight and could 
> be
> JCH> > corrected.
> JCH> 
> JCH> I think this is "intentional" in the sense that "--cc" feature is
> JCH> fundamentally and conceptually incompatible with "--ext-diff".
>
>  Thank you for your reply, Junio, I hadn't realized that --cc was dependent
> on textual diff output format before, but now I understand why it can't
> respect --ext-diff.

Having established that, I should also add that "--cc fundamentally
is incompatible with --ext-diff" does not justify that
"--cc when given with --ext-diff just ignores and uses the usual
diff".

An equally (or even more) valid consequence could have been to
disable "--cc" processing for paths that would trigger an external
diff driver.  After all, the user told us that the contents would
not compare well with the usual "diff"; we know that "--cc" output
that summarizes the usual diff output is useless.

What we show instead is an interesting thing to think about.

For example, we _could_ also ignore what external diff driver
produces in this case (as we know it won't be producing an
appropriate input to the "--cc" post-processing), and pretend
as if comparing an old version of foo.sln with a new version of
foo.sln produced a diff like this:

diff --git a/foo.sln b/foo.sln
index d7ff46e,b829410
--- a/foo.sln
+++ b/foo.sln
@@ 1,1 @@
-d7ff46ec4a016c6ab7d233b9d4a196ecde623528  - generated file
+b829410f6da0afc14353b4621d2fdf874181a9f7  - generated file

then you might see in a merge that merges two versions of foo.sln
and result in another version of foo.sln in your "--cc" output a
hunk that is like this:

diff --cc foo.sln
index d7ff46e,6c9aaa1..b829410
--- a/foo.sln
+++ b/foo.sln
@@@ 1,1 @@@
- d7ff46ec4a016c6ab7d233b9d4a196ecde623528  - generated file
 -6c9aaa1ae63a2255a215c1287e38e75fcc5fc5d3  - generated file
++b829410f6da0afc14353b4621d2fdf874181a9f7  - generated file

which would at least tell you that there was a merge, and if the
merge took the full contents of the file from one of the commits and
recorded as the result of the merge, then you wouldn't see them in
the "--cc" output.

It happens that the above is fairly easily doable with today's Git
without any modification.  Here is how.

(1) Have this in your .git/config

[diff "uninteresting"]
textconv = /path/to/uninteresting-textconv-script

(2) Mark your .sln paths as uninteresting in your .gitattributes

*.sln   diff=uninteresting

(3) Have this textconv filter in /path/to/uninteresting-textconv-script

#!/bin/sh
printf "%s generated file\n" "$(sha1sum <"$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: Ability to remember last known good build

[Stefan Beller]

On Fri, Mar 11, 2016 at 10:08 AM, Junio C Hamano  wrote:
> "Pedroso, Osiris"  writes:
>
>> I participate in an open source project that any pull merge is accepted, no 
>> matter what.
>>
>> This makes for lots of broken builds, even though we do have Travis-CI 
>> enabled on the project, because people will merge a request before even the 
>> build is complete.
>>
>> Therefore, I would like to remember the id of the commit of the last 
>> successful build. This would be updated by the Travis-CI script itself upon 
>> a successful build.
>>
>> I imagine best option would be to merge master to a certain branch named 
>> "Last_known_Linux_build" or "Last_known_Windows_build" or even 
>> "Last_known_build_all_tests_passing".
>>
>> I am new to git, but some other experienced co-volunteers tell me that it 
>> may not be possible due to authentication issues.
>>
>> Any better way of accomplishing this?
>
> "test && git branch -f last-good"?

Travis-CI enabled, tells me they're using Github and are distributed,
so one contributor would want to know the last known good state of
a remote, that others push to, without testing all commits locally.

So maybe the question is better rephrased as: "How do we keep track of
the last good state using the distributed nature of Git?"

I would rather ask the more fundamental question of the workflow
of having everything merged despite tests failing. Also accepting
pull requests no matter what, sounds suspicious to me. (Can I sneak
in security issues or delete all files and it still is pulled?)

> --
> 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
--
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: [BUG?] fetch into shallow sends a large number of objects

[Jeff King]

On Fri, Mar 11, 2016 at 07:47:34AM +0700, Duy Nguyen wrote:

> Well, assume again that F and G are ref heads, and their respective
> distance to C and D are the same (like the below graph), then "fetch
> --deptch=" can mark C and D as shallow cut points because
> --depth traverses from refs until the distance is met, it does not do
> total exclusion ^C like rev-list.
> 
>--- B  C  H  F
>   /  /
>  --- D  E  G

Right, so I think we would only apply the "use existing cutoffs as
bounds" thing when we were not otherwise given a --depth. Because it can
definitely cause us to override the depth (and there's no need to; the
point is to avoid linking in all of history, and --depth already solves
that). So we probably do want the client to ask "I'm not giving you a
depth, but please use my existing shallows as cutoffs".

I think a more interesting case here is when we have C as a cutoff, and
somebody fetches "E" directly. They are part of the truncated history.
So we should exclude them from a fetch of "G", but if the user asked for
them directly, that probably needs to override the existing shallow
cutoff.

We probably want to compute the --boundary of "E ^C", but then omit from
that any items that are directly in want_obj. IOW, it is OK to truncate
at depth=1 due to an existing cutoff, but not at depth=0. :)

That does mean we would then fetch all of the history leading up to E,
but I think that's OK. If the user didn't specify --depth and they are
fetching from behind the shallow-cutoff point, we don't have any real
way of knowing how much they wanted (though I guess it would also be
sensible to just apply depth=1 in such a case).

-Peff
--
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: Ability to remember last known good build

[Junio C Hamano]

"Pedroso, Osiris"  writes:

> I participate in an open source project that any pull merge is accepted, no 
> matter what.
>
> This makes for lots of broken builds, even though we do have Travis-CI 
> enabled on the project, because people will merge a request before even the 
> build is complete.
>
> Therefore, I would like to remember the id of the commit of the last 
> successful build. This would be updated by the Travis-CI script itself upon a 
> successful build.
>
> I imagine best option would be to merge master to a certain branch named 
> "Last_known_Linux_build" or "Last_known_Windows_build" or even 
> "Last_known_build_all_tests_passing".
>
> I am new to git, but some other experienced co-volunteers tell me that it may 
> not be possible due to authentication issues.
>
> Any better way of accomplishing this?

"test && git branch -f last-good"?
--
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

Ability to remember last known good build

[Pedroso, Osiris]

I participate in an open source project that any pull merge is accepted, no 
matter what.

This makes for lots of broken builds, even though we do have Travis-CI enabled 
on the project, because people will merge a request before even the build is 
complete.

Therefore, I would like to remember the id of the commit of the last successful 
build. This would be updated by the Travis-CI script itself upon a successful 
build.

I imagine best option would be to merge master to a certain branch named 
"Last_known_Linux_build" or "Last_known_Windows_build" or even 
"Last_known_build_all_tests_passing".

I am new to git, but some other experienced co-volunteers tell me that it may 
not be possible due to authentication issues.

Any better way of accomplishing this?

Appreciate any comments,
Osiris Pedroso

--
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 00/48] Libifying git apply

[Christian Couder]

On Thu, Mar 10, 2016 at 10:26 AM, Duy Nguyen  wrote:
> On Thu, Mar 10, 2016 at 12:48 AM, Christian Couder
>  wrote:
>> This is a patch series about libifying "git apply" functionality, to
>> be able to use this functionality in "git am" without spawning new
>> processes. This should make "git am" and "git rebase" significantly
>> faster.
>>
>> This has been discussed in the following thread:
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/287236/
>>
>> This RFC patch series for now just gets rid of the global variables
>> and refactors the code around a bit.
>>
>> As suggested by Junio the global variables in builtin/apply.c are just
>> thrown into a single "apply_state" structure that is passed around the
>> callchain. A new parameter called "state" that is a pointer to the
>> "apply_state" structure comes at the beginning of the helper functions
>> that need it.
>>
>> Before I make further changes to handle erroneous input and make the
>> libified functions not die() and properly clean things up, I'd be
>> happy to get some feedback.
>
> I didn't review individual patches. Instead I redid the whole thing
> (in a slightly different way) and compared my end result to yours. In
> general it looks good but..
>
> 1) I think you should focus the series on moving global vars to struct
> apply_state only. You can save code move patches for the later phase.
> Easier to review.

I am not sure what you mean by "the later phase", but I tend to agree
and that's mostly what I did.
Most of the code move are at the end of the series so after the global
variable related changes.
There are a few exceptions because in a few cases it was difficult to
understand the code without a refactoring.
In such cases I think the refactoring can also actually help the reviewer.

> 2) Given that there are only four local variables shadowing global
> ones, p_value, linenr and options, I think it's ok to drop 1/48 and
> 2/48 and keep the good old names. You just need to mention about them
> and what function they are declared in in the relevant "move global
> ..." patch so the reviewer is aware of it.

About 1/48 (builtin/apply: avoid parameter shadowing 'p_value') this
is discussed in its own thread.
In 2/48 (builtin/apply: avoid parameter shadowing 'linenr'), I change
the "good old name" only twice in a small function, and I don't think
keeping the old name is a big deal there.

> 3) Moving lock_file to struct apply_state, then putting the whole
> struct on stack, could be problematic. I believe there's a global
> linked list keeping references of all lock_file variables until the
> end of time, so we can't destroy lock_file/apply_state until we are
> certain it's safe to do so. We could simply leave lock_file as a
> global var for now (with a note in struct apply_state so we don't
> forget). We can always do fancy stuff like malloc() later on if we
> need to.

Thanks for this information. Though I prefer to do the fancy stuff
right away, that is something like:

diff --git a/builtin/apply.c b/builtin/apply.c
index 4f57bce..6029869 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -52,6 +52,12 @@ struct apply_state {
const char *prefix;
int prefix_length;

+   /*
+* Since lockfile.c keeps a linked list of all created
+* lock_file structures, it isn't safe to free(lock_file).
+*/
+   struct lock_file *lock_file;
+
int apply;
int allow_overlap;
int apply_in_reverse;
@@ -4517,8 +4523,6 @@ static int write_out_results(struct apply_state
*state, struct patch *list)
return errs;
 }

-static struct lock_file lock_file;
-
 #define INACCURATE_EOF (1<<0)
 #define RECOUNT(1<<1)

@@ -4566,8 +4570,10 @@ static int apply_patch(struct apply_state *state,
state->apply = 0;

state->update_index = state->check_index && state->apply;
-   if (state->update_index && newfd < 0)
-   newfd = hold_locked_index(_file, 1);
+   if (state->update_index && newfd < 0) {
+   state->lock_file = xcalloc(1, sizeof(struct lock_file));
+   newfd = hold_locked_index(state->lock_file, 1);
+   }

if (state->check_index) {
if (read_cache() < 0)
@@ -4780,7 +4786,7 @@ static int apply_all_patches(struct apply_state *state,
}

if (state->update_index) {
-   if (write_locked_index(_index, _file, COMMIT_LOCK))
+   if (write_locked_index(_index, state->lock_file,
COMMIT_LOCK))
die(_("Unable to write new index file"));
}

It makes it possible to remove the 'newfd' variable as it is only used
to check if hold_locked_index() has already been called and now
"state->lock_file == NULL" can be used to do that.

By the way I wonder if I should also change "static struct lock_file
lock" in build_fake_ancestor().

> 4) I noticed on the interdiff that there are 

Re: [BUG?] fetch into shallow sends a large number of objects

[Junio C Hamano]

Duy Nguyen  writes:

> Well, assume again that F and G are ref heads, and their respective
> distance to C and D are the same (like the below graph), then "fetch
> --deptch=" can mark C and D as shallow cut points because
> --depth traverses from refs until the distance is met, it does not do
> total exclusion ^C like rev-list.
>
>--- B  C  H  F
>   /  /
>  --- D  E  G

Hmph, so we do not realize that D is reachable from C because we do
not even look at the parents of the latter.  Is there a remification
that breaks the proposed mental view coming from this?

Let me think aloud.  Later when we fetch again (without asking to
change the depth but without refusing an unsolicited depth change),
a fetch of history that lead to descendants of F or G will still not
realize that C and D are related, because the traversal down to C
would stop there without noticing that E is reachable from C.  That
won't break.  If the tips of the history have been rewound and now
points at the tips of a history that is forked before B and D (let's
say the new tip to replace G is Z), would that be a problem?  The
side that does have the full history would notice that the fork
point X is an ancestors of cutoff C and D, and can tell that the
side-branch proper, Y and Z, are interesting but X is where their
truncated history must stop.

--- B  C  H  F
   /  /
 -- X --- D  E  G
 \
  Y --- Z

While doing so the side that originally had C and D as the cutoff
would be told that now X is also a cutoff.  In general, because the
side that has the full history and gets fetched would not have to
know about _all_ the refs and objects the fetching side has, so if
it didn't learn about the shallow side still having G, it wouldn't
be able to say that D is no longer an interesting cutoff, but would
that be a problem?  The side with full history should also be able
to notice that E can also be added as a cutoff.  Would that be a
good thing to do?  If so, perhaps we should have done that when the
original history was shallowing cloned with depth=2 in your picture,
perhaps?  After all, even though the shallow side may not know C and
D are related, the side that supplied the truncated history knows,
so after depth traversal finds C and D, it perhaps can postprocess
it to realize C and E is the set that should be sent as the cutoff?

--
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 for collaboration on RDF data

[Jordan Lewis]

Hello Mark,

I realize this is an old post, but I was looking into doing exactly what you are
 talking about. I did some research into JGIt to see if I could leverage any of
that code to create an RDF Git-like experience. I came to the conclusion that
I would either need to extend a lot of that codebase or create my own. I was
leaning towards creating my own solution.

I was wondering what you ended up doing for your case? Did you find any
external libraries to help accomplish this? Was it a completely custom
solution?

Any pointers or advice would be greatly appreciated.

Cheers,
Jordan Lewis

--
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 00/48] Libifying git apply

[Christian Couder]

On Wed, Mar 9, 2016 at 7:14 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> One point I'd especially welcome feedback about is the fact that there
>> are many boolean options that are using OPT_BOOL(...), so they use an
>> int. And there are a few others that are using OPT_BIT(...), so they
>> use just a bit. I wonder if it is worth it to try to be consistent,
>> and maybe also to try to save some memory.
>>
>> Related to this, some of the variables for these options have not been
>> moved into the "apply_state" structure, because they are not global to
>> the file, but maybe for consistency they should be.
>
> These might be worthy clean-ups but that is only if they are done
> after we make sure conversion proper is done faithfully to the
> original, i.e. without introducing unnecessary bugs.  I'd advise
> against doing them before the libification is done.

Ok, I will try to avoid those kind of clean-ups.
--
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 v7 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 11, 2016 at 7:00 PM, Matthieu Moy
 wrote:
> What Junio says is that you don't need to write
>
> static int config_autostash = 0;
>
> since it is equivalent to
>
> static int config_autostash;
>
> But there's nothing wrong with having a static variable defaulting to 0.

My bad. I should have read Junio's comment more carefully.

config_autostash can be default to 0. And thus
 if (opt_autostash != 1)
die_on_unclean_work_tree(prefix);

can be replaced by
   if (!opt_autostash)
die_on_unclean_work_tree(prefix);

and thus opt_autostash will be either 0 or 1 and we don't
have to worry about it being -1 (whenever --rebase is passed).

I will make the changes accordingly.

Thanks,
Mehul
--
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: Sample pre-push hook can crash

[Stephen Morton]

That is interesting, so in the case of a non-ff push, there is no way
for a pre-push hook to know what is being pushed in order to run?

Steve


On Thu, Mar 10, 2016 at 4:43 PM, Junio C Hamano  wrote:
> Stephen Morton  writes:
>
>> The sample pre-push hook provided with git [1] will crash if the local
>> repo is not up to date with the remote as $remote_sha is not present
>> in the local repo. I'm not sure if this patch is exactly correct, it's
>> just provided as an example.
>>
>> Given that people are likely crafting their own solutions based on the
>> examples, it's probably good to get right.
>
> It's probably good to get right, but I do not think use of @{u} is
> making it right, unfortunately.  You may not necessarily have @{u}
> configured, and you may not even pushing to the configured remote
> branch.
>
> The spirit of the sample hook, I think, is to validate the new
> commits you are publishing, so if you cannot even determine which
> ones are new and which ones are not, failing the "push" by exiting
> with non-zero status is the right behaviour for this sample.
>
> So perhaps something like this may be more appropriate as an
> example.
>
>  templates/hooks--pre-push.sample | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/templates/hooks--pre-push.sample 
> b/templates/hooks--pre-push.sample
> index 6187dbf..7ef6780 100755
> --- a/templates/hooks--pre-push.sample
> +++ b/templates/hooks--pre-push.sample
> @@ -41,7 +41,12 @@ do
> fi
>
> # Check for WIP commit
> -   commit=`git rev-list -n 1 --grep '^WIP' "$range"`
> +   commit=`git rev-list -n 1 --grep '^WIP' "$range"` || {
> +   # we do not even know about the range...
> +   echo >&2 "Non-ff update to $remote_ref"
> +   echo >&2 "fetch from there first"
> +   exit 1
> +   }
> if [ -n "$commit" ]
> then
> echo >&2 "Found WIP commit in $local_ref, not pushing"
--
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 2/6] gitweb.perl : added ability to debug requests coming through gitweb interface by adding an option to request URL (also requires server-side envvar "GITWEB_MAY_DEBUG=yes")

[Jim Klimov]

---
 gitweb/gitweb.perl | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 030d429..c715472 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -74,6 +74,11 @@ sub evaluate_uri {
our $home_link = $my_uri || "/";
 }
 
+# Request parameters
+our ($action, $project, $DEBUG, $file_name, $file_parent, $hash, $hash_parent, 
$hash_base,
+ $hash_parent_base, @extra_options, $page, $searchtype, $search_use_regexp,
+ $searchtext, $search_regexp, $project_filter);
+
 # core git executable to use
 # this can just be "git" if your webserver has a sensible PATH
 our $GIT = "++GIT_BINDIR++/git";
@@ -803,6 +808,7 @@ our %input_params = ();
 our @cgi_param_mapping = (
project => "p",
action => "a",
+   debug => "debug",
file_name => "f",
file_parent => "fp",
hash => "h",
@@ -1033,9 +1039,6 @@ sub evaluate_path_info {
}
 }
 
-our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, 
$hash_base,
- $hash_parent_base, @extra_options, $page, $searchtype, $search_use_regexp,
- $searchtext, $search_regexp, $project_filter);
 sub evaluate_and_validate_params {
our $action = $input_params{'action'};
if (defined $action) {
@@ -1053,6 +1056,23 @@ sub evaluate_and_validate_params {
}
}
 
+   our $DEBUG = $input_params{'debug'};
+   if (defined $DEBUG) {
+   if ( $DEBUG =~ /^([Yy][Ee][Ss]|[Oo][Nn]|1|[Tt][Rr][Uu][Ee])$/ ) 
{
+   if ( defined($ENV{'GITWEB_MAY_DEBUG'}) && 
$ENV{'GITWEB_MAY_DEBUG'} eq "yes" ) {
+   $DEBUG = 1; #true
+   } else {
+   printf STDERR "Invalid action parameter: 
DEBUG=$DEBUG was requested but server-side GITWEB_MAY_DEBUG=yes is not set\n";
+   die_error(403, "Invalid action parameter: DEBUG 
was requested but server-side GITWEB_MAY_DEBUG=yes is not set");
+   $DEBUG = 0; #false
+   }
+   } else {
+   $DEBUG = 0; #false
+   }
+   } else {
+   $DEBUG = 0; # false
+   }
+
our $project_filter = $input_params{'project_filter'};
if (defined $project_filter) {
if (!is_valid_pathname($project_filter)) {
-- 
2.1.4

--
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 5/6] gitweb.perl changed (and tested) to return HTTP-404 when missing objects are requested via snapshot

[Jim Klimov]

---
 gitweb/gitweb.perl| 62 +--
 t/t9502-gitweb-standalone-parse-output.sh | 24 ++--
 2 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fc5b62d..2369ae3 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7418,20 +7418,60 @@ sub git_snapshot {
%latest_date = parse_date($co{'committer_epoch'}, 
$co{'committer_tz'});
}
 
-   print $cgi->header(
-   -type => $known_snapshot_formats{$format}{'type'},
-   -content_disposition => 'inline; filename="' . $filename . '"',
-   %co ? (-last_modified => $latest_date{'rfc2822'}) : (),
-   -status => '200 OK');
-
printf STDERR "Starting git-archive: $cmd\n" if $DEBUG;
-   open my $fd, "-|", $cmd
-   or die_error(500, "Execute git-archive failed");
+   my $fd;
+   if ( ! open $fd, "-|", $cmd ) {
+   print $cgi->header(-status => '500 Execute git-archive failed');
+   die_error(500, "Execute git-archive failed");
+   return;
+   }
printf STDERR "Started git-archive...\n" if $DEBUG;
-   binmode STDOUT, ':raw';
-   print <$fd>;
-   binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
+   my $tempByte;
+   my $readSize = read ($fd, $tempByte, 1);
+   my $retCode = 200;
+   if ( defined $readSize ) {
+   if ( $readSize > 0 ) {
+   print $cgi->header(
+   -type => 
$known_snapshot_formats{$format}{'type'},
+   -content_disposition => 'inline; filename="' . 
$filename . '"',
+   %co ? (-last_modified => 
$latest_date{'rfc2822'}) : (),
+   -status => '200 OK' );
+   binmode STDOUT, ':raw';
+   print $tempByte;
+   if ( ! print <$fd> ) {
+   $retCode = 503;
+   }
+   binmode STDOUT, ':utf8'; # as set at the beginning of 
gitweb.cgi
+   } else {
+   $retCode = 404;
+   }
+   } else {
+   $readSize = -1;
+   $retCode = 500;
+   }
+
close $fd;
+   my $retError = "" ;
+   if ( ($? >> 8) != 0 ) {
+   $retCode = 500;
+   if ( $readSize == 0 ) {
+   # We had empty but not failed read - re-inspect stderr
+   $retError = `$cmd 2>&1`;
+   if ( $retError =~ /did not match any/ ) {
+   $retCode = 404;
+   }
+   }
+   }
+   if ( $retError ne "" ) {
+   $retError = "$retError";
+   }
+
+   if ( $retCode == 404 ) {
+   die_error(404, "Not Found - maybe requested objects absent in 
git path?" . "$retError");
+   } elsif ( $retCode == 500 ) {
+   die_error(500, "Failed to transmit output from git-archive" . 
"$retError");
+   }
+
printf STDERR "Finished posting output of git-archive...\n" if $DEBUG;
 }
 
diff --git a/t/t9502-gitweb-standalone-parse-output.sh 
b/t/t9502-gitweb-standalone-parse-output.sh
index 11a116f..a2ae5c4 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -156,21 +156,21 @@ test_expect_success 'snapshot certain objects: have 
expected content in master b
 '
 test_debug 'cat gitweb.headers && cat file_list'
 
-test_expect_success 'snapshot certain objects: have expected content in master 
branch - subdir name is required in requested nested path (bad path - empty 
output)' '
+test_expect_success 'snapshot certain objects: have expected content in master 
branch - subdir name is required in requested nested path (bad path - empty 
output and/or HTTP-404)' '
rm -f gitweb.body file_list &&
BRANCH=master &&
gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=third" &&
-   [ ! -s gitweb.body ]
+   [ ! -s gitweb.body -o -n "`head -1 gitweb.headers | egrep "^Status: 404 
"`" ]
 '
-test_debug 'cat gitweb.headers && cat file_list'
+test_debug 'cat gitweb.headers && ls -la gitweb.body file_list || true'
 
-test_expect_success 'snapshot certain objects: have expected content in master 
branch - correct subdir name is required in requested nested path (bad path - 
empty output)' '
+test_expect_success 'snapshot certain objects: have expected content in master 
branch - correct subdir name is required in requested nested path (bad path - 
empty output and/or HTTP-404)' '
rm -f gitweb.body file_list &&
BRANCH=master &&
gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=dir1/third" &&
-   [ ! -s gitweb.body ]
+   [ ! -s gitweb.body -o -n "`head -1 gitweb.headers | egrep "^Status: 404 
"`" ]
 '

[PATCH 1/6] gitweb.perl : added ability to "git archive" just certain paths (files or subdirs, not whole repo) via gitweb interface

[Jim Klimov]

---
 gitweb/gitweb.perl | 17 +
 1 file changed, 17 insertions(+)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 05d7910..030d429 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7328,6 +7328,15 @@ sub git_snapshot {
die_error(403, "Unsupported snapshot format");
}
 
+   if (!defined($hash)) {
+   $hash="";
+   if ( $file_name && $file_name =~ /^([^:]*):(.*)$/ ) {
+   $hash = "$1";
+   $file_name = "$2";
+   }
+   if ( $hash eq "") { $hash = "HEAD"; }
+   printf STDERR "Defaulted hash to '$hash' ('h=' URL argument was 
missing)\n";
+   }
my $type = git_get_type("$hash^{}");
if (!$type) {
die_error(404, 'Object does not exist');
@@ -7345,6 +7354,14 @@ sub git_snapshot {
git_cmd(), 'archive',
"--format=$known_snapshot_formats{$format}{'format'}",
"--prefix=$prefix/", $hash);
+   if ($file_name) {
+   # To fetch several pathnames use space-separation, e.g.
+   # "...git-web?p=proj.git;a=snapshot;f=file1%20file2
+   # To fetch pathnames with spaces, escape them, e.g.
+   # "...git-web?p=proj.git;a=snapshot;f=file\%20name
+   $cmd .= " " . $file_name;
+   }
+
if (exists $known_snapshot_formats{$format}{'compressor'}) {
$cmd .= ' | ' . 
quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
}
-- 
2.1.4

--
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 6/6] gitweb.perl : the optional DEBUG functionality is now unit-tested

[Jim Klimov]

---
 t/t9502-gitweb-standalone-parse-output.sh | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/t/t9502-gitweb-standalone-parse-output.sh 
b/t/t9502-gitweb-standalone-parse-output.sh
index a2ae5c4..4f1ddbf 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -253,6 +253,41 @@ test_expect_success 'snapshot certain objects: have 
expected content in xx/test
 test_debug 'cat gitweb.headers && cat file_list'
 
 # --
+# optional debugging in log, if allowed on server and requested by user
+
+test_expect_success 'snapshot: debugging logged as forbidden when not defined 
in server environment' '
+   rm -f gitweb.body gitweb.log gitweb.headers gitweb.output &&
+   gitweb_run "p=.git;a=snapshot;h=master;sf=tar;debug=yes" &&
+   grep "GITWEB_MAY_DEBUG=yes is not set" < gitweb.log >/dev/null
+'
+test_debug 'cat gitweb.headers gitweb.log'
+
+test_expect_success 'snapshot: debugging logged as forbidden when not allowed 
in server environment' '
+   rm -f gitweb.body gitweb.log gitweb.headers gitweb.output &&
+   GITWEB_MAY_DEBUG=xxx && export GITWEB_MAY_DEBUG &&
+   gitweb_run "p=.git;a=snapshot;h=master;sf=tar;debug=yes" &&
+   grep "GITWEB_MAY_DEBUG=yes is not set" < gitweb.log >/dev/null
+'
+test_debug 'cat gitweb.headers gitweb.log'
+
+test_expect_success 'snapshot: debugging present when allowed in server 
environment' '
+   rm -f gitweb.body gitweb.log gitweb.headers gitweb.output &&
+   GITWEB_MAY_DEBUG=yes && export GITWEB_MAY_DEBUG &&
+   gitweb_run "p=.git;a=snapshot;h=master;sf=tar;debug=yes" &&
+   ! grep "GITWEB_MAY_DEBUG=yes is not set" < gitweb.log >/dev/null &&
+   grep "git-archive" < gitweb.log >/dev/null
+'
+test_debug 'cat gitweb.headers gitweb.log'
+
+test_expect_success 'snapshot: debugging absent when not allowed in server 
environment' '
+   rm -f gitweb.body gitweb.log gitweb.headers gitweb.output &&
+   GITWEB_MAY_DEBUG=xxx && export GITWEB_MAY_DEBUG &&
+   gitweb_run "p=.git;a=snapshot;h=master;sf=tar;debug=yes" &&
+   ! grep "git-archive" < gitweb.log >/dev/null
+'
+test_debug 'cat gitweb.headers gitweb.log'
+
+# --
 # forks of projects
 
 test_expect_success 'forks: setup' '
-- 
2.1.4

--
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 v7 2/2] pull --rebase: add --[no-]autostash flag

[Matthieu Moy]

Mehul Jain  writes:

> On Fri, Mar 11, 2016 at 10:21 AM, Paul Tan  wrote:
>>>  static int config_autostash = -1;
>>
>> Hmm, why can't config_autostash just default to 0?
>
> Previously Junio recommended not to explicitly initialize a
> static to 0 (or NULL).
> http://thread.gmane.org/gmane.comp.version-control.git/287709/focus=287726

What Junio says is that you don't need to write

static int config_autostash = 0;

since it is equivalent to

static int config_autostash;

But there's nothing wrong with having a static variable defaulting to 0.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 4/6] gitweb.perl support for snapshots with lists of specified files is now tested

[Jim Klimov]

---
 t/t9502-gitweb-standalone-parse-output.sh | 156 --
 1 file changed, 148 insertions(+), 8 deletions(-)

diff --git a/t/t9502-gitweb-standalone-parse-output.sh 
b/t/t9502-gitweb-standalone-parse-output.sh
index 0796a43..11a116f 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -27,25 +27,46 @@ $known_snapshot_formats{'tar'} = {
 $feature{'snapshot'}{'default'} = ['tar'];
 EOF
 
-# Call check_snapshot with the arguments " []"
+# Call list_snapshot with the argument ""
 #
 # This will check that gitweb HTTP header contains proposed filename
-# as  with '.tar' suffix added, and that generated tarfile
-# (gitweb message body) has  as prefix for al files in tarfile
+# as  with '.tar' suffix added, and lists its content to
+# stdout of this routine (in "tar test" default listing format)
 #
-#  default to 
-check_snapshot () {
-   basename=$1
-   prefix=${2:-"$1"}
+#  defaults to 
+#
+list_snapshot () {
+   basename="`echo "$1" | sed 's,\/,\.,g'`"
echo "basename=$basename"
grep "filename=.*$basename.tar" gitweb.headers >/dev/null 2>&1 &&
-   "$TAR" tf gitweb.body >file_list &&
+   "$TAR" tf gitweb.body >file_list
+}
+
+#
+# Call check_snapshot with the arguments " []"
+#
+# This uses list_snapshot() above to list the tarfile .tar received
+# from gitweb, and that this generated tarfile (gitweb message body) has
+#  prepended as prefix for all objects in the tarfile
+# The tarfile listing is exchanged via the "file_list" temporary file
+#
+#  defaults to 
+#
+check_snapshot () {
+   basename="$1"
+   prefix=${2:-"$1"}
+   list_snapshot "$basename" &&
! grep -v -e "^$prefix$" -e "^$prefix/" -e "^pax_global_header$" 
file_list
 }
 
+# Note: the "xx/test" branch only contains file "foo"; others land in "master"
+# Call test_commit with the arguments " [ [ []]]"
 test_expect_success setup '
test_commit first foo &&
+   mkdir -p dir1 && test_commit bar dir1/second bar second &&
git branch xx/test &&
+   mkdir -p dir2 && test_commit pif dir2/third pif third &&
+   test_commit wow dir2/"fourth file" wow wow &&
FULL_ID=$(git rev-parse --verify HEAD) &&
SHORT_ID=$(git rev-parse --verify --short=7 HEAD)
 '
@@ -112,6 +133,125 @@ test_expect_success 'snapshot: hierarchical branch name 
(xx/test)' '
 '
 test_debug 'cat gitweb.headers'
 
+test_expect_success 'snapshot sanity: have expected content in xx/test branch 
- do not have /first file in full snapshot' '
+   rm -f gitweb.body file_list &&
+   BRANCH=xx/test &&
+   gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar" &&
+   ID=$(git rev-parse --verify --short=7 "$BRANCH") &&
+   list_snapshot ".git-$BRANCH-$ID" &&
+   ! grep "first" file_list
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot certain objects: have expected content in master 
branch - only those under subdir dir2/ and not others' '
+   rm -f gitweb.body file_list &&
+   BRANCH=master &&
+   gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=dir2" &&
+   ID=$(git rev-parse --verify --short=7 "$BRANCH") &&
+   list_snapshot ".git-$BRANCH-$ID" &&
+   ! grep "foo" file_list &&
+   ! grep "dir1/second" file_list &&
+   grep "dir2/third" file_list &&
+   grep "dir2/fourth file" file_list
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot certain objects: have expected content in master 
branch - subdir name is required in requested nested path (bad path - empty 
output)' '
+   rm -f gitweb.body file_list &&
+   BRANCH=master &&
+   gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=third" &&
+   [ ! -s gitweb.body ]
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot certain objects: have expected content in master 
branch - correct subdir name is required in requested nested path (bad path - 
empty output)' '
+   rm -f gitweb.body file_list &&
+   BRANCH=master &&
+   gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=dir1/third" &&
+   [ ! -s gitweb.body ]
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot certain objects: have expected content in master 
branch - can request filenames with spaces (backslash + HTML-escape)' '
+   rm -f gitweb.body file_list &&
+   BRANCH=master &&
+   gitweb_run "p=.git;a=snapshot;h=$BRANCH;sf=tar;f=dir2/fourth\%20file" &&
+   ID=$(git rev-parse --verify --short=7 "$BRANCH") &&
+   list_snapshot ".git-$BRANCH-$ID" &&
+   ! grep "foo" file_list &&
+   ! grep "dir1/second" file_list &&
+   ! grep "dir2/third" file_list &&
+   grep "dir2/fourth file" file_list
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot certain objects: have expected content in master 
branch - can request list of filenames separated by 

[PATCH 3/6] gitweb.perl : added ability to DEBUG progression through "git archive"

[Jim Klimov]

---
 gitweb/gitweb.perl | 29 +
 1 file changed, 29 insertions(+)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c715472..fc5b62d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -891,6 +891,7 @@ sub evaluate_query_params {
 # now read PATH_INFO and update the parameter list for missing parameters
 sub evaluate_path_info {
return if defined $input_params{'project'};
+   printf STDERR "path_info='$path_info'\n" if $DEBUG;
return if !$path_info;
$path_info =~ s,^/+,,;
return if !$path_info;
@@ -7336,6 +7337,24 @@ sub git_snapshot {
if (!@snapshot_fmts) {
die_error(403, "Snapshots not allowed");
}
+
+   if ($DEBUG) {
+   my $v; my $i;
+   printf STDERR "path_info='".$path_info."'\n";
+   printf STDERR "input_params: { ";
+   foreach $i (keys (%input_params)) {
+   $v = $input_params{$i};
+   if (defined ($v)) {
+   if ($i eq "extra_options" ) {
+   printf STDERR "  '$i' => [".@{$v}."] ; 
";
+   } else {
+   printf STDERR "  '$i' => '$v' ; ";
+   }
+   }
+   }
+   printf STDERR "} \n";
+   }
+
# default to first supported snapshot format
$format ||= $snapshot_fmts[0];
if ($format !~ m/^[a-z0-9]+$/) {
@@ -7367,6 +7386,13 @@ sub git_snapshot {
my ($name, $prefix) = snapshot_name($project, $hash);
my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
 
+   if ($DEBUG) {
+   # name of the tarball to generate
+   if (defined $filename)  { printf STDERR 
"filename='$filename'\n"; }
+   # value of the 'f=' URL parameter
+   if (defined $file_name) { printf STDERR 
"file_name='$file_name'\n"; }
+   }
+
my %co = parse_commit($hash);
exit_if_unmodified_since($co{'committer_epoch'}) if %co;
 
@@ -7398,12 +7424,15 @@ sub git_snapshot {
%co ? (-last_modified => $latest_date{'rfc2822'}) : (),
-status => '200 OK');
 
+   printf STDERR "Starting git-archive: $cmd\n" if $DEBUG;
open my $fd, "-|", $cmd
or die_error(500, "Execute git-archive failed");
+   printf STDERR "Started git-archive...\n" if $DEBUG;
binmode STDOUT, ':raw';
print <$fd>;
binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
close $fd;
+   printf STDERR "Finished posting output of git-archive...\n" if $DEBUG;
 }
 
 sub git_log_generic {
-- 
2.1.4

--
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 v7 2/2] pull --rebase: add --[no-]autostash flag

[Mehul Jain]

On Fri, Mar 11, 2016 at 10:21 AM, Paul Tan  wrote:
>>  static int config_autostash = -1;
>
> Hmm, why can't config_autostash just default to 0?

Previously Junio recommended not to explicitly initialize a
static to 0 (or NULL).
http://thread.gmane.org/gmane.comp.version-control.git/287709/focus=287726

Defaulting config_autostash = 0 will work too as you explained.

>> +   if (opt_autostash == 1)
>> +   argv_array_push(, "--autostash");
>> +   else if (opt_autostash == 0)
>> +   argv_array_push(, "--no-autostash");
>
> The precise testing for specific values of -1, 0 and 1 throughout the
> code makes me uncomfortable. Ordinarily, I would expect a simple
>
> argv_array_push(, opt_autostash ? "--autostash" : "--no-autostash");

This looks good. I will change accordingly.

> Stepping back a bit, the only reason why we introduced opt_autostash =
> -1 as a possible value is because we need to detect if
> --[no-]autostash is being used with git-merge. I consider that a
> kludge, because if git-merge supports --autostash as well (possibly in
> the future), then we will not need this -1 value.
>
> So, from that point of view, a -1 value is okay as a workaround, but
> kludges, and hence the -1 value, should be gotten rid off as soon as
> possible.

That right! But until git-merge doesn't support --autostash, it's necessary to
have opt_autostash = -1 as default.

I wonder if it will be a good thing to add the following line to the
commit message
"Changes needed to be introduced:
Change opt_autostash = 0 as default as soon as git-merge supports
--autostash option, also display no error when "git pull --autostash"
is called."

Possibly it would be better to add gmane link of your review in the
commit message
for further clarification.

Thanks,
Mehul
--
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] commit: add a commit.verbose config variable

[Roberto Tyley]

On 11 March 2016 at 05:44, Eric Sunshine  wrote:
> On Fri, Mar 11, 2016 at 05:45:27AM +0530, Pranit Bauva wrote:
>> Actually I am sending the patches with submitGit herokuapp because my
>> institute proxy does not allow IMAP/POP3 connections.

Really glad to hear this is helping you Pranit - I hadn't even thought
of the blocked IMAP/POP3 connections problem, I'm not sure what other
method you could have easily used to get round this.

> That's unfortunate. Your separate "cover letter" often arrives hours
> later than the patch itself. Perhaps Roberto can comment on submitGit
> and per-patch commentary.

This sounds like an improvement I need to make to submitGit, I've
created an issue here:

https://github.com/rtyley/submitgit/issues/30
--
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] commit: add a commit.verbose config variable

[Pranit Bauva]

On Fri, Mar 11, 2016 at 11:14 AM, Eric Sunshine  wrote:

> It's a bit tricky if you're not used to it, but check-for-diff
> actually does what you want, and does so in a more direct way. While
> it's true that it's not an "editor" per se, it does get access to the
> entire block of text that would normally appear in your editor during
> an interactive commit. And, this is happening before the commit has
> been written to history. So, check-for-diff gets a chance to look at
> the full text that would appear in your editor, and can therefore
> check if it contains the expected "diff --git" string.

Yes, this was new to me. Thanks for explaining it in an elaborate
manner. It took me some time to actually understand the behavior of
check-for-diff (a tricky one). And it does the task pretty nicely!

> 'test_i18ngrep' is intended for strings which may be translated,
> however, since the expected "diff --git" string should never be
> translated, check-for-diff's use of 'grep' is correct, whereas
> 'test_i18ngrep' would be misleading (if not actively wrong).

I should have read the docs before using this method and not just
blindly using it. I will definitely take care of that next time.

> As an experiment, I rewrote the four new tests in terms of
> check-for-diff (with "test_set_editor check-for-diff" already in
> effect). Here's what they look like, and they function as expected:
>
> test_expect_success 'commit.verbose true and --verbose omitted' '
> git -c commit.verbose=true commit --amend
> '
>
> test_expect_success 'commit.verbose true and --no-verbose' '
> test_must_fail git -c commit.verbose=true commit --amend --no-verbose
> '
>
> test_expect_success 'commit.verbose false and --verbose' '
> git -c commit.verbose=false commit --amend --verbose
> '
>
> test_expect_success 'commit.verbose false and --verbose omitted' '
> test_must_fail git -c commit.verbose=false commit --amend
> '
>
> These are modeled after the "initial commit shows verbose diff" test
> earlier in the script.

Thanks a lot for helping me with the tests. I will add the status
tests and then resend the patch. This was a nice exercise!
--
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