Re: [PATCH 2/2] submodule: munge paths to submodule git directories

2018-08-15 Thread Aaron Schrab

At 15:33 -0700 08 Aug 2018, Brandon Williams  wrote:

Teach "submodule_name_to_gitdir()" to munge a submodule's name (by url
encoding it) before using it to build a path to the submodule's gitdir.


Seems like this will be a problem if it results in names that exceed 
NAME_MAX? On common systems that's 255, so it's probably not going to be 
common; but it certainly could for some repositories.


[PATCH v3] sequencer: use configured comment character

2018-07-15 Thread Aaron Schrab
At 10:15 -0700 12 Jul 2018, Junio C Hamano  wrote:
>Aaron Schrab  writes:
>> Note that the comment_line_char has already been resolved by this point,
>> even if the user has configured the comment character to be selected
>> automatically.
>
>Isn't this a slight lie?

Looking into that a bit further, it does seem like my explanation above 
was incorrect.  Here's another attempt to explain why setting 
core.commentChar=auto isn't a problem for this change.

8< -

Use the configured comment character when generating comments about
branches in a todo list.  Failure to honor this configuration causes a
failure to parse the resulting todo list.

Setting core.commentChar to "auto" will not be honored here, and the
previously configured or default value will be used instead. But, since
the todo list will consist of only generated content, there should not
be any non-comment lines beginning with that character.

Signed-off-by: Aaron Schrab 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 4034c0461b..caf91af29d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3991,7 +3991,7 @@ static int make_script_with_merges(struct 
pretty_print_context *pp,
entry = oidmap_get(, >object.oid);
 
if (entry)
-   fprintf(out, "\n# Branch %s\n", entry->string);
+   fprintf(out, "\n%c Branch %s\n", comment_line_char, 
entry->string);
else
fprintf(out, "\n");
 
-- 
2.18.0.419.gfe4b301394



Re: [PATCH 0/2] Fix --rebase-merges with custom commentChar

2018-07-11 Thread Aaron Schrab
Sorry for taking so long to get back to this. Hopefully the following 
will be acceptable to everyone.

8< -

Subject: [PATCH v2] sequencer: use configured comment character

Use the configured comment character when generating comments about
branches in a todo list.  Failure to honor this configuration causes a
failure to parse the resulting todo list.

Note that the comment_line_char has already been resolved by this point,
even if the user has configured the comment character to be selected
automatically.

Signed-off-by: Aaron Schrab 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 4034c0461b..caf91af29d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3991,7 +3991,7 @@ static int make_script_with_merges(struct 
pretty_print_context *pp,
entry = oidmap_get(, >object.oid);
 
if (entry)
-   fprintf(out, "\n# Branch %s\n", entry->string);
+   fprintf(out, "\n%c Branch %s\n", comment_line_char, 
entry->string);
else
fprintf(out, "\n");
 
-- 
2.18.0.419.gfe4b301394



[PATCH] sequencer: use configured comment character

2018-06-27 Thread Aaron Schrab
Use configured comment character when generating comments about branches
in an instruction sheet.  Failure to honor this configuration causes a
failure to parse the resulting instruction sheet.

Signed-off-by: Aaron Schrab 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 4034c0461b..caf91af29d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3991,7 +3991,7 @@ static int make_script_with_merges(struct 
pretty_print_context *pp,
entry = oidmap_get(, >object.oid);
 
if (entry)
-   fprintf(out, "\n# Branch %s\n", entry->string);
+   fprintf(out, "\n%c Branch %s\n", comment_line_char, 
entry->string);
else
fprintf(out, "\n");
 
-- 
2.18.0.419.gfe4b301394



Re: [PATCH v3] Documentation: declare "core.ignorecase" as internal variable

2018-06-27 Thread Aaron Schrab

At 12:11 -0700 27 Jun 2018, Junio C Hamano  wrote:

Hmph.  Do other people have difficulty applying this patch to their
trees?  It is just several lines long so I could retype it myself,
but I guess "Content-Type: text/plain; charset=utf-8; format=flowed"
has destroyed formatting of the patch rather badly.


Yes, format=flowed requires lines that start with a space (along with 
'>' or 'From ') to be space-stuffed, adding a leading space. This will 
affect context lines in patches.


I was able to apply it cleanly (I think) by sending the message to: 


 sed '/@@/,$s/^  / /' | git am

That's replacing two leading spaces with one.


Re: [PATCH/RFC] completion: complete all possible -no-

2018-05-08 Thread Aaron Schrab

At 17:24 +0200 08 May 2018, Duy Nguyen  wrote:

It took me so long to reply partly because I remember seeing some guy
doing clever trick with tab completion that also shows a short help
text in addition to the complete words. I could not find that again
and from my reading (also internet searching) it's probably not
possible to do this without trickery.


Was that perhaps using zsh rather than bash? Below is some of the 
display from its git completion (this is likely affected somewhat by my 
configuration).  The group descriptions (lines that begin with 
"Completing") appear in a different color, and are not available for 
selection.


1113$ git c
Completing alias
ci   -- alias for 'commit -v'
cia  -- alias for 'commit -v -a'
co   -- alias for 'checkout'
conf -- alias for 'config'
Completing main porcelain command
checkout -- checkout branch or paths to working tree
cherry-pick  -- apply changes introduced by some existing commits
citool   -- graphical alternative to git commit
clean-- remove untracked files from working tree
clone-- clone repository into new directory
commit   -- record changes to repository
Completing ancillary manipulator command
config   -- get and set repository or global options
Completing ancillary interrogator command
cherry   -- find commits not merged upstream
count-objects-- count unpacked objects and display their disk consumption
Completing plumbing manipulator command
checkout-index   -- copy files from index to working directory
commit-tree  -- create new commit object
Completing plumbing interrogator command
cat-file -- provide content or type information for repository objects

1114$ git commit -
Completing option
--all  -a   -- stage all modified and deleted paths
--allow-empty   -- allow recording an empty commit
--allow-empty-message   -- allow recording a commit with an empty 
message
--amend -- amend the tip of the current branch
--author-- override the author name used in the commit


Re: Is there a way to have local changes in a branch 'bake' while working in different branches?

2016-12-15 Thread Aaron Schrab

At 20:14 + 15 Dec 2016, Larry Minton  wrote:
Let's say I have a code change that I want to 'bake' for a while 
locally, just to make sure some edge case doesn't pop up while I am 
working on other things.  Is there any practical way of doing that?
I could constantly merge that 'bake me' branch into other branches as I 
work on them and then remove those changes from the branches before 
sending them out for code review, but sooner or later pretty much 
guaranteed to screw that up


That sounds like the best way to me. How do you envision screwing it up?

If you anticipate messing up while removing the changes, that's only 
likely if there are conflicts and any other strategy is likely to be 
worse there.


If you suspect you'll forget to remove them before sending for code 
review there may be ways to help with that. Easiest you can add a large 
notice in the commit message(s) and/or comments; this may not prevent 
going for review but reviewers should catch it pretty quickly. To help 
prevent it even getting that far you may be able to add a pre-push hook 
to prevent such commits from being pushed to somewhere other than a 
private fork or a branch with a name that clearly indicates that it 
contains experimental code.


Re: git describe number of commits different from git log count

2016-12-05 Thread Aaron Schrab

At 12:17 +0100 05 Feb 2016, Jan Hudec  wrote:

I have a repository with following situation:

   $ git describe next
   v4.1-2196-g5a414d7
   $ git describe next --match=v4.2
   v4.2-4757-g5a414d7

Since the tag with fewest commits since is selected, it appears logical.
However, v4.2 is descendant of v4.1, so it does not make sense for it to have
more commits since. And rev-list or log confirm that:

   $ git rev-list v4.1..next | wc -l
   2196
   $ git rev-list v4.2..next | wc -l
   1152

The number of commits since v4.1 matches what the describe counted, but the
number of commits since v4.2 does not.


I'm encountering what seems to be a similar issue. I'm working with the 
`build` branch of https://github.com/aschrab/mutt currently at 65b7094.  
Most of the commits in that repo come from a mercurial repository, but I 
don't think that is really effecting things (other than being related to 
the need to use the --tags option).


 $ git describe --tags
 mutt-1-7-1-rel-137-g65b7094

 $ git describe --tags --match=mutt-1-7-2-rel
 mutt-1-7-2-rel-6246-g65b7094

 $ git rev-list --count mutt-1-7-2-rel..HEAD
 126

 $ git rev-list --count mutt-1-7-1-rel..HEAD
 137

 $ git --version
 git version 2.10.2

The number of additional commits shown when I force the tag is 
completely insane! That's nearly every commit that is part of that 
branch:


 1036$ git rev-list --count HEAD
 6250

Both of the tags above should be reachable along the first-parent path.  
If I add the `--first-parent` option to the describe command it picks 
the expected tag and the number additional commits seems sane:


 $ git describe --tags --first-parent
 mutt-1-7-2-rel-14-g65b7094


A couple errors dealing with uninitialized submodules

2016-10-20 Thread Aaron Schrab
I was working with a fresh clone of a repository where I'd forgotten 
that one of the directories was setup as a submodule, so I hadn't 
initialized it.


I tried to add a symlink to a location outside the repository and this 
failed with an assertion (exact text in comment below). When looking 
into that I realized that the directory was meant to be a submodule and 
decided to try to change that.  So I tried to remove the submodule, and 
got an error (misleadingly) saying that couldn't be done because it uses 
a .git directory.


I first noticed this with git 2.9.3 from Debian unstable, but I also see 
it building from v2.10.1-502-g6598894 fetched from master recently.


The following script replicates both of these issues. These could both 
be classified as "don't do that", although at lease the assertion is 
quite ugly.


--- >8 
#!/bin/sh -e

directory=$(mktemp -d)
echo "Using directory '$directory'"
cd $directory
git init --quiet orig
(
 cd orig
 # Using a random, small repository for the submodule.
 git submodule --quiet add https://github.com/diepm/vim-rest-console.git sub
 git commit -m init >/dev/null
)
git clone --quiet orig dup
cd dup

(
 cd sub
 ln -s /tmp/dont_care
 # Next command aborts with
 # git: pathspec.c:317: prefix_pathspec: Assertion `item->nowildcard_len <= item->len && 
item->prefix <= item->len' failed.`
 git add . || : expected to fail
)

rm -f .git/index.lock
# Next command fails with error wrongly saying that the submodule uses a .git
# directory.  I believe that the real problem is that the uninitialized
# submodule has content.
git rm sub


Re: [PATCH 18/18] alternates: use fspathcmp to detect duplicates

2016-10-04 Thread Aaron Schrab

At 16:36 -0400 03 Oct 2016, Jeff King  wrote:

On a case-insensitive filesystem, we should realize that
"a/objects" and "A/objects" are the same path.


The current repository being on a case-insensitive filesystem doesn't 
guarantee that the alternates are as well.


On the other hand, I suspect that people who use a case-insensitive 
filesystem would be less likely to use names which differ only by case.


Re: [Opinion gathering] Git remote whitelist/blacklist

2016-05-24 Thread Aaron Schrab

At 14:55 +0200 24 May 2016, Matthieu Moy  wrote:

So, when trying a forbidden push, Git would deny it and the only way to
force the push would be to remove the blacklist from the config, right?

Probably the sanest way to go. I thought about adding a "git push
--force-even-if-in-blacklist" or so, but I don't think the feature
deserves one specific option (hence add some noise in `git push -h`).


It might make sense to bypass the blacklist checking if the existing 
--no-verify is used.  In the past I've used a pre-push hook to implement 
a similar method of preventing accidental pushes, and found that to be a 
good way to skip the checking when I wanted to override the check for a 
specific push.  The builtin blacklist checking could be seen as another 
type of verification.  The downside to that would be that if the 
blacklist was used along with a pre-push hook for different types of 
checks users would likely only be able to see the error message from one 
of them; but that could also apply to a pre-push hook that implements 
different types of checks and short circuits at the first failure.

--
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 1/2] send-email: new option to quote an email and reply to

2016-05-24 Thread Aaron Schrab

At 14:49 +0200 24 May 2016, Matthieu Moy  wrote:

Samuel GROOT  writes:


What kind of help text would you want to see?

Maybe something like this:

  GIT: Quoted message body below.
  GIT: Feel free to trim down the quoted text
  GIT: to only relevant portions.

As "GIT:" portions are ignored when parsed by `git send-email`.


That's an option, but in the context of email, I think these
instructions are not necessary.


In an ideal world that would be true.  But in the real world I think 
evidence of many messages to this mailing list containing full quotes 
suggests it might be helpful. I'd actually argue that the message be 
more forceful, making it a suggestion/request to trim rather than simply 
telling the user that it's allowed.

--
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: Redirect git subcommand to itself?

2015-05-29 Thread Aaron Schrab

At 10:38 +0200 29 May 2015, Christian Neukirchen chneukirc...@gmail.com wrote:

Junio C Hamano gits...@pobox.com writes:


 * You can help yourself with something like this, I suppose:

   [alias]
git = !sh -c 'exec git \$@\' -

   but I personally feel that it is too ugly to live as part of our
   official suggestion, so please do not send a patch to add it as
   a built-in alias ;-).


So I thought I was clever, but this didn't work:


I've done the same as the original poster a few times myself, so awhile 
ago I added the following to my .gitconfig file:


[alias]
   git = !f() { git $@; }; f

And this has worked for me.
--
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: difftool: honor --trust-exit-code for builtin tools

2014-11-17 Thread Aaron Schrab

At 10:11 -0800 16 Nov 2014, Junio C Hamano gits...@pobox.com wrote:

It does not have any significance that a random shell implementation
is not POSIX compliant.  That would merely mean that such a shell
cannot be used to run POSIX shell scripts like our Porcelain.


Right, and I suspect that it's very rare for zsh to be used as /bin/sh.  
I've heard of people doing it just to see what would fail, but not of 
anybody doing that for regular use.



I would suspect that zsh has more posixly correct mode, with which
it _can_ run POSIX shell scripts, and I would imagine that this 
$status is an alias $? business is disabled in that mode? 


Yes, if zsh is invoked as either sh or ksh it attempts to emulate 
the usual semantics of the named shell.  One of the differences is that 
$status isn't special in the emulation modes.

--
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


Merge without marking conflicts in working tree

2014-11-17 Thread Aaron Schrab
Is there a way to do a merge but only record conflicts in the index, not 
update the working versions of files with conflict markers?


Like many people, I use git to manage configuration files for my shell, 
editor, git itself, and a number of other things.  The vast majority of 
times that I update things no conflicts are occur and everything just 
works, so I'd like to avoid extra work in this case.  But occasionally a 
conflict will occur, and if it's in a file that will be read while 
trying to resolve the conflict this can make things more difficult.


I'd like to find a way to have the conflict recorded in just the index 
without touching the working tree.  I could then use my usual tools to 
resolve the conflict without the errors caused by the conflict markers.  
I generally use vim+fugitive to resolve conflicts anyway, and typically 
the first step I take is to replace the working-tree version with the 
merge-base version, completely ignoring any conflict markers.


If there isn't currently a way to do this, I was thinking of 
implementing something like an ours value for merge.conflictstyle 
configuration.

--
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] CEST is +0200 during April

2013-08-17 Thread Aaron Schrab
Mattias, the convention on this list is to send directly to participants 
in the thread as well as the list.  So I've added Andreas as a 
recipient.


At 05:52 +0200 17 Aug 2013, Mattias Andrée maand...@operamail.com wrote:

-   For example CET (which is 2 hours ahead UTC) is `+0200`.
+   For example CEST (which is 2 hours ahead UTC) is `+0200`.


While changing that line it would be good to also fix the grammar:

+   For example CEST (which is 2 hours ahead of UTC) is `+0200`.
^^
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] config: add support for http.url.* settings

2013-07-12 Thread Aaron Schrab

At 06:07 -0700 12 Jul 2013, Kyle J. McKay mack...@gmail.com wrote:
I don't think it's necessary to split the URL apart though.  Whatever 
URL the user gave to git on the command line (at some point even if 
it's now stored as a remote setting in config) complete with URL-
encoding, user names, port names, etc. is the same url, possibly 
shortened, that needs to be used for the http.url.option setting.


This seems to be assuming that the configuration is done after the URL 
is entered and that URLs are always entered manually.  I don't think 
either of those assumptions is valid.  A user may want to specify http 
settings for all repositories on a specified host and so add settings 
for that host to ~/.gitconfig expecting those settings to be used later.  
A URL in a slightly different format may later be copy+pasted without 
the user realizing that it won't use that config due to one of the 
versions being in a non-canonical form.


I think that's simple and very easy to explain and avoids user 
confusion and surprise while still allowing a default to be set for a 
site but easily overridden for a portion of that site without needing 
to worry about the order config files are processed or the order of the 
[http url] sections within them.


I agree that the method is easy to explain, but I think a user may very 
well be surprised and confused in a scenario like I described above.  
And having the order not matter (in some cases) for these configuration 
items deviates from how other configuration values are handled.

--
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 7/7] push: document --lockref

2013-07-09 Thread Aaron Schrab

At 12:53 -0700 09 Jul 2013, Junio C Hamano gits...@pobox.com wrote:

+This is meant to make `--force` safer to use.  Imagine that you have
+to rebase what you have already published.  You will have to
+`--force` the push to replace the history you originally published
+with the rebased history.  If somebody else built on top of your
+original history while you are rebasing, the tip of the branch at
+the remote may advance with her commit, and blindly pushing with
+`--force` will lose her work.  By using this option to specify that
+you expect the history you are updating is what you rebased and want
+to replace, you can make sure other people's work will not be losed
+by a forced push. in such a case.


s/losed/lost/

How does this behave if --force is not used?  I think it would be best 
if it was a no-op in that case to make it easy to add a config option to 
turn this on by default.

--
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 4/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Aaron Schrab

At 13:05 +0100 11 Apr 2013, Adam Spiers g...@adamspiers.org wrote:

The above use case suggests that empty STDIN is actually a reasonable
scenario (e.g. when the caller doesn't know in advance whether any
queries need to be fed to the background process until after it's
already started), so we make the minor behavioural change that no
pathspec given. is no longer emitted in when STDIN is empty.


The last in there looks to be misplaced.  Was that originally 
something like in the case?  If so the removed words should be 
restored or the lingering one removed as well.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference

2013-04-09 Thread Aaron Schrab

At 17:24 -0700 08 Apr 2013, Jonathan Nieder jrnie...@gmail.com wrote:

+test_expect_success 'clone using repo with gitfile as a reference' '
+   git clone --separate-git-dir=L A M 
+   git clone --reference=M A N 


What should happen if I pass --reference=M/.git?


That isn't supported and I wouldn't expect it to work.  The --reference 
option is documented as taking the location of a repository as the 
argument and I wouldn't consider a .git file to be a repository.  I also 
can't think of a reason that it would be very useful since it should be 
simple to just refer to the directory containing the .git file.  But if 
others disagree, I could be convinced to add support for that.


I also wouldn't consider it breakage if that use would start working, so 
I don't see a point in adding a test to check that that usage fails.



+   echo $base_dir/L/objects  expected 


The usual style in tests is to include no space after redirection
operators.


Fixed for the next version, pending further comments.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference

2013-04-09 Thread Aaron Schrab

At 09:47 -0700 09 Apr 2013, Junio C Hamano ju...@pobox.com wrote:

Aaron Schrab aa...@schrab.com writes:

But if others disagree, I could be convinced to add support for that.


If M/.git weren't a gitfile that points elsewhere, that request
ought to work, no?  A gitfile is the moral equilvalent of a symbolic
link, meant to help people on platforms and filesystems that lack
symbolic links, so in that sense, not supporting the case goes
against the whole reason why we have added support for gitfile in
the first place, I think.


OK, I'm convinced.  I'll modify it to support that as well.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/2] Using gitfile repository with clone --reference

2013-04-09 Thread Aaron Schrab
Here's the third version of my series for dealing with gitfiles in clone
--reference.

The first patch is unchanged from the previous version except for the
addition of a Reviewed-by line.

The second patch has been modified so that it now supports having a .git
file supplied as the argument to the option directly rather than only
dealing with that if the containing directory was supplied.  This makes
the first patch from the series more important, since it would make even
less sense to complain that the path isn't a directory when a
non-directory is acceptable.

I've also fixed the minor style issue in the test script from the previous
versions.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/2] clone: Fix error message for reference repository

2013-04-09 Thread Aaron Schrab
Do not report that an argument to clone's --reference option is not a
local directory.  Nothing checks for the existence or type of the path
as supplied by the user; checks are only done for particular contents of
the supposed directory, so we have no way to know the status of the
supplied path.  Telling the user that a directory doesn't exist when
that isn't actually known may lead him or her on the wrong path to
finding the problem.

Instead just state that the entered path is not a local repository which
is really all that is known about it.  It could be more helpful to state
the actual paths which were checked, but I believe that giving a good
description of that would be too verbose for a simple error message and
would be too dependent on implementation details.

Signed-off-by: Aaron Schrab aa...@schrab.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

diff --git a/builtin/clone.c b/builtin/clone.c
index f9c380e..0a1e0bf 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, 
void *cb_data)
free(ref_git);
ref_git = ref_git_git;
} else if (!is_directory(mkpath(%s/objects, ref_git)))
-   die(_(reference repository '%s' is not a local directory.),
+   die(_(reference repository '%s' is not a local repository.),
item-string);
 
strbuf_addf(alternate, %s/objects, ref_git);
-- 
1.8.2.677.g9202ef0

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/2] clone: Allow repo using gitfile as a reference

2013-04-09 Thread Aaron Schrab
Try reading gitfile files when processing --reference options to clone.
This will allow, among other things, using a submodule checked out with
a recent version of git as a reference repository without requiring the
user to have internal knowledge of submodule layout.

Signed-off-by: Aaron Schrab aa...@schrab.com

diff --git a/builtin/clone.c b/builtin/clone.c
index 0a1e0bf..58fee98 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -232,11 +232,21 @@ static void strip_trailing_slashes(char *dir)
 static int add_one_reference(struct string_list_item *item, void *cb_data)
 {
char *ref_git;
+   const char *repo;
struct strbuf alternate = STRBUF_INIT;
 
-   /* Beware: real_path() and mkpath() return static buffer */
+   /* Beware: read_gitfile(), real_path() and mkpath() return static 
buffer */
ref_git = xstrdup(real_path(item-string));
-   if (is_directory(mkpath(%s/.git/objects, ref_git))) {
+
+   repo = read_gitfile(ref_git);
+   if (!repo)
+   repo = read_gitfile(mkpath(%s/.git, ref_git));
+   if (repo) {
+   free(ref_git);
+   ref_git = xstrdup(repo);
+   }
+
+   if (!repo  is_directory(mkpath(%s/.git/objects, ref_git))) {
char *ref_git_git = mkpathdup(%s/.git, ref_git);
free(ref_git);
ref_git = ref_git_git;
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 2a7b78b..719d778 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -185,4 +185,17 @@ test_expect_success 'fetch with incomplete alternates' '
! grep  want $tag_object $U.K
 '
 
+test_expect_success 'clone using repo with gitfile as a reference' '
+   git clone --separate-git-dir=L A M 
+   git clone --reference=M A N 
+   echo $base_dir/L/objects expected 
+   test_cmp expected $base_dir/N/.git/objects/info/alternates
+'
+
+test_expect_success 'clone using repo pointed at by gitfile as reference' '
+   git clone --reference=M/.git A O 
+   echo $base_dir/L/objects expected 
+   test_cmp expected $base_dir/O/.git/objects/info/alternates
+'
+
 test_done
-- 
1.8.2.677.g9202ef0

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] clone: Fix error message for reference repository

2013-04-08 Thread Aaron Schrab

At 06:58 -0700 08 Apr 2013, Junio C Hamano gits...@pobox.com wrote:

I do agree that it would be nice to dereference .git gitfile when we
deal with --reference argument, but you do not want to use in-tree
repository of a submodule working tree.  What happens when you have
to check out a version of the containing superproject that did not
have the submodule you are borrowing from?  The directory will
disappear, leaving the borrowing repository still pointing at it
with its .git/objects/info/alternates file, no?


No, submodule directories don't get removed when you checkout a version 
which didn't contain that submodule.  I believe that there are plans to 
change that for submodules which store the repository data under the 
containing project's .git directory; but removing the submodule working 
tree would not affect a repository using that submodule as a reference, 
since the reading of the .git file is only done during the initial 
clone.  I don't think that the risk of such a repository being deleted 
or moved is substantially higher than for any other repository.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] clone: Fix error message for reference repository

2013-04-08 Thread Aaron Schrab

At 08:30 -0700 08 Apr 2013, Junio C Hamano gits...@pobox.com wrote:

You switch to a version of the superproject with a plain file at
dirA/ or there is nothing at dirA.  The checkout will fail and you
need to manually rectify the situation [*1*], but after that is
done, you do not have any repository at /path/to/super/dirA/.git
anymore.

That was the reason why I recommended against the practice.


So you're essentially saying you don't want to support using a new-world 
submodule as a reference because using an old-world submodule as such is 
likely to be problematic?  Even though the type of submodule that is 
actually likely to cause problems would currently be accepted as a 
reference repository?  That seems somewhat perverse to me.


Also, nothing in this series is strictly about submodules; that just 
happens to be what I was working with when I noticed the issue.  It 
would apply to any repository created with --separate-git-dir, although 
submodules are likely to be the most common occurrence by far.



So you are right that we do not remove in the new world order, but
then --reference can be given to point at the real location ;-)


Yes, that's definitely a possibility.  But I think that the location of 
the work tree for a repository is much more likely to come to a user's 
mind than the location of a non-bare repository.  Especially when 
dealing with submodules where the repository location was decided for 
the user, and is somewhat of an implementation detail that the user 
shouldn't need to care about.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] clone: Fix error message for reference repository

2013-04-08 Thread Aaron Schrab

At 10:57 -0700 08 Apr 2013, Junio C Hamano gits...@pobox.com wrote:

In general I am in favor of resolving a gitfile given to --reference
when clone interprets it, and have it use the location of the real
underlying object store when it grabs objects not in there from the
origin and store the location of the real underlying object store in
the objects/info/alternates of the newly created repository.  But
that is not limited to the gitfile used at the root level of a
submodule checkout.


Yes, I agree that it's not limited to submodules.  The commit message 
for the second part of this series only mentioned submodules because I 
suspect that is by far the most common use of gitfiles.  The commit 
message for the first didn't even mention submodules at all, they were 
only brought up because I was asked about what lead to me having an 
issue.



Blindly using .git at the root level of submodule checkout as a
reference is what I was recommending against as a general
precaution.


I agree with that.  But I still don't think it's relevant to this patch 
series.



You may be dealing with an old-style submodule checkout.


No, the submodule in question was done with the new style.  If it were 
an old-style checkout my attempt to clone using that as a reference 
would have worked without issue (at least at clone time).

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference

2013-04-08 Thread Aaron Schrab

At 11:00 -0700 08 Apr 2013, Junio C Hamano gits...@pobox.com wrote:

Aaron Schrab aa...@schrab.com writes:

Good catch.  I'll fix that in the next version.


Thanks.  The patch otherwise looks good to me.


Great, I'll plan to send version 2 of this series later today.
--
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/2] Using gitfile repository with clone --reference

2013-04-08 Thread Aaron Schrab
Here's the promised second version of this series.

The diff in the first patch is unchanged, but I have made significant
changes to the commit message to hopefully to a better job of describing
why I think the old error message is bad.

For the second patch I've eliminated the need to do a cast.

Although I'm sending these as a series, the changes are independent both
textually and semantically.

--
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 1/2] clone: Fix error message for reference repository

2013-04-08 Thread Aaron Schrab
Do not report that an argument to clone's --reference option is not a
local directory.  Nothing checks for the existence or type of the path
as supplied by the user; checks are only done for particular contents of
the supposed directory, so we have no way to know the status of the
supplied path.  Telling the user that a directory doesn't exist when
that isn't actually known may lead him or her on the wrong path to
finding the problem.

Instead just state that the entered path is not a local repository which
is really all that is known about it.  It could be more helpful to state
the actual paths which were checked, but I believe that giving a good
description of that would be too verbose for a simple error message and
would be too dependent on implementation details.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 builtin/clone.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f9c380e..0a1e0bf 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, 
void *cb_data)
free(ref_git);
ref_git = ref_git_git;
} else if (!is_directory(mkpath(%s/objects, ref_git)))
-   die(_(reference repository '%s' is not a local directory.),
+   die(_(reference repository '%s' is not a local repository.),
item-string);
 
strbuf_addf(alternate, %s/objects, ref_git);
-- 
1.7.10.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 2/2] clone: Allow repo using gitfile as a reference

2013-04-07 Thread Aaron Schrab
Try reading gitfile files when processing --reference options to clone.
This will allow, among other things, using a submodule checked out with
a recent version of git as a reference repository without requiring the
user to have internal knowledge of submodule layout.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 builtin/clone.c| 13 ++---
 t/t5700-clone-reference.sh |  7 +++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 0a1e0bf..376ded8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -231,12 +231,19 @@ static void strip_trailing_slashes(char *dir)
 
 static int add_one_reference(struct string_list_item *item, void *cb_data)
 {
-   char *ref_git;
+   char *ref_git, *repo;
struct strbuf alternate = STRBUF_INIT;
 
-   /* Beware: real_path() and mkpath() return static buffer */
+   /* Beware: read_gitfile(), real_path() and mkpath() return static 
buffer */
ref_git = xstrdup(real_path(item-string));
-   if (is_directory(mkpath(%s/.git/objects, ref_git))) {
+
+   repo = (char *)read_gitfile(mkpath(%s/.git, ref_git));
+   if (repo) {
+   free(ref_git);
+   ref_git = xstrdup(repo);
+   }
+
+   if (!repo  is_directory(mkpath(%s/.git/objects, ref_git))) {
char *ref_git_git = mkpathdup(%s/.git, ref_git);
free(ref_git);
ref_git = ref_git_git;
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 2a7b78b..7a9044c 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -185,4 +185,11 @@ test_expect_success 'fetch with incomplete alternates' '
! grep  want $tag_object $U.K
 '
 
+test_expect_success 'clone using repo with gitfile as a reference' '
+   git clone --separate-git-dir=L A M 
+   git clone --reference=M A N 
+   echo $base_dir/L/objects  expected 
+   test_cmp expected $base_dir/N/.git/objects/info/alternates
+'
+
 test_done
-- 
1.8.2.677.g7422c62

--
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 1/2] clone: Fix error message for reference repository

2013-04-07 Thread Aaron Schrab
Do not report an argument to clone's --reference option is not a local
directory.  Nothing checks for the actual directory so we have no way to
know if whether or not exists.  Telling the user that a directory doesn't
exist when that isn't actually known may lead him or her on the wrong
path to finding the problem.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 builtin/clone.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f9c380e..0a1e0bf 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, 
void *cb_data)
free(ref_git);
ref_git = ref_git_git;
} else if (!is_directory(mkpath(%s/objects, ref_git)))
-   die(_(reference repository '%s' is not a local directory.),
+   die(_(reference repository '%s' is not a local repository.),
item-string);
 
strbuf_addf(alternate, %s/objects, ref_git);
-- 
1.8.2.677.g7422c62

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] clone: Fix error message for reference repository

2013-04-07 Thread Aaron Schrab

At 16:48 -0700 07 Apr 2013, Jonathan Nieder jrnie...@gmail.com wrote:

Hi Aaron,


Thanks for the feedback.


Aaron Schrab wrote:


Do not report an argument to clone's --reference option is not a local
directory.  Nothing checks for the actual directory so we have no way to
know if whether or not exists.  Telling the user that a directory doesn't
exist when that isn't actually known may lead him or her on the wrong
path to finding the problem.


I don't understand the above explanation.  Could you give an example?


I originally noticed this while trying to use a submodule as a reference 
repository.  Since that submodule was first checked out using a recent 
version of git it used a .git file rather than having a .git directory.  
This caused the checks to fail, and the misleading error message had me 
checking for a typo in the path which I'd supplied.


I'll attempt to clarify that message in the next version.


--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -241,7 +241,7 @@ static int add_one_reference(struct string_list_item *item, 
void *cb_data)
free(ref_git);
ref_git = ref_git_git;
} else if (!is_directory(mkpath(%s/objects, ref_git)))
-   die(_(reference repository '%s' is not a local directory.),
+   die(_(reference repository '%s' is not a local repository.),


is_directory calls stat and checks if its target is a directory.  Is
the problem that /path/to/repo.git might be a directory but
/path/to/repo.git/objects may not?


In my case the issue was that /path/to/repo is a directory, but 
/path/to/repo/.git/objects (which is checked shortly before the above 
context) didn't exist since /path/to/repo/.git is a file.



Would it make sense for the message to say something like the
following?

fatal: alternate object store '/path/to/repo.git/objects' is not a 
local directory


That would also avoid lying to the user.  But if combined with the 
second patch in this series it could cause confusion for a different 
reason.  Once .git files are honored, the path reported there may have 
no relation to the path supplied by the user.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] clone: Allow repo using gitfile as a reference

2013-04-07 Thread Aaron Schrab

At 16:51 -0700 07 Apr 2013, Jonathan Nieder jrnie...@gmail.com wrote:

-   char *ref_git;
+   char *ref_git, *repo;

[...]

+   repo = (char *)read_gitfile(mkpath(%s/.git, ref_git));


Why not make repo a const char * and avoid the cast?  The above
would seem to make it too tempting to treat the return value from
read_gitfile() as a mutable buffer instead of a real_path string that
should be copied asap.


Good catch.  I'll fix that in the next version.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] clone: Fix error message for reference repository

2013-04-07 Thread Aaron Schrab

At 20:06 -0400 07 Apr 2013, I wrote:

At 16:48 -0700 07 Apr 2013, Jonathan Nieder jrnie...@gmail.com wrote:

Would it make sense for the message to say something like the
following?

fatal: alternate object store '/path/to/repo.git/objects' is not a 
local directory


That would also avoid lying to the user.  But if combined with the 
second patch in this series it could cause confusion for a different 
reason.  Once .git files are honored, the path reported there may have 
no relation to the path supplied by the user.


Thinking on this further, even without the companion patch there's 
another issue.  The problem isn't just that 
/path/supplied/by/user/objects isn't a directory.  It's that neither 
that nor /path/supplied/by/user/.git/objects is a directory.  And in 
many cases it's the latter that the user would be expecting to have been 
used.  Reporting on just the last name checked isn't really a good 
description of what's going on.

--
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/3] pre-push hook support

2013-01-12 Thread Aaron Schrab
Main changes since the initial version:

 * The first patch converts the existing hook callers to use the new
   find_hook() function.
 * Information about what is to be pushed is now sent over a pipe rather
   than passed as command-line parameters.

Aaron Schrab (3):
  hooks: Add function to check if a hook exists
  push: Add support for pre-push hooks
  Add sample pre-push hook script

 Documentation/githooks.txt   |  29 +
 builtin/commit.c |   6 +-
 builtin/push.c   |   1 +
 builtin/receive-pack.c   |  25 
 run-command.c|  15 -
 run-command.h|   1 +
 t/t5571-pre-push-hook.sh | 129 +++
 templates/hooks--pre-push.sample |  53 
 transport.c  |  60 ++
 transport.h  |   1 +
 10 files changed, 300 insertions(+), 20 deletions(-)
 create mode 100755 t/t5571-pre-push-hook.sh
 create mode 100644 templates/hooks--pre-push.sample

-- 
1.8.1.340.g425b78d

--
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 1/3] hooks: Add function to check if a hook exists

2013-01-12 Thread Aaron Schrab
Create find_hook() function to determine if a given hook exists and is
executable.  If it is, the path to the script will be returned,
otherwise NULL is returned.

This encapsulates the tests that are used to check for the existence of
a hook in one place, making it easier to modify those checks if that is
found to be necessary.  This also makes it simple for places that can
use a hook to check if a hook exists before doing, possibly lengthy,
setup work which would be pointless if no such hook is present.

The returned value is left as a static value from get_pathname() rather
than a duplicate because it is anticipated that the return value will
either be used as a boolean, immediately added to an argv_array list
which would result in it being duplicated at that point, or used to
actually run the command without much intervening work.  Callers which
need to hold onto the returned value for a longer time are expected to
duplicate the return value themselves.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 builtin/commit.c   |  6 ++
 builtin/receive-pack.c | 25 +++--
 run-command.c  | 15 +--
 run-command.h  |  1 +
 4 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..65d08d2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1327,8 +1327,6 @@ static int git_commit_config(const char *k, const char 
*v, void *cb)
return git_status_config(k, v, s);
 }
 
-static const char post_rewrite_hook[] = hooks/post-rewrite;
-
 static int run_rewrite_hook(const unsigned char *oldsha1,
const unsigned char *newsha1)
 {
@@ -1339,10 +1337,10 @@ static int run_rewrite_hook(const unsigned char 
*oldsha1,
int code;
size_t n;
 
-   if (access(git_path(post_rewrite_hook), X_OK)  0)
+   argv[0] = find_hook(post-rewrite);
+   if (!argv[0])
return 0;
 
-   argv[0] = git_path(post_rewrite_hook);
argv[1] = amend;
argv[2] = NULL;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ff781fe..e8878de 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -182,9 +182,6 @@ struct command {
char ref_name[FLEX_ARRAY]; /* more */
 };
 
-static const char pre_receive_hook[] = hooks/pre-receive;
-static const char post_receive_hook[] = hooks/post-receive;
-
 static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 
2)));
 static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 
2)));
 
@@ -242,10 +239,10 @@ static int run_and_feed_hook(const char *hook_name, 
feed_fn feed, void *feed_sta
const char *argv[2];
int code;
 
-   if (access(hook_name, X_OK)  0)
+   argv[0] = find_hook(hook_name);
+   if (!argv[0])
return 0;
 
-   argv[0] = hook_name;
argv[1] = NULL;
 
memset(proc, 0, sizeof(proc));
@@ -331,15 +328,14 @@ static int run_receive_hook(struct command *commands, 
const char *hook_name,
 
 static int run_update_hook(struct command *cmd)
 {
-   static const char update_hook[] = hooks/update;
const char *argv[5];
struct child_process proc;
int code;
 
-   if (access(update_hook, X_OK)  0)
+   argv[0] = find_hook(update);
+   if (!argv[0])
return 0;
 
-   argv[0] = update_hook;
argv[1] = cmd-ref_name;
argv[2] = sha1_to_hex(cmd-old_sha1);
argv[3] = sha1_to_hex(cmd-new_sha1);
@@ -532,24 +528,25 @@ static const char *update(struct command *cmd)
}
 }
 
-static char update_post_hook[] = hooks/post-update;
-
 static void run_update_post_hook(struct command *commands)
 {
struct command *cmd;
int argc;
const char **argv;
struct child_process proc;
+   char *hook;
 
+   hook = find_hook(post-update);
for (argc = 0, cmd = commands; cmd; cmd = cmd-next) {
if (cmd-error_string || cmd-did_not_exist)
continue;
argc++;
}
-   if (!argc || access(update_post_hook, X_OK)  0)
+   if (!argc || !hook)
return;
+
argv = xmalloc(sizeof(*argv) * (2 + argc));
-   argv[0] = update_post_hook;
+   argv[0] = hook;
 
for (argc = 1, cmd = commands; cmd; cmd = cmd-next) {
char *p;
@@ -704,7 +701,7 @@ static void execute_commands(struct command *commands, 
const char *unpacker_erro
   0, cmd))
set_connectivity_errors(commands);
 
-   if (run_receive_hook(commands, pre_receive_hook, 0)) {
+   if (run_receive_hook(commands, pre-receive, 0)) {
for (cmd = commands; cmd; cmd = cmd-next) {
if (!cmd-error_string)
cmd-error_string = pre-receive hook declined;
@@ -994,7 +991,7 @@ int cmd_receive_pack(int argc, const char **argv

[PATCH v2 2/3] push: Add support for pre-push hooks

2013-01-12 Thread Aaron Schrab
Add support for a pre-push hook which can be used to determine if the
set of refs to be pushed is suitable for the target repository.  The
hook is run with two arguments specifying the name and location of the
destination repository.

Information about what is to be pushed is provided by sending lines of
the following form to the hook's standard input:

  local ref SP local sha1 SP remote ref SP remote sha1 LF

If the hook exits with a non-zero status, the push will be aborted.

This will allow the script to determine if the push is acceptable based
on the target repository and branch(es), the commits which are to be
pushed, and even the source branches in some cases.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 Documentation/githooks.txt |  29 ++
 builtin/push.c |   1 +
 t/t5571-pre-push-hook.sh   | 129 +
 transport.c|  60 +
 transport.h|   1 +
 5 files changed, 220 insertions(+)
 create mode 100755 t/t5571-pre-push-hook.sh

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..d839233 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -176,6 +176,35 @@ save and restore any form of metadata associated with the 
working tree
 (eg: permissions/ownership, ACLS, etc).  See contrib/hooks/setgitperms.perl
 for an example of how to do this.
 
+pre-push
+
+
+This hook is called by 'git push' and can be used to prevent a push from taking
+place.  The hook is called with two parameters which provide the name and
+location of the destination remote, if a named remote is not being used both
+values will be the same.
+
+Information about what is to be pushed is provided on the hook's standard
+input with lines of the form:
+
+  local ref SP local sha1 SP remote ref SP remote sha1 LF
+
+For instance, if the command +git push origin master:foreign+ were run the
+hook would receive a line like the following:
+
+  refs/heads/master 67890 refs/heads/foreign 12345
+
+although the full, 40-character SHA1s would be supplied.  If the foreign ref
+does not yet exist the `remote SHA1` will be 40 `0`.  If a ref is to be
+deleted, the `local ref` will be supplied as `(delete)` and the `local
+SHA1` will be 40 `0`.  If the local commit was specified by something other
+than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be
+supplied as it was originally given.
+
+If this hook exits with a non-zero status, 'git push' will abort without
+pushing anything.  Information about why the push is rejected may be sent
+to the user by writing to standard error.
+
 [[pre-receive]]
 pre-receive
 ~~~
diff --git a/builtin/push.c b/builtin/push.c
index 8491e43..b158028 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -407,6 +407,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, progress, progress, N_(force progress 
reporting)),
OPT_BIT(0, prune, flags, N_(prune locally removed refs),
TRANSPORT_PUSH_PRUNE),
+   OPT_BIT(0, no-verify, flags, N_(bypass pre-push hook), 
TRANSPORT_PUSH_NO_HOOK),
OPT_END()
};
 
diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
new file mode 100755
index 000..d68fed7
--- /dev/null
+++ b/t/t5571-pre-push-hook.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+
+test_description='check pre-push hooks'
+. ./test-lib.sh
+
+# Setup hook that always succeeds
+HOOKDIR=$(git rev-parse --git-dir)/hooks
+HOOK=$HOOKDIR/pre-push
+mkdir -p $HOOKDIR
+write_script $HOOK EOF
+exit 0
+EOF
+
+test_expect_success 'setup' '
+   git config push.default upstream 
+   git init --bare repo1 
+   git remote add parent1 repo1 
+   test_commit one 
+   git push parent1 HEAD:foreign
+'
+write_script $HOOK EOF
+exit 1
+EOF
+
+COMMIT1=$(git rev-parse HEAD)
+export COMMIT1
+
+test_expect_success 'push with failing hook' '
+   test_commit two 
+   test_must_fail git push parent1 HEAD
+'
+
+test_expect_success '--no-verify bypasses hook' '
+   git push --no-verify parent1 HEAD
+'
+
+COMMIT2=$(git rev-parse HEAD)
+export COMMIT2
+
+write_script $HOOK 'EOF'
+echo $1 actual
+echo $2 actual
+cat actual
+EOF
+
+cat expected EOF
+parent1
+repo1
+refs/heads/master $COMMIT2 refs/heads/foreign $COMMIT1
+EOF
+
+test_expect_success 'push with hook' '
+   git push parent1 master:foreign 
+   diff expected actual
+'
+
+test_expect_success 'add a branch' '
+   git checkout -b other parent1/foreign 
+   test_commit three
+'
+
+COMMIT3=$(git rev-parse HEAD)
+export COMMIT3
+
+cat expected EOF
+parent1
+repo1
+refs/heads/other $COMMIT3 refs/heads/foreign $COMMIT2
+EOF
+
+test_expect_success 'push to default' '
+   git push 
+   diff expected actual
+'
+
+cat expected EOF
+parent1
+repo1
+refs/tags/one $COMMIT1 refs/tags/tag1 $_z40
+HEAD~ $COMMIT2 refs/heads/prev $_z40
+EOF

[PATCH v2 3/3] Add sample pre-push hook script

2013-01-12 Thread Aaron Schrab
Create a sample of a script for a pre-push hook.  The main purpose is to
illustrate how a script may parse the information which is supplied to
such a hook.  The script may also be useful to some people as-is for
avoiding to push commits which are marked as a work in progress.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 templates/hooks--pre-push.sample | 53 
 1 file changed, 53 insertions(+)
 create mode 100644 templates/hooks--pre-push.sample

diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
new file mode 100644
index 000..15ab6d8
--- /dev/null
+++ b/templates/hooks--pre-push.sample
@@ -0,0 +1,53 @@
+#!/bin/sh
+
+# An example hook script to verify what is about to be pushed.  Called by git
+# push after it has checked the remote status, but before anything has been
+# pushed.  If this script exits with a non-zero status nothing will be pushed.
+#
+# This hook is called with the following parameters:
+#
+# $1 -- Name of the remote to which the push is being done
+# $2 -- URL to which the push is being done
+#
+# If pushing without using a named remote those arguments will be equal.
+#
+# Information about the commits which are being pushed is supplied as lines to
+# the standard input in the form:
+#
+#   local ref local sha1 remote ref remote sha1
+#
+# This sample shows how to prevent push of commits where the log message starts
+# with WIP (work in progress).
+
+remote=$1
+url=$2
+
+z40=
+
+IFS=' '
+while read local_ref local_sha remote_ref remote_sha
+do
+   if [ $local_sha = $z40 ]
+   then
+   # Handle delete
+   else
+   if [ $remote_sha = $z40 ]
+   then
+   # New branch, examine all commits
+   range=$local_sha
+   else
+   # Update to existing branch, examine new commits
+   range=$remote_sha..$local_sha
+   fi
+
+   # Check for WIP commit
+   commit=`git rev-list -n 1 --grep '^WIP' $range`
+   if [ -n $commit ]
+   then
+   echo Found WIP commit in $local_ref, not pushing
+   exit 1
+   fi
+   fi
+done
+
+exit 0
-- 
1.8.1.340.g425b78d

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] pre-push hook support

2012-12-29 Thread Aaron Schrab

At 18:01 -0800 28 Dec 2012, Junio C Hamano gits...@pobox.com wrote:

One lesson we learned long time ago while doing hooks is to avoid
unbound number of command line arguments and instead feed them from
the standard input.  I think this should do the same.


Good point.  I had been trying to keep the interface for this hook as 
close as possible to the ones for other client-side hooks on the theory 
that less development effort may go into those than for server-side 
hooks.  But thinking on that more I certainly see that this could easily 
run into limits on argument length on some systems; especially when it's 
likely that each of those arguments is likely to be over 100 bytes long.


I'll work on an updated version which sends the variable length 
information over a pipe, using the command-line arguments only to pass 
the remote name and URL.



How does the hook communicate its decision to the calling Git?

Will it be all-or-none, or I'll allow these but not those?


Currently it just uses the exit code to communicate that back, so it's 
all-or-none.  I think I'll keep that in the updated version as well.


A future enhancement could modify the protocol to support reading from 
the hook's stdout the names of remote refs which are to be rejected, I 
think that just having the option for all-or-nothing is a good starting 
point.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] hooks: Add function to check if a hook exists

2012-12-29 Thread Aaron Schrab

At 18:08 -0800 28 Dec 2012, Junio C Hamano gits...@pobox.com wrote:

Aaron Schrab aa...@schrab.com writes:


Create find_hook() function to determine if a given hook exists and is
executable.  If it is the path to the script will be returned, otherwise
NULL is returned.


Sounds like a sensible thing to do.  To make sure the API is also
sensible, all the existing hooks should be updated to use this API,
no?


I'd been trying to keep the changes limited.  I'll see about modifying 
the existing places that run hooks in v2 of the series.



This is in support for an upcoming run_hook_argv() function which will
expect the full path to the hook script as the first element in the
argv_array.


There is currently a public function called run_hook() that squats
on the good name with a kludgy API that is too specific to using
separate index file.  Back when it was a private helper in the
implementation of git commit, it was perfectly fine, but it was
exported without giving much thought on the API.

If you are introducing a new run_hook_* function, give it a generic
enough API that lets all the existing hook callers to use it.  I
would imagine that the API requirement may be modelled after
run_command() API so that we can pass argv[] and tweak the hook's
environ[], as well as feeding its stdin and possibly reading from
its stdout.  That would be very useful.


I think the attraction of the run_hook() API is its simplicity.  It's 
currently a fairly thin wrapper around the run_command() API.  I suspect 
that if the run_hook() API were made generic enough to support all of 
the existing hook callers it would greatly complicate the existing calls 
to run_hook() while not providing much benefit to hook callers which 
can't currently use it beyond what run_command() offers.


Since I'm going to be changing the interface for this hook in v2 of the 
series so that it will be more complicated than can be readily addressed 
with the run_hook() API (and will have use a fixed number of arguments 
anyway) I'll be dropping the run_hook_argv() function.

--
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 1/4] hooks: Add function to check if a hook exists

2012-12-28 Thread Aaron Schrab
Create find_hook() function to determine if a given hook exists and is
executable.  If it is the path to the script will be returned, otherwise
NULL is returned.

This is in support for an upcoming run_hook_argv() function which will
expect the full path to the hook script as the first element in the
argv_array.  This also makes it simple for places that can use a hook to
check if a hook exists before doing, possibly lengthy, setup work which
would be pointless if no such hook is present.

The returned value is left as a static value from get_pathname() rather
than a duplicate because it is anticipated that the return value will
either be used as a boolean, or immediately added to an argv_array list
which would result in it being duplicated at that point.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 run-command.c |   15 +--
 run-command.h |1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/run-command.c b/run-command.c
index 3b982e4..49c8fa0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -735,6 +735,15 @@ int finish_async(struct async *async)
 #endif
 }
 
+char *find_hook(const char *name)
+{
+   char *path = git_path(hooks/%s, name);
+   if (access(path, X_OK)  0)
+   path = NULL;
+
+   return path;
+}
+
 int run_hook(const char *index_file, const char *name, ...)
 {
struct child_process hook;
@@ -744,11 +753,13 @@ int run_hook(const char *index_file, const char *name, 
...)
va_list args;
int ret;
 
-   if (access(git_path(hooks/%s, name), X_OK)  0)
+   p = find_hook(name);
+   if (!p)
return 0;
 
+   argv_array_push(argv, p);
+
va_start(args, name);
-   argv_array_push(argv, git_path(hooks/%s, name));
while ((p = va_arg(args, const char *)))
argv_array_push(argv, p);
va_end(args);
diff --git a/run-command.h b/run-command.h
index 850c638..221ce33 100644
--- a/run-command.h
+++ b/run-command.h
@@ -45,6 +45,7 @@ int start_command(struct child_process *);
 int finish_command(struct child_process *);
 int run_command(struct child_process *);
 
+extern char *find_hook(const char *name);
 extern int run_hook(const char *index_file, const char *name, ...);
 
 #define RUN_COMMAND_NO_STDIN 1
-- 
1.7.10.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 2/4] hooks: support variable number of parameters

2012-12-28 Thread Aaron Schrab
Define the run_hook_argv() function to allow hooks to be created where
the number of parameters to be passed is variable.  The existing
run_hook() function uses stdarg to allow it to receive a variable number
of arguments, but the number of arguments that a given caller is passing
is fixed at compile time.  This function will allow the caller of a hook
to determine the number of arguments to pass when preparing to call the
hook.

The first use of this function will be for a pre-push hook which will
add an argument for every reference which is to be pushed.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 run-command.c |   20 +---
 run-command.h |2 ++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/run-command.c b/run-command.c
index 49c8fa0..e07202b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,7 +2,6 @@
 #include run-command.h
 #include exec_cmd.h
 #include sigchain.h
-#include argv-array.h
 
 #ifndef SHELL_PATH
 # define SHELL_PATH /bin/sh
@@ -746,10 +745,8 @@ char *find_hook(const char *name)
 
 int run_hook(const char *index_file, const char *name, ...)
 {
-   struct child_process hook;
struct argv_array argv = ARGV_ARRAY_INIT;
-   const char *p, *env[2];
-   char index[PATH_MAX];
+   const char *p;
va_list args;
int ret;
 
@@ -764,6 +761,17 @@ int run_hook(const char *index_file, const char *name, ...)
argv_array_push(argv, p);
va_end(args);
 
+   ret = run_hook_argv(index_file, argv);
+   argv_array_clear(argv);
+   return ret;
+}
+
+int run_hook_argv(const char *index_file, struct argv_array argv)
+{
+   struct child_process hook;
+   char index[PATH_MAX];
+   const char *env[2];
+
memset(hook, 0, sizeof(hook));
hook.argv = argv.argv;
hook.no_stdin = 1;
@@ -775,7 +783,5 @@ int run_hook(const char *index_file, const char *name, ...)
hook.env = env;
}
 
-   ret = run_command(hook);
-   argv_array_clear(argv);
-   return ret;
+   return run_command(hook);
 }
diff --git a/run-command.h b/run-command.h
index 221ce33..12faa5b 100644
--- a/run-command.h
+++ b/run-command.h
@@ -1,6 +1,7 @@
 #ifndef RUN_COMMAND_H
 #define RUN_COMMAND_H
 
+#include argv-array.h
 #ifndef NO_PTHREADS
 #include pthread.h
 #endif
@@ -47,6 +48,7 @@ int run_command(struct child_process *);
 
 extern char *find_hook(const char *name);
 extern int run_hook(const char *index_file, const char *name, ...);
+extern int run_hook_argv(const char *index_file, struct argv_array);
 
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD 2  /*If this is to be git sub-command */
-- 
1.7.10.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 4/4] Add sample pre-push hook script

2012-12-28 Thread Aaron Schrab
Create a sample of a script for a pre-push hook.  The main purpose is to
illustrate how a script may parse the parameters which are supplied to
such a hook.  The script may also be useful to some people as-is for
avoiding to push commits which are marked as a work in progress.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 templates/hooks--pre-push.sample |   63 ++
 1 file changed, 63 insertions(+)
 create mode 100644 templates/hooks--pre-push.sample

diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample
new file mode 100644
index 000..1d3b4a3
--- /dev/null
+++ b/templates/hooks--pre-push.sample
@@ -0,0 +1,63 @@
+#!/bin/sh
+
+# An example hook script to verify what is about to be pushed.
+# Called by git push after it has checked the remote status, but before
+# anything has been pushed.  If this script exits with a non-zero status
+# nothing will be pushed.
+#
+# This hook is called with the following parameters:
+#
+# $1 -- Name of the remote to which the push is being done
+# $2 -- URL to which the push is being done
+#
+#   If pushing without using a named remote those arguments will be equal.
+#
+# Further arguments provide information about the commits which are being
+# pushed in the form:
+#
+#   local ref:local sha1:remote ref:remote sha1
+#
+# This sample shows how to prevent push of commits where the log
+# message starts with WIP (work in progress).
+
+remote=$1
+url=$2
+shift 2
+
+z40=
+
+old_ifs=$IFS
+for to_push in $@
+do
+   # Split the value into its parts
+   IFS=:
+   set -- $to_push
+   IFS=$old_ifs
+
+   local_ref=$1
+   local_sha=$2
+   remote_ref=$3
+   remote_sha=$4
+
+   if [ $local_sha = $z40 ]
+   then
+   range=''
+   # Handle deletes
+   else
+   if [ $remote_sha = $z40 ]
+   then
+   range=$local_sha
+   else
+   range=$remote_sha..$local_sha
+   fi
+
+   commit=`git rev-list -n 1 --grep '^WIP' $range`
+   if [ -n $commit ]
+   then
+   echo Found WIP commit in $local_ref, not pushing
+   exit 1
+   fi
+   fi
+done
+
+exit 0
-- 
1.7.10.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 3/4] push: Add support for pre-push hooks

2012-12-28 Thread Aaron Schrab
Add support for a pre-push hook which can be used to determine if the
set of refs to be pushed is suitable for the target repository.  The
hook should be supplied with:

 1. name of the remote being used, or the URL if not using a named
remote
 2. the URL to which we're pushing
 3. descriptions of what references are to be pushed

Each reference to be pushed should be described in a separate parameter
to the hook script in the form:

  local ref:local sha1:remote ref:remote sha1

This will allow the script to determine if the push is acceptable based
on the target repository and branch(es), the commits which are to be
pushed, and even the source branches in some cases.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 Documentation/githooks.txt |   28 +
 builtin/push.c |1 +
 t/t5571-pre-push-hook.sh   |  145 
 transport.c|   25 
 transport.h|1 +
 5 files changed, 200 insertions(+)
 create mode 100755 t/t5571-pre-push-hook.sh

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..e9539bb 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -176,6 +176,34 @@ save and restore any form of metadata associated with the 
working tree
 (eg: permissions/ownership, ACLS, etc).  See contrib/hooks/setgitperms.perl
 for an example of how to do this.
 
+pre-push
+
+
+This hook is called by 'git push' and can be used to prevent a push from
+taking place.  The hook is called with a variable number of parameters.
+
+The first parameters provide the name and location of the destination
+remote, if a named remote is not being used both values will be the same.
+
+Remaining parameters provide information about the commits which are to be
+pushed and the ref names being used.  These arguments take the form:
+
+  local ref:local sha1:remote ref:remote sha1
+
+For instance, if the command +git push origin master:foreign+ were run the
+hook would be called with a third arugment similar to:
+
+  refs/heads/master:67890:refs/heads/foreign:12345
+
+although the full, 40-character SHA1s would be supplied.  If the foreign ref
+does not yet exist the `remote SHA1` will be 40 `0`.  If a ref is to be
+deleted, the `local ref` will be supplied as `(delete)` and the `local
+SHA1` will be 40 `0`.  If the local commit was specified by something other
+than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be
+supplied as it was originally given.
+
+If this hook exits with a non-zero status, 'git push' will abort.
+
 [[pre-receive]]
 pre-receive
 ~~~
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..c33fb9b 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -399,6 +399,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BOOL(0, progress, progress, N_(force progress 
reporting)),
OPT_BIT(0, prune, flags, N_(prune locally removed refs),
TRANSPORT_PUSH_PRUNE),
+   OPT_BIT(0, no-verify, flags, N_(bypass pre-push hook), 
TRANSPORT_PUSH_NO_HOOK),
OPT_END()
};
 
diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh
new file mode 100755
index 000..5444c9b
--- /dev/null
+++ b/t/t5571-pre-push-hook.sh
@@ -0,0 +1,145 @@
+#!/bin/sh
+
+test_description='check pre-push hooks'
+. ./test-lib.sh
+
+# Setup hook that always succeeds
+HOOKDIR=$(git rev-parse --git-dir)/hooks
+HOOK=$HOOKDIR/pre-push
+mkdir -p $HOOKDIR
+cat $HOOK EOF
+#!/bin/sh
+exit 0
+EOF
+chmod +x $HOOK
+
+test_expect_success 'setup' '
+   git config push.default upstream 
+   git init --bare repo1 
+   git remote add parent1 repo1 
+   test_commit one 
+   git push parent1 HEAD:foreign
+'
+cat $HOOK EOF
+#!/bin/sh
+exit 1
+EOF
+
+COMMIT1=$(git rev-parse HEAD)
+export COMMIT1
+
+test_expect_success 'push with failing hook' '
+   test_commit two 
+   test_must_fail git push parent1 HEAD
+'
+
+test_expect_success '--no-verify bypasses hook' '
+   git push --no-verify parent1 HEAD
+'
+
+COMMIT2=$(git rev-parse HEAD)
+export COMMIT2
+
+cat $HOOK 'EOF'
+#!/bin/sh -ex
+test $# = 3
+test $1 = parent1
+test $2 = repo1
+test $3 = refs/heads/master:$COMMIT2:refs/heads/foreign:$COMMIT1
+EOF
+
+test_expect_success 'push with hook' '
+   git push parent1 master:foreign
+'
+
+test_expect_success 'add a branch' '
+   git checkout -b other 
+   test_commit three
+'
+
+COMMIT3=$(git rev-parse HEAD)
+export COMMIT3
+
+cat $HOOK 'EOF'
+#!/bin/sh -ex
+test $# = 4
+test $1 = parent1
+test $2 = repo1
+test $3 = refs/heads/other:$COMMIT3:refs/heads/foreign:$COMMIT2
+test $4 = refs/heads/master:$COMMIT2:refs/heads/new:$_z40
+EOF
+
+test_expect_success 'push multiple refs' '
+   git push parent1 other:foreign master:new
+'
+
+test_expect_success 'add a branch with an upstream' '
+   git checkout -t -b tracking parent1/foreign

[PATCH] Use longer alias names in subdirectory tests

2012-12-28 Thread Aaron Schrab
When testing aliases in t/t1020-subdirectory.sh use longer names so that
they're less likely to conflict with a git-* command somewhere in the
$PATH.

I have a git-ss command in my path which prevents the 'ss' alias from
being used.  This command will always fail for git.git, causing the test
to fail.  Even if the command succeeded, that would be a false success
for the test since the alias wasn't actually used.  A longer, more
descriptive name will make it much less likely that somebody has a
command in their $PATH which will shadow the alias created for the test.

While here, use a longer name for the 'test' alias as well since that is
also short and meaningful enough to make it not unlikely that somebody
would have a command in their $PATH which will shadow that as well.

Signed-off-by: Aaron Schrab aa...@schrab.com
---
 t/t1020-subdirectory.sh | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index e23ac0e..1e2945e 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -111,19 +111,19 @@ test_expect_success 'read-tree' '
 
 test_expect_success 'alias expansion' '
(
-   git config alias.ss status 
+   git config alias.test-status-alias status 
cd dir 
git status 
-   git ss
+   git test-status-alias
)
 '
 
 test_expect_success NOT_MINGW '!alias expansion' '
pwd expect 
(
-   git config alias.test !pwd 
+   git config alias.test-alias-directory !pwd 
cd dir 
-   git test ../actual
+   git test-alias-directory ../actual
) 
test_cmp expect actual
 '
@@ -131,9 +131,9 @@ test_expect_success NOT_MINGW '!alias expansion' '
 test_expect_success 'GIT_PREFIX for !alias' '
printf dir/ expect 
(
-   git config alias.test !sh -c \printf \$GIT_PREFIX\ 
+   git config alias.test-alias-directory !sh -c \printf 
\$GIT_PREFIX\ 
cd dir 
-   git test ../actual
+   git test-alias-directory ../actual
) 
test_cmp expect actual
 '
-- 
1.8.1.rc3.16.g47d6ba6

--
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: git config -l should not expose sensitive information

2012-12-20 Thread Aaron Schrab

At 10:04 -0500 20 Dec 2012, Jeff King p...@peff.net wrote:
The problem seems to be that people are giving bad advice to tell 
people to post git config -l output without looking at. Maybe we 
could help them with a git config --share-config option that dumps 
all config, but sanitizes the output. It would need to have a list of 
sensitive keys (which does not exist yet), and would need to not just 
mark up things like smtppass, but would also need to pull credential 
information out of remote.*.url strings. And maybe more (I haven't 
thought too long on it).


If such an option is added, it is likely to cause more people to think 
that there is no need to examine the output before sharing it.  But, I 
don't think that the sanitizing could ever be sufficient to guarantee 
that.


Tools outside of the core git tree may add support for new config keys 
which are meant to contain sensitive information, and there would be no 
way for `git config` to know about those.


Even for known sensitive keys, the person entering it might have made a 
typo in the name (e.g.  smptpass) preventing it from being recognized as 
sensitive by the software, but easily recognizable as such by a human.


There's also the problem of varying opinions on what is considered as 
sensitive.  You mention credential information in URLs, but some people 
may consider the entire URL as something which they would not want to 
expose.


I think that attempting to do this would only result in a false sense of 
security.

--
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


Strange behaviour of git diff branch1 ... branch2

2012-10-26 Thread Aaron Schrab

I came across this odd question on stackoverflow:
http://stackoverflow.com/q/13092854/1507392


If git diff is run with ... as a separate argument between two 
commit-ish arguments causes it to produce strange output.  The 
differences seem to be the same as if ... was left out, but change 
lines begin with 4 + or - characters rather than just 1.


Can anybody explain what is happening here?  I don't have any reason to 
want to use that form myself, but I'm very curious about why it produces 
this odd output.


Thanks.
--
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