Re: [msysGit] [PATCH 07/14] Fix BASIC_LDFLAGS and COMPAT_CFLAGS for 64bit MinGW-w64

2014-10-14 Thread Marat Radchenko
On Thu, Oct 09, 2014 at 09:22:19PM +0200, Johannes Schindelin wrote:
> Hi,
> 
> On Wed, 8 Oct 2014, Marat Radchenko wrote:
> 
> > +CC_MACH := $(shell sh -c '$(CC) -dumpmachine 2>/dev/null || echo not')
> 
> There is a rather huge problem with that. The latest mingw-w64 release,
> 4.9.1, does not do what you expect here: while '.../mingw32/bin/gcc -m32
> -o 32.exe test.c' and '.../mingw32/bin/gcc -m64 -o 64.exe test.c' work
> fine, producing i686 and x86_64 executables respectively,
> '.../mingw32/bin/gcc -dumpmachine' prints i686-w64-mingw32 *always*, even
> when specifying the -m64 option.
> 
> So unfortunately, the test introduced by this patch (intended to figure
> out whether the build targets i686, and skip a compiler and a linker
> option otherwise) is incorrect.

According to [1], it is by design. For now, I suggest using separate
gcc binaries for 32/64, without messing with -m32. Of course we can
fallback to `./configure` that will determine bitness by compiling something.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52096#c1
--
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/3] "-x" tracing option for tests

2014-10-14 Thread Michael Haggerty
On 10/13/2014 11:07 PM, Jeff King wrote:
> On Mon, Oct 13, 2014 at 11:24:42AM -0700, Junio C Hamano wrote:
>> [...]
>> Is your plan to reroll the prune-mtime stuff on top of these?  The
>> additional safety those patches would give us is valuable and they
>> are pretty straight-forward---I was hoping to have them in the 2.2
>> release.
> 
> Yes, I've delayed while thinking about the issues that Michael raised.
> There are basically two paths I see:
> 
>   1. These do not solve all problems/races, but are a solid base and
>  sensible path forward for further changes which we can worry about
>  later.
> 
>   2. There is a better way to provide prune safety, and these patches
>  will get in the way of doing that.
> 
> I wanted to make sure we are on path (1) and not path (2). :)

FWIW I think we are on path (1).

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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] Allow building Git with Asciidoctor

2014-10-14 Thread Jakub Narębski

brian m. carlson wrote:

On Mon, Oct 13, 2014 at 01:41:31PM -0700, Junio C Hamano wrote:

"brian m. carlson"  writes:


The second two patches implement some basic support for building with
Asciidoctor.  The first of these moves some items into variables due to
some differences between the AsciiDoc and Asciidoctor command lines.
The user can then override these values when invoking make.

The final patch adds support for the linkgit macro.  Asciidoctor uses
Ruby extensions to implement macro support, unlike AsciiDoc, which uses
a configuration file.


What I do not understand is that 3/4 lets you drop inclusion of
asciidoc.conf which contains a lot more than just linkgit:
definition.


Asciidoctor just doesn't understand the -f argument, so trying to pass
it is going to fail.  For Asciidoctor, you're going to want to do
something like "-I. -rasciidoctor/extensions -rextensions" there
instead.

As for the rest of the asciidoc.conf file, the DocBook manpage header
declarations are implemented automatically by Asciidoctor after my
recent patches.  The paragraph hacks do not appear to be necessary with
Asciidoctor, so they've been omitted.

That leaves the attributes.  All but litdd are built-in to Asciidoctor,
and I can reroll with a modification to extensions.rb that implements
that one.


Would it be possible to automatically convert asciidoc.conf file to 
Asciidoctor extension?


--
Jakub Narębski


--
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] [kernel] completion: silence "fatal: Not a git repository" error

2014-10-14 Thread John Szakmeister
It is possible that a user is trying to run a git command and fail to realize
that they are not in a git repository or working tree.  When trying to complete
an operation, __git_refs would fall to a degenerate case and attempt to use
"git for-each-ref", which would emit the error.

Let's fix this by shunting the error message coming from "git for-each-ref".

Signed-off-by: John Szakmeister 
---
 contrib/completion/git-completion.bash | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 5ea5b82..31b4739 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -388,7 +388,8 @@ __git_refs ()
;;
*)
echo "HEAD"
-   git for-each-ref --format="%(refname:short)" -- 
"refs/remotes/$dir/" | sed -e "s#^$dir/##"
+   git for-each-ref --format="%(refname:short)" -- \
+   "refs/remotes/$dir/" 2>/dev/null | sed -e "s#^$dir/##"
;;
esac
 }
-- 
2.0.1

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


Re: [PATCH 0/4] Allow building Git with Asciidoctor

2014-10-14 Thread brian m. carlson
On Tue, Oct 14, 2014 at 12:07:51PM +0200, Jakub Narębski wrote:
> brian m. carlson wrote:
> >On Mon, Oct 13, 2014 at 01:41:31PM -0700, Junio C Hamano wrote:
> >>
> >>What I do not understand is that 3/4 lets you drop inclusion of
> >>asciidoc.conf which contains a lot more than just linkgit:
> >>definition.
> >
> >Asciidoctor just doesn't understand the -f argument, so trying to pass
> >it is going to fail.  For Asciidoctor, you're going to want to do
> >something like "-I. -rasciidoctor/extensions -rextensions" there
> >instead.
> >
> >As for the rest of the asciidoc.conf file, the DocBook manpage header
> >declarations are implemented automatically by Asciidoctor after my
> >recent patches.  The paragraph hacks do not appear to be necessary with
> >Asciidoctor, so they've been omitted.
> >
> >That leaves the attributes.  All but litdd are built-in to Asciidoctor,
> >and I can reroll with a modification to extensions.rb that implements
> >that one.
> 
> Would it be possible to automatically convert asciidoc.conf file to
> Asciidoctor extension?

It is in theory possible, but it's going to result in a lot of messy
code.  I'm also not sure that Junio wants more than the minimal amount
of Ruby possible, since the goal has been to move away from scripting
languages and to C.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH] Initialise hash variable to prevent compiler warnings

2014-10-14 Thread Felipe Franciosi
On Tue, Oct 14, 2014 at 2:13 AM, Junio C Hamano  wrote:
> On Mon, Oct 13, 2014 at 2:53 PM, Felipe Franciosi  wrote:
>>
>> On Mon, Oct 13, 2014 at 9:12 PM, Junio C Hamano  wrote:
>>>
>>> FNV/I/IDIV10/0 covers all the possibilities of (method & 3), I would
>>> have to say that the compiler needs to be fixed.
>>>
>>> Or insert "default:" just before "case HASH_METHOD_0:" line?
>>>
>>> I dunno.
>>
>> Hmm... The "default:" would work, but is it really that bad to initialise a
>> local variable in this case?
>>
>> In any case, the compilation warning is annoying. Do you prefer the default
>> or the initialisation?
>
> If I really had to choose between the two, adding a useless initialization
> would be the less harmful choice. Adding a meaningless "default:" robs
> another chance from the compilers to diagnose a future breakage we
> might add (namely, we may extend methods and forget to write a
> corresponding case arm for the new method value, which a smart
> compiler can and do diagnose as a switch that does not handle
> all the possible values.
>
> Thanks.

I see your point; the code is correct today because it covers all
cases. Nevertheless, some versions of gcc (the one I used was 4.1.2
from CentOS 5.10 -- haven't tested others) might generate an annoying
warning.

Noting that, I also like my code to compile as cleanly as possible in
all environments that it might be used. Being a bit defensive in that
sense and initialising local variables is what I would do. On top of
that (and putting the compiler flaw aside for a moment), having it
sensibly initialised is another way of protecting the code against
errors introduced in the future.

What do you think?

Cheers,
F.
--
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] Multiple worktrees vs. submodules fixes

2014-10-14 Thread Duy Nguyen
On Sun, Oct 12, 2014 at 12:13 PM, Max Kirillov  wrote:
> These are fixes of issues with submodules with use of multiple working
> trees.

I think the patches look fine from the nd/multiple-work-trees writer's
perspective. I know too little about submodules to judge if this is
the right way and not that way..
-- 
Duy
--
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/3] "-x" tracing option for tests

2014-10-14 Thread Jeff King
On Tue, Oct 14, 2014 at 10:52:30AM +0200, Michael Haggerty wrote:

> >> Is your plan to reroll the prune-mtime stuff on top of these?  The
> >> additional safety those patches would give us is valuable and they
> >> are pretty straight-forward---I was hoping to have them in the 2.2
> >> release.
> > 
> > Yes, I've delayed while thinking about the issues that Michael raised.
> > There are basically two paths I see:
> > 
> >   1. These do not solve all problems/races, but are a solid base and
> >  sensible path forward for further changes which we can worry about
> >  later.
> > 
> >   2. There is a better way to provide prune safety, and these patches
> >  will get in the way of doing that.
> > 
> > I wanted to make sure we are on path (1) and not path (2). :)
> 
> FWIW I think we are on path (1).

Good. :)

I was preparing this to re-send, but I realized there is one snag. I
mentioned that we should probably be ignoring already-broken links from
recent objects to missing objects. For the traversal in pack-objects, we
can use revs->ignore_missing_links for this. But for the one in
git-prune itself, we use mark_reachable, which does not respect that
option.

I think mark_reachable's traversal is essentially the same as the one in
list-objects.c, and the two can be merged. I'll look into that, but I
ran out of time for it tonight (er, this morning. Oops).

-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: [RFC/PATCH] fsck: do not canonicalize modes in trees we are checking

2014-10-14 Thread Jeff King
On Mon, Oct 13, 2014 at 09:37:27AM +1100, Ben Aveling wrote:

> A question about fsck - is there a reason it doesn't have an option to
> delete bad objects?

If the objects are reachable, then deleting them would create other big
problems (i.e., we would be breaking the object graph!).

If they are not, then it is probably safest for them to go away via the
normal means (repack/prune via "git gc").  Deleting via fsck would mean
replicating the reachability and deletion code found elsewhere.

-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 0/4] Allow building Git with Asciidoctor

2014-10-14 Thread Jeff King
On Sat, Oct 11, 2014 at 11:37:32PM +, brian m. carlson wrote:

> This series is designed to implement the changes necessary to build Git
> using Asciidoctor instead of AsciiDoc.

Thanks. I had always taken the attitude that we wrote for the original
Python AsciiDoc, and that using AsciiDoctor was a choice that
git-scm.com made, and something they would have to deal with as far as
compatibility (AFAIK, AsciiDoctor grew out of git-scm.com's home-grown
asciidoc parser).

What's the status on AsciiDoc versus AsciiDoctor? The latter seems more
actively developed these days, but perhaps that is just my perception.
The incompatibilities seem fairly minimal (if those first two patches
are the extent of it, I have no problem at all trying to remain
compatible with both). Would it ever make sense to switch to AsciiDoctor
as our official command-line build program? I know it is supposed to be
much faster (though a lot of the slowness in our build chain is due to
docbook, not asciidoc itself).

Specifically I'm not excited about getting into a state where we have to
maintain both an asciidoc.conf file _and_ ruby extensions for
asciidoctor. I don't mind if somebody wants to step up and keep the
asciidoctor bits in sync with the asciidoc.conf, but I feel like one of
them needs to be considered the "master".

-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] [kernel] completion: silence "fatal: Not a git repository" error

2014-10-14 Thread John Szakmeister
On Tue, Oct 14, 2014 at 6:49 AM, John Szakmeister  wrote:
> It is possible that a user is trying to run a git command and fail to realize
> that they are not in a git repository or working tree.  When trying to 
> complete
> an operation, __git_refs would fall to a degenerate case and attempt to use
> "git for-each-ref", which would emit the error.
>
> Let's fix this by shunting the error message coming from "git for-each-ref".
>
> Signed-off-by: John Szakmeister 
> ---

Sorry for the "[kernel]" in the subject.  I must have forgotten to
remove that off of my format-patch invocation.  If you need me to
resubmit without it, I can do that.

Thanks!

-John
--
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] receive-pack: plug minor memory leak in unpack()

2014-10-14 Thread Jeff King
On Mon, Oct 13, 2014 at 12:08:09PM -0700, Junio C Hamano wrote:

> > I wonder if run-command should provide a managed env array similar
> > to the "args" array.
> 
> I took a look at a few of them:

I took a brief look, too.

I had hoped we could just all it "env" and everybody would be happy
using it instead of bare pointers. But quite a few callers assign
"local_repo_env" to to child_process.env. We could copy it into an
argv_array, of course, but that really feels like working around the
interface. So I think we would prefer to keep both forms available.

That raises the question: what should it be called? The "argv_array"
form of "argv" is called "args". The more I see it, the more I hate that
name, as the two are easily confused. We could have:

  const char **argv;
  struct argv_array argv_array;
  const char **env;
  struct argv_array env_array;

though "argv_array" is a lot to type when you have a string of
argv_array_pushf() calls (which are already by themselves kind of
verbose). Maybe that's not too big a deal, though.

We could flip it to give the managed version the short name (and calling
the unmanaged version "env_ptr" or something). That would require
munging the existing callers, but the tweak would be simple.

>  - daemon.c::handle() uses a static set of environment variables
>that are not built with argv_array().  Same for argv.

Most of the callers you mentioned are good candidates. This one is
tricky.

The argv array gets malloc'd and set up by the parent git-daemon
process. Then each time we get a client, we create a new struct
child_process that references it. So using the managed argv-array would
actually be a bit more complicated (and not save any memory; we just
always point to the single copy for each child).

For the environment, we build it in a function-local buffer, point the
child_process's env field at it, start the child, and then copy the
child_process into our global list of children. When we notice a child
is dead (by linearly going through the list with waitpid), we free the
list entry. So there are a few potentially bad things here:

  1. We memcpy the child_process to put it on the list. Which does work,
 though it feels a little like we are violating the abstraction
 barrier.

  2. The child_process in the list points to the local "env" buffer that
 is no longer valid. There's no bug because we don't ever look at
 it. Moving to a managed env would fix that. But I have to wonder if
 we even want to be keeping the "struct child_process" around in the
 first place (all we really care about is the pid).

  3. If we do move to a managed env, then we expect it to get cleaned up
 in finish_command. But we never call that; we just free the list
 memory containing the child_process. We would want to call
 finish_command. Except that we will have reaped the process already
 with our call to waitpid() from check_dead_children. So we'd need a
 new call to do just the cleanup without the wait, I guess.

  4. For every loop on the listen socket, we call waitpid() for each
 living child, which is a bit wasteful. We'd probably do better to
 call waitpid(0, &status, WNOHANG), and then look up the resulting
 pid in a hashmap or sorted list when we actually see something that
 died. I don't know that this is a huge problem in practice. We use
 git-daemon pretty heavily on our backend servers at GitHub, and it
 seems to use about 5% of a CPU constantly on each machine. Which is
 kind of lame, given that it isn't doing anything at all, but is
 hardly earth-shattering.

So I'm not sure if it is worth converting to a managed env. There's a
bit of ugliness, but none of it is causing any problems, and doing so
opens a can of worms. The most interesting thing to fix (to me, anyway)
is number 4, but that has nothing to do with the env in the first place.
:)

-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 2/2] Remove spurious 'no threads support' warnings

2014-10-14 Thread Etienne Buira
On Mon, Oct 13, 2014 at 9:54 PM, Junio C Hamano  wrote:
> Etienne Buira  writes:
>
>> Threads count being defaulted to 0 (autodetect), and --disable-pthreads
>> build checking that thread count==1, there were spurious warnings about
>> threads being ignored, despite not specified on command line/conf.
>>
>> Fixes tests 5521 and 5526 that were broken in --disable-pthreads builds
>> because of those warnings.
>>
>> Signed-off-by: Etienne Buira 
>> ---
>
> I am not sure if this is the right fix.
>
> Shouldn't a --threads=0 from the command line (when there is a
> pack.threads configuration hardcoding some number to override it)
> give a chance to the auto detection codepath to ask online_cpus()
> and receive 1 on NO_PTHREADS build to avoid triggering the same
> warning you are squelching with this patch?
>
> That is, something like this instead, perhaps?

Indeed, your patch is better.

> -- >8 --
> Subject: [PATCH] pack-objects: set number of threads before checking and 
> warning
>
> Under NO_PTHREADS build, we warn when delta_search_threads is not
> set to 1, because that is the only sensible value on a single
> threaded build.
>
> However, the auto detection that kicks in when that variable is set
> to 0 (e.g. there is no configuration variable or command line option,
> or an explicit --threads=0 is given from the command line to override
> the pack.threads configuration to force auto-detection) was not done
> before the condition to issue this warning was tested.
>
> Move the auto-detection code and place it at an appropriate spot.
>
> Signed-off-by: Junio C Hamano 
> ---
>  builtin/pack-objects.c | 6 --
>  thread-utils.h | 4 
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index d391934..a715237 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1972,8 +1972,6 @@ static void ll_find_deltas(struct object_entry **list, 
> unsigned list_size,
>
> init_threaded_search();
>
> -   if (!delta_search_threads)  /* --threads=0 means autodetect */
> -   delta_search_threads = online_cpus();
> if (delta_search_threads <= 1) {
> find_deltas(list, &list_size, window, depth, processed);
> cleanup_threaded_search();
> @@ -2685,6 +2683,10 @@ int cmd_pack_objects(int argc, const char **argv, 
> const char *prefix)
> pack_compression_level = Z_DEFAULT_COMPRESSION;
> else if (pack_compression_level < 0 || pack_compression_level > 
> Z_BEST_COMPRESSION)
> die("bad pack compression level %d", pack_compression_level);
> +
> +   if (!delta_search_threads)  /* --threads=0 means autodetect */
> +   delta_search_threads = online_cpus();
> +
>  #ifdef NO_PTHREADS
> if (delta_search_threads != 1)
> warning("no threads support, ignoring --threads");
> diff --git a/thread-utils.h b/thread-utils.h
> index 6fb98c3..d9a769d 100644
> --- a/thread-utils.h
> +++ b/thread-utils.h
> @@ -7,5 +7,9 @@
>  extern int online_cpus(void);
>  extern int init_recursive_mutex(pthread_mutex_t*);
>
> +#else
> +
> +#define online_cpus() 1
> +
>  #endif
>  #endif /* THREAD_COMPAT_H */
> --
> 2.1.2-468-g1a77c5b
>
--
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] Handle atexit list internaly for unthreaded builds

2014-10-14 Thread Etienne Buira
On Mon, Oct 13, 2014 at 10:00 PM, Junio C Hamano  wrote:
> Etienne Buira  writes:
>
>> Wrap atexit()s calls on unthreaded builds to handle callback list
>> internally.
>>
>> This is needed because on unthreaded builds, asyncs inherits parent's
>> atexit() list, that gets run as soon as the async exit()s (and again at
>> the end of the parent process). That led to remove temporary and lock
>> files too early.
>
> ... that does not explain what you did to builtin/clone.c at all.
> Care to enlighten us, please?

Checking current pid against the one that registered the atexit()
routine (what builtin/clone.c currently does) is another way to guard
against the same issue, but it needs to store a pid for each atexit()
call whenever code called after might use async.
>From what I've seen, two out of all atexit() calls were guarded like that:
- builtin/clone.c:cmd_clone
- lockfile.c:lock_file (overlooked it at first, would you mind to
s/temporary and lock files/temporary files/ the commit message if no
other round is needed? I'll do it otherwise).

So the changes in builtin/clone.c are just there because the patch
makes this check redundant (still needed in lockfile.c, as it loops
over a list that might refer to parent process's lockfiles).

>> Fixes test 5537 (temporary shallow file vanished before unpack-objects
>> could open it)
>>
>> V4: fix bogus preprocessor directives
>
> Please do not write this "V4:" line in the log message.  People who
> read "git log" output and find this description would not know
> anything about the previous faulty ones.  Putting it _after_ the
> three-dash line below will help the reviewers a lot.
>
>>
>> Thanks-to: Duy Nguyen 
>> Thanks-to: Andreas Schwab 
>
> Usually we spell these "Helped-by: " instead.
>
>> Signed-off-by: Etienne Buira 
>> ---
>
> Thanks.
>
>>  builtin/clone.c   |  5 -
>>  git-compat-util.h |  5 +
>>  run-command.c | 40 
>>  shallow.c |  7 ++-
>>  4 files changed, 47 insertions(+), 10 deletions(-)
>>
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index bbd169c..e122f33 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -390,7 +390,6 @@ static void clone_local(const char *src_repo, const char 
>> *dest_repo)
>>
>>  static const char *junk_work_tree;
>>  static const char *junk_git_dir;
>> -static pid_t junk_pid;
>>  static enum {
>>   JUNK_LEAVE_NONE,
>>   JUNK_LEAVE_REPO,
>> @@ -417,8 +416,6 @@ static void remove_junk(void)
>>   break;
>>   }
>>
>> - if (getpid() != junk_pid)
>> - return;
>>   if (junk_git_dir) {
>>   strbuf_addstr(&sb, junk_git_dir);
>>   remove_dir_recursively(&sb, 0);
>> @@ -758,8 +755,6 @@ int cmd_clone(int argc, const char **argv, const char 
>> *prefix)
>>   struct refspec *refspec;
>>   const char *fetch_pattern;
>>
>> - junk_pid = getpid();
>> -
>>   packet_trace_identity("clone");
>>   argc = parse_options(argc, argv, prefix, builtin_clone_options,
>>builtin_clone_usage, 0);
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index f587749..6dd63dd 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -577,6 +577,11 @@ int inet_pton(int af, const char *src, void *dst);
>>  const char *inet_ntop(int af, const void *src, char *dst, size_t size);
>>  #endif
>>
>> +#ifdef NO_PTHREADS
>> +#define atexit git_atexit
>> +extern int git_atexit(void (*handler)(void));
>> +#endif
>> +
>>  extern void release_pack_memory(size_t);
>>
>>  typedef void (*try_to_free_t)(size_t);
>> diff --git a/run-command.c b/run-command.c
>> index 35a3ebf..0f9a9b0 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -624,6 +624,45 @@ static int async_die_is_recursing(void)
>>   return ret != NULL;
>>  }
>>
>> +#else
>> +
>> +static struct {
>> + void (**handlers)(void);
>> + size_t nr;
>> + size_t alloc;
>> +} git_atexit_hdlrs;
>> +
>> +static int git_atexit_installed;
>> +
>> +static void git_atexit_dispatch()
>> +{
>> + size_t i;
>> +
>> + for (i=git_atexit_hdlrs.nr ; i ; i--)
>> + git_atexit_hdlrs.handlers[i-1]();
>> +}
>> +
>> +static void git_atexit_clear()
>> +{
>> + free(git_atexit_hdlrs.handlers);
>> + memset(&git_atexit_hdlrs, 0, sizeof(git_atexit_hdlrs));
>> + git_atexit_installed = 0;
>> +}
>> +
>> +#undef atexit
>> +int git_atexit(void (*handler)(void))
>> +{
>> + ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, 
>> git_atexit_hdlrs.alloc);
>> + git_atexit_hdlrs.handlers[git_atexit_hdlrs.nr++] = handler;
>> + if (!git_atexit_installed) {
>> + if (atexit(&git_atexit_dispatch))
>> + return -1;
>> + git_atexit_installed = 1;
>> + }
>> + return 0;
>> +}
>> +#define atexit git_atexit
>> +
>>  #endif
>>
>>  int start_async(struct async *async)
>> @@ -682,6 +721,7 @@ int start_async(struct async *async)
>>

Re: [PATCH] Initialise hash variable to prevent compiler warnings

2014-10-14 Thread Junio C Hamano
On Tue, Oct 14, 2014 at 4:44 AM, Felipe Franciosi  wrote:
> On Tue, Oct 14, 2014 at 2:13 AM, Junio C Hamano  wrote:
>>
>> If I really had to choose between the two, adding a useless initialization
>> would be the less harmful choice. Adding a meaningless "default:" robs
>> ...
> Being a bit defensive in that
> sense and initialising local variables is what I would do. On top of
> that (and putting the compiler flaw aside for a moment), having it
> sensibly initialised is another way of protecting the code against
> errors introduced in the future.

That is a false sense of safety. You will not know if the new method
introduced in the future would behave sensibly if the variable is left
in a state the blanket initialization created, so setting it to 0 upfront
is not really being defensive; you would rob compilers a chance
to notice something is amiss in the future code with the initialization,
just like a "default:" would. We need to accept that both are not about
being defensive but are ways to work around stupid compilers from
reporting false positives.

I am not saying that we should not do a work around. I am only
saying that it is wrong to try selling such a work around as a defensive
good practice, which is not.

> What do you think?

Again, if I really had to choose between the two, adding a useless
initialization would be the less harmful choice, as the other one has
an extra downside.
--
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] Allow building Git with Asciidoctor

2014-10-14 Thread Junio C Hamano
Jeff King  writes:

> On Sat, Oct 11, 2014 at 11:37:32PM +, brian m. carlson wrote:
>
> Specifically I'm not excited about getting into a state where we have to
> maintain both an asciidoc.conf file _and_ ruby extensions for
> asciidoctor. I don't mind if somebody wants to step up and keep the
> asciidoctor bits in sync with the asciidoc.conf, but I feel like one of
> them needs to be considered the "master".

My so-far-unstated inclination, since seeing the patch to fix the
unbalanced example block separators from Brian (which was outside
and before this four-patch series), has been to keep our Makefile in
Documentation/ aware only of AsciiDoc while maintaining *.txt files
in a state so that AsciiDoctor could also be used to process them,
if people want to futz with their copies of Documentation/Makefile.

I do not mind to have the machinery to run AsciiDoctor too much in
my tree.  It may make it easier for those who use it to spot places
in *.txt that need (in)compatibility workarounds between the two
formatters than keeping it outside.

But somebody needs to maintain that machinery and that will not be
me.

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 0/4] Multiple worktrees vs. submodules fixes

2014-10-14 Thread Jens Lehmann

Am 14.10.2014 um 14:17 schrieb Duy Nguyen:

On Sun, Oct 12, 2014 at 12:13 PM, Max Kirillov  wrote:

These are fixes of issues with submodules with use of multiple working
trees.


I think the patches look fine from the nd/multiple-work-trees writer's
perspective. I know too little about submodules to judge if this is
the right way and not that way..


Sorry, I was too busy to review this work until now, but here we go:

If I understand multiple work trees correctly, everything except work
tree local stuff is redirected into GIT_COMMON_DIR. This is a very cool
feature I'd love to see on our CI server to reduce disk footprint and
clone times, especially for submodules!

But I can't see how that can work by just sharing the modules directory
tree, as that contains work tree related files - e.g. the index - for
each submodule. AFAICS sharing them between work trees will work only
if the content of the modules directory is partly present in GIT_DIR -
for work tree related files - and only the common stuff is taken from
GIT_COMMON_DIR (Or did I just miss the magic that already does that?).
And I didn't try to wrap my head around recursive submodules yet ...

Until that problem is solved it looks wrong to pass GIT_COMMON_DIR into
submodule recursion, I believe GIT_COMMON_DIR should be added to the
local_repo_env array (and even if it is passed on later, we might have
to append "/modules/" to make it point to the correct
location).

But maybe I'm missing something?
--
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] Multiple worktrees vs. submodules fixes

2014-10-14 Thread Junio C Hamano
Jens Lehmann  writes:

> But I can't see how that can work by just sharing the modules directory
> tree, as that contains work tree related files - e.g. the index - for
> each submodule. AFAICS sharing them between work trees will work only
> if the content of the modules directory is partly present in GIT_DIR -
> for work tree related files - and only the common stuff is taken from
> GIT_COMMON_DIR (Or did I just miss the magic that already does that?).

The first time I saw the patch 3/4 in this series, my reaction was
"Huh, why should the repository data and branch tips be separated
out into multiple independent copies for the same module?  Do we
force users to synchronise between these copies?  It does not make
any sense at all."

But that was until I read your message ;-) You are right that the
index and HEAD are dependent to a particular working tree that is
checked out.  There may be other things that logically are per-
working tree.

And multiple-worktree _is_ about keeping the same repository and
history data (i.e. object database, refs, rerere database, reflogs
for refs/*) only once, while allowing multiple working trees attached
to that single copy.

So it appears to me that to create a checkout-to copy of a
superproject with a submodule, a checkout-to copy of the
superproject would have a submodule, which is a checkout-to copy of
the submodule in the superproject.

> And I didn't try to wrap my head around recursive submodules yet ...
>
> Until that problem is solved it looks wrong to pass GIT_COMMON_DIR into
> submodule recursion, I believe GIT_COMMON_DIR should be added to the
> local_repo_env array (and even if it is passed on later, we might have
> to append "/modules/" to make it point to the correct
> location).
--
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] pass config slots as pointers instead of offsets

2014-10-14 Thread Junio C Hamano
From: Jonathan Nieder 
Date: Tue, 7 Oct 2014 15:16:57 -0400

Many config-parsing helpers, like parse_branch_color_slot,
take the name of a config variable and an offset to the
"slot" name (e.g., "color.branch.plain" is passed along with
"13" to effectively pass "plain"). This is leftover from the
time that these functions would die() on error, and would
want the full variable name for error reporting.

These days they do not use the full variable name at all.
Passing a single pointer to the slot name is more natural,
and lets us more easily adjust the callers to use skip_prefix
to avoid manually writing offset numbers.

This is effectively a continuation of 9e1a5eb, which did the
same for parse_diff_color_slot. This patch covers all of the
remaining similar constructs.

Signed-off-by: Jeff King 
Signed-off-by: Junio C Hamano 
---

 * Can we have your Sign-off (and a quick eyeballing again), please?

 builtin/branch.c | 16 
 builtin/commit.c | 19 +--
 builtin/log.c|  2 +-
 log-tree.c   |  4 ++--
 log-tree.h   |  2 +-
 5 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0591b22..b2e1895c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -62,19 +62,19 @@ static unsigned char merge_filter_ref[20];
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
-static int parse_branch_color_slot(const char *var, int ofs)
+static int parse_branch_color_slot(const char *slot)
 {
-   if (!strcasecmp(var+ofs, "plain"))
+   if (!strcasecmp(slot, "plain"))
return BRANCH_COLOR_PLAIN;
-   if (!strcasecmp(var+ofs, "reset"))
+   if (!strcasecmp(slot, "reset"))
return BRANCH_COLOR_RESET;
-   if (!strcasecmp(var+ofs, "remote"))
+   if (!strcasecmp(slot, "remote"))
return BRANCH_COLOR_REMOTE;
-   if (!strcasecmp(var+ofs, "local"))
+   if (!strcasecmp(slot, "local"))
return BRANCH_COLOR_LOCAL;
-   if (!strcasecmp(var+ofs, "current"))
+   if (!strcasecmp(slot, "current"))
return BRANCH_COLOR_CURRENT;
-   if (!strcasecmp(var+ofs, "upstream"))
+   if (!strcasecmp(slot, "upstream"))
return BRANCH_COLOR_UPSTREAM;
return -1;
 }
@@ -88,7 +88,7 @@ static int git_branch_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (starts_with(var, "color.branch.")) {
-   int slot = parse_branch_color_slot(var, 13);
+   int slot = parse_branch_color_slot(var + 13);
if (slot < 0)
return 0;
if (!value)
diff --git a/builtin/commit.c b/builtin/commit.c
index 5ed6036..5a8a29e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1238,22 +1238,21 @@ static int dry_run_commit(int argc, const char **argv, 
const char *prefix,
return commitable ? 0 : 1;
 }
 
-static int parse_status_slot(const char *var, int offset)
+static int parse_status_slot(const char *slot)
 {
-   if (!strcasecmp(var+offset, "header"))
+   if (!strcasecmp(slot, "header"))
return WT_STATUS_HEADER;
-   if (!strcasecmp(var+offset, "branch"))
+   if (!strcasecmp(slot, "branch"))
return WT_STATUS_ONBRANCH;
-   if (!strcasecmp(var+offset, "updated")
-   || !strcasecmp(var+offset, "added"))
+   if (!strcasecmp(slot, "updated") || !strcasecmp(slot, "added"))
return WT_STATUS_UPDATED;
-   if (!strcasecmp(var+offset, "changed"))
+   if (!strcasecmp(slot, "changed"))
return WT_STATUS_CHANGED;
-   if (!strcasecmp(var+offset, "untracked"))
+   if (!strcasecmp(slot, "untracked"))
return WT_STATUS_UNTRACKED;
-   if (!strcasecmp(var+offset, "nobranch"))
+   if (!strcasecmp(slot, "nobranch"))
return WT_STATUS_NOBRANCH;
-   if (!strcasecmp(var+offset, "unmerged"))
+   if (!strcasecmp(slot, "unmerged"))
return WT_STATUS_UNMERGED;
return -1;
 }
@@ -1291,7 +1290,7 @@ static int git_status_config(const char *k, const char 
*v, void *cb)
return 0;
}
if (starts_with(k, "status.color.") || starts_with(k, "color.status.")) 
{
-   int slot = parse_status_slot(k, 13);
+   int slot = parse_status_slot(k + 13);
if (slot < 0)
return 0;
if (!v)
diff --git a/builtin/log.c b/builtin/log.c
index 4389722..4c5fc4b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -389,7 +389,7 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (starts_with(var, "color.decorate."))
-   return parse_decorate_color_config(var, 15, value);
+   return parse_decorate_color_config(var, var + 15, value);
if (!strcmp(var, "log.ma

Re: [PATCH] pass config slots as pointers instead of offsets

2014-10-14 Thread Jonathan Nieder
Junio C Hamano wrote:

>  builtin/branch.c | 16 
>  builtin/commit.c | 19 +--
>  builtin/log.c|  2 +-
>  log-tree.c   |  4 ++--
>  log-tree.h   |  2 +-
>  5 files changed, 21 insertions(+), 22 deletions(-)

Signed-off-by: Jonathan Nieder 
--
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] [kernel] completion: silence "fatal: Not a git repository" error

2014-10-14 Thread Junio C Hamano
John Szakmeister  writes:

> It is possible that a user is trying to run a git command and fail to realize
> that they are not in a git repository or working tree.  When trying to 
> complete
> an operation, __git_refs would fall to a degenerate case and attempt to use
> "git for-each-ref", which would emit the error.
>
> Let's fix this by shunting the error message coming from "git for-each-ref".

Hmph, do you mean this one?

$ cd /var/tmp ;# not a git repository
$ git checkout 

->

$ git checkout fatal: Not a git repository (or any of the parent 
directories): .git
HEAD 

I agree it is ugly, but would it be an improvement for the end user,
who did not realize that she was not in a directory where "git checkout"
makes sense, not to tell her that she is not in a git repository in
some way?

--
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] Multiple worktrees vs. submodules fixes

2014-10-14 Thread Max Kirillov
On Tue, Oct 14, 2014 at 10:26:42AM -0700, Junio C Hamano wrote:
> And multiple-worktree _is_ about keeping the same repository and
> history data (i.e. object database, refs, rerere database, reflogs for
> refs/*) only once, while allowing multiple working trees attached to
> that single copy.
> 
> So it appears to me that to create a checkout-to copy of a
> superproject with a submodule, a checkout-to copy of the superproject
> would have a submodule, which is a checkout-to copy of the submodule
> in the superproject.

That's right, this linking should be more implicit.

But here are a lot of nuances. For example, it makes sense to have a
superproject checkout without submodules being initialized (so that they
don't waste space and machine time for working tree, which often is more
than repository data). And it may happen so that this checkout is the
master repository for superproject checkouts. But this should not
prevent users from using initialized submodules in other checkouts.

Then, a checkout copy of a submodule can be standalone (for example, git
and git-html-docs are submodules of msysgit). Or, it can even belong to
some other superproject. And in that cases they still should be able to
be linked.

Considering all above, and also the thing that I am quite new to
submodules (but have to use them currently), I did not intend to create
any new UI, only to make backend handle the already existing linked
checkouts, which can be made manually.

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


Re: [PATCH] git-prompt.sh: Hide prompt for ignored pwd

2014-10-14 Thread Johannes Sixt
Am 14.10.2014 um 04:32 schrieb Jess Austin:
> diff --git a/contrib/completion/git-prompt.sh 
> b/contrib/completion/git-prompt.sh
> index c5473dc..d7559ff 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -84,6 +84,11 @@
>  # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
>  # the colored output of "git status -sb" and are available only when
>  # using __git_ps1 for PROMPT_COMMAND or precmd.
> +#
> +# If you would like __git_ps1 to do nothing in the case when the current
> +# directory is set up to be ignored by git, then set
> +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set
> +# bash.hideOnIgnoredPwd to true in the repository configuration.
>  
>  # check whether printf supports -v
>  __git_printf_supports_v=
> @@ -501,6 +506,13 @@ __git_ps1 ()
>   local f="$w$i$s$u"
>   local gitstring="$c$b${f:+$z$f}$r$p"
>  
> + if [ -n "$(git check-ignore .)" ] &&
> +( [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] ||
> +  [ "$(git config --bool bash.hideOnIgnoredPwd)" = "true" ] )

Ahem, no. Please do not punish users who are not interested in the new
feature with two new processes every time __git_ps() is run. Think of
Windows where fork() is really, *really* expensive.

BTW, you can write '{ foo || bar; }' to bracket a || chain without a
sub-process.

> + then
> + printf_format=""
> + fi
> +
>   if [ $pcmode = yes ]; then
>   if [ "${__git_printf_supports_v-}" != yes ]; then
>   gitstring=$(printf -- "$printf_format" "$gitstring")

-- Hannes

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


Re: [PATCH] git-prompt.sh: Hide prompt for ignored pwd

2014-10-14 Thread Richard Hansen
On 2014-10-14 14:47, Johannes Sixt wrote:
> Am 14.10.2014 um 04:32 schrieb Jess Austin:
>> diff --git a/contrib/completion/git-prompt.sh 
>> b/contrib/completion/git-prompt.sh
>> index c5473dc..d7559ff 100644
>> --- a/contrib/completion/git-prompt.sh
>> +++ b/contrib/completion/git-prompt.sh
>> @@ -84,6 +84,11 @@
>>  # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
>>  # the colored output of "git status -sb" and are available only when
>>  # using __git_ps1 for PROMPT_COMMAND or precmd.
>> +#
>> +# If you would like __git_ps1 to do nothing in the case when the current
>> +# directory is set up to be ignored by git, then set
>> +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set
>> +# bash.hideOnIgnoredPwd to true in the repository configuration.
>>  
>>  # check whether printf supports -v
>>  __git_printf_supports_v=
>> @@ -501,6 +506,13 @@ __git_ps1 ()
>>  local f="$w$i$s$u"
>>  local gitstring="$c$b${f:+$z$f}$r$p"
>>  
>> +if [ -n "$(git check-ignore .)" ] &&
>> +   ( [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] ||
>> + [ "$(git config --bool bash.hideOnIgnoredPwd)" = "true" ] )
> 
> Ahem, no. Please do not punish users who are not interested in the new
> feature with two new processes every time __git_ps() is run. Think of
> Windows where fork() is really, *really* expensive.

Is this why bash.showDirtyState and friends aren't checked unless the
corresponding environment variable is set to a non-empty value?

Regardless, it would be nice if the behavior matched the other bash.*
variables (only check the bash.* variable if the corresponding
environment variable is set, and default to true).  The following should
fix it:

if [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] &&
   [ "$(git config --bool bash.hideOnIgnoredPwd)" != "false" ] &&
   [ "$(git check-ignore .)" ]
then
...

-Richard

> 
> BTW, you can write '{ foo || bar; }' to bracket a || chain without a
> sub-process.
> 
>> +then
>> +printf_format=""
>> +fi
>> +
>>  if [ $pcmode = yes ]; then
>>  if [ "${__git_printf_supports_v-}" != yes ]; then
>>  gitstring=$(printf -- "$printf_format" "$gitstring")
> 
> -- Hannes
> 

--
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] [kernel] completion: silence "fatal: Not a git repository" error

2014-10-14 Thread John Szakmeister
On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano  wrote:
> John Szakmeister  writes:
>
>> It is possible that a user is trying to run a git command and fail to realize
>> that they are not in a git repository or working tree.  When trying to 
>> complete
>> an operation, __git_refs would fall to a degenerate case and attempt to use
>> "git for-each-ref", which would emit the error.
>>
>> Let's fix this by shunting the error message coming from "git for-each-ref".
>
> Hmph, do you mean this one?
>
> $ cd /var/tmp ;# not a git repository
> $ git checkout 
>
> ->
>
> $ git checkout fatal: Not a git repository (or any of the parent 
> directories): .git
> HEAD
>
> I agree it is ugly, but would it be an improvement for the end user,
> who did not realize that she was not in a directory where "git checkout"
> makes sense, not to tell her that she is not in a git repository in
> some way?

I had thought about that too, but I think--for me--it comes down to two things:

1) We're not intentionally trying to inform the user anywhere else
that they are not in a git repo.  We simply fail to complete anything,
which I think is an established behavior.
2) It mingles with the stuff already on the command line, making it
confusing to know what you typed.  Then you end up ctrl-c'ing your way
out of it and starting over--which is the frustrating part.

For me, I thought it better to just be more well-behaved.  I've also
run across this issue when I legitimately wanted to do something--I
wish I could remember what it was--with a remote repo and didn't
happen to be in a git working tree.  It was frustrating to see this
error message then too, for the same reason as above.  I use tab
completion quite extensively, so spitting things like this out making
it difficult to move forward is a problem.

Would it be better to check that "$dir" is non-empty and then provide
the extra bits of information?  We could then avoid giving the user
anything in that case.

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


Re: [PATCH] git-prompt.sh: Hide prompt for ignored pwd

2014-10-14 Thread Richard Hansen
On 2014-10-13 22:32, Jess Austin wrote:
> Set __git_ps1 to display nothing when present working directory is
> ignored, triggered by either the new environmental variable
> GIT_PS1_HIDE_ON_IGNORED_PWD or the new repository configuration
> variable bash.hideOnIgnoredPwd (or both). In the absence of these
> settings this change has no effect.
> 
> Many people manage e.g. dotfiles in their home directory with git.
> This causes the prompt generated by __git_ps1 to refer to that "top
> level" repo while working in any descendant directory. That can be
> distracting, so this patch helps one shut off that noise.
> 
> Signed-off-by: Jess Austin 
> ---
> On Thu, Oct 9, 2014 at 5:09 PM, Richard Hansen  wrote:
>> On 2014-10-09 06:27, Jess Austin wrote:
>>> Would you want this configured in each repo (i.e. via a line in 
>>> ".git/config"),
>>> or would you prefer something global so that it only need be set in one
>>> place? I'm not sure how the latter technique would work, so if that seems
>>> better please advise on how to go about that.
>>
>> A 'git config' variable is fine.  The bash.showDirtyState,
>> bash.showUntrackedFiles, and bash.showUpstream config variables seem
>> like good examples to follow.
> 
> I think this is what you meant. I changed the name of the envvar. Now the
> variables are GIT_PS1_HIDE_ON_IGNORED_PWD and bash.hideOnIgnoredPwd. I
> admit these are still kind of unwieldy, but maybe now they're more 
> descriptive?

I do prefer the new names.  They are long, but how often will someone
have to type it?  In this case it's better to be descriptive than to be
short.  (I wonder if adding two letters would improve readability
further:  GIT_PS1_HIDE_WHEN_PWD_IGNORED and bash.hideWhenPwdIgnored.)

To avoid scaring people who might not want this feature enabled, I
recommend changing the subject line to something like this:

git-prompt.sh: Option to hide prompt for ignored pwd

> 
> Please advise!
> 
> cheers,
> Jess
> 
>  contrib/completion/git-prompt.sh | 12 
>  t/t9903-bash-prompt.sh   | 42 
> 
>  2 files changed, 54 insertions(+)
> 
> diff --git a/contrib/completion/git-prompt.sh 
> b/contrib/completion/git-prompt.sh
> index c5473dc..d7559ff 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -84,6 +84,11 @@
>  # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
>  # the colored output of "git status -sb" and are available only when
>  # using __git_ps1 for PROMPT_COMMAND or precmd.
> +#
> +# If you would like __git_ps1 to do nothing in the case when the current
> +# directory is set up to be ignored by git, then set
> +# GIT_PS1_HIDE_ON_IGNORED_PWD to a nonempty value, or set
> +# bash.hideOnIgnoredPwd to true in the repository configuration.

As mentioned in my previous email, I would prefer the code to follow the
behavior of the other config variables (the environment variable has to
be set *and* the config variable has to be non-false).

>  
>  # check whether printf supports -v
>  __git_printf_supports_v=
> @@ -501,6 +506,13 @@ __git_ps1 ()
>   local f="$w$i$s$u"
>   local gitstring="$c$b${f:+$z$f}$r$p"
>  
> + if [ -n "$(git check-ignore .)" ] &&

Rather than:

[ -n "$(git check-ignore .)" ]

I would prefer:

git check-ignore -q .

For example:

if [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] &&
   [ "$(git config --bool bash.hideOnIgnoredPwd)" != "false" ] &&
   git check-ignore -q .
then
...

-Richard


> +( [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] ||
> +  [ "$(git config --bool bash.hideOnIgnoredPwd)" = "true" ] )
> + then
> + printf_format=""
> + fi
> +
>   if [ $pcmode = yes ]; then
>   if [ "${__git_printf_supports_v-}" != yes ]; then
>   gitstring=$(printf -- "$printf_format" "$gitstring")
> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> index 9150984..a8ef8a3 100755
> --- a/t/t9903-bash-prompt.sh
> +++ b/t/t9903-bash-prompt.sh
> @@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' '
>   git commit -m "another b2" file &&
>   echo 000 >file &&
>   git commit -m "yet another b2" file &&
> + mkdir ignored_dir &&
> + echo "ignored_dir/" >> .gitignore &&
>   git checkout master
>  '
>  
> @@ -588,4 +590,44 @@ test_expect_success 'prompt - zsh color pc mode' '
>   test_cmp expected "$actual"
>  '
>  
> +test_expect_success 'prompt - hide on ignored pwd - shell variable unset 
> with config disabled' '
> + printf " (master)" >expected &&
> + (
> + cd ignored_dir &&
> + __git_ps1 >"$actual"
> + ) &&
> + test_cmp expected "$actual"
> +'
> +
> +test_expect_success 'prompt - hide on ignored pwd - shell variable unset 
> with config enabled' '
> + printf "" >expected &&
> + test_config bash.hideOnIgnoredPwd true &&
> + (
> + cd ignored_dir &&
> + 

Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes

2014-10-14 Thread Jens Lehmann

Am 14.10.2014 um 20:34 schrieb Max Kirillov:

On Tue, Oct 14, 2014 at 10:26:42AM -0700, Junio C Hamano wrote:

And multiple-worktree _is_ about keeping the same repository and
history data (i.e. object database, refs, rerere database, reflogs for
refs/*) only once, while allowing multiple working trees attached to
that single copy.

So it appears to me that to create a checkout-to copy of a
superproject with a submodule, a checkout-to copy of the superproject
would have a submodule, which is a checkout-to copy of the submodule
in the superproject.


That's right, this linking should be more implicit.


Yep. And for the submodule of a submodule too ... ;-)


But here are a lot of nuances. For example, it makes sense to have a
superproject checkout without submodules being initialized (so that they
don't waste space and machine time for working tree, which often is more
than repository data).


Hmm, I'm not sure if this is a problem. If the GIT_COMMON_DIR does have
the submodule repo but it isn't initialized locally, we shouldn't have a
problem (except for wasting some disk space if not a single checkout-to
superproject initializes this submodule). And if GIT_COMMON_DIR does not
have the submodule repo yet, wouldn't it be cloned the moment we init
the submodule in the checkout-to? Or would that need extra functionality?

> And it may happen so that this checkout is the

master repository for superproject checkouts. But this should not
prevent users from using initialized submodules in other checkouts.


I could live with the restriction that submodule's GIT_COMMON_DIRs always
live in their checkout-to superproject's GIT_COMMON_DIR. This would still
be an improvement for CI servers that have multiple clones of a super-
project, as they would all share their submodule common dirs at least
per superproject.


Then, a checkout copy of a submodule can be standalone (for example, git
and git-html-docs are submodules of msysgit). Or, it can even belong to
some other superproject. And in that cases they still should be able to
be linked.


Maybe such configurations would have to be handled manually to achieve
maximum savings. At least I could live with that.


Considering all above, and also the thing that I am quite new to
submodules (but have to use them currently), I did not intend to create
any new UI, only to make backend handle the already existing linked
checkouts, which can be made manually.


Maybe the way to go is to restrict GIT_COMMON_DIR to superprojects and
have a distinct GIT_COMMON_MODULES_DIR? We could even set that to the
same location for all submodules of different superprojects to achieve
minimum disk footprint (assuming they all have different names across
all superprojects and recursion levels).

Hmm, so I tend towards adding GIT_COMMON_DIR to local_repo_env until
we figured out how to handle this. Without that I fear bad things will
happen, at least for a superproject with multiple checkout-to work trees
where the same submodule is initialized more than once ...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] clone: --dissociate option to mark that reference is only temporary

2014-10-14 Thread Junio C Hamano
While use of the --reference option to borrow objects from an
existing local repository of the same project is an effective way to
reduce traffic when cloning a project over the network, it makes the
resulting "borrowing" repository dependent on the "borrowed"
repository.  After running

git clone --reference=P $URL Q

the resulting repository Q will be broken if the borrowed repository
P disappears.

The way to allow the borrowed repository to be removed is to repack
the borrowing repository (i.e. run "git repack -a -d" in Q); while
power users may know it very well, it is not easily discoverable.

Teach a new "--dissociate" option to "git clone" to run this
repacking for the user.

Signed-off-by: Junio C Hamano 
---

 * This comes from
   http://thread.gmane.org/gmane.comp.version-control.git/243918/focus=245397
   which is one of the low-hanging entries in the leftover-bits list
   http://git-blame.blogspot.com/p/leftover-bits.html

   Yes, I must have been really bored to do this ;-)

 Documentation/git-clone.txt | 11 +--
 builtin/clone.c | 25 +
 t/t5700-clone-reference.sh  | 17 +
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 0363d00..f1f2a3f 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git clone' [--template=]
  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
  [-o ] [-b ] [-u ] [--reference ]
- [--separate-git-dir ]
+ [--dissociate] [--separate-git-dir ]
  [--depth ] [--[no-]single-branch]
  [--recursive | --recurse-submodules] [--] 
  []
@@ -98,7 +98,14 @@ objects from the source repository into a pack in the cloned 
repository.
require fewer objects to be copied from the repository
being cloned, reducing network and local storage costs.
 +
-*NOTE*: see the NOTE for the `--shared` option.
+*NOTE*: see the NOTE for the `--shared` option, and also the
+`--dissociate` option.
+
+--dissociate::
+   Borrow the objects from reference repositories specified
+   with the `--reference` options only to reduce network
+   transfer and stop borrowing from them after a clone is made
+   by making necessary local copies of borrowed objects.
 
 --quiet::
 -q::
diff --git a/builtin/clone.c b/builtin/clone.c
index bbd169c..780fbd5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -48,6 +48,7 @@ static int option_verbosity;
 static int option_progress = -1;
 static struct string_list option_config;
 static struct string_list option_reference;
+static int option_dissociate;
 
 static int opt_parse_reference(const struct option *opt, const char *arg, int 
unset)
 {
@@ -93,6 +94,8 @@ static struct option builtin_clone_options[] = {
N_("create a shallow clone of that depth")),
OPT_BOOL(0, "single-branch", &option_single_branch,
N_("clone only one branch, HEAD or --branch")),
+   OPT_BOOL(0, "dissociate", &option_dissociate,
+N_("use --reference only while cloning")),
OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
   N_("separate git dir from working tree")),
OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
@@ -736,6 +739,21 @@ static void write_refspec_config(const char* 
src_ref_prefix,
strbuf_release(&value);
 }
 
+static void dissociate_from_references(void)
+{
+   struct child_process cmd;
+
+   memset(&cmd, 0, sizeof(cmd));
+   argv_array_pushl(&cmd.args, "repack", "-a", "-d", NULL);
+   cmd.git_cmd = 1;
+   cmd.out = -1;
+   cmd.no_stdin = 1;
+   if (run_command(&cmd))
+   die(_("cannot repack to clean up"));
+   if (unlink(git_path("objects/info/alternates")) && errno != ENOENT)
+   die_errno(_("cannot unlink temporary alternates file"));
+}
+
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
int is_bundle = 0, is_local;
@@ -883,6 +901,10 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
 
if (option_reference.nr)
setup_reference();
+   else if (option_dissociate) {
+   warning(_("--dissociate given, but there is no --reference"));
+   option_dissociate = 0;
+   }
 
fetch_pattern = value.buf;
refspec = parse_fetch_refspec(1, &fetch_pattern);
@@ -996,6 +1018,9 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
transport_unlock_pack(transport);
transport_disconnect(transport);
 
+   if (option_dissociate)
+   dissociate_from_references();
+
junk_mode = JUNK_LEAVE_REPO;
err = checkout();
 
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 6537911..3e783fc 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-referen

Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes

2014-10-14 Thread Max Kirillov
On Tue, Oct 14, 2014 at 07:09:45PM +0200, Jens Lehmann wrote:
> Until that problem is solved it looks wrong to pass
> GIT_COMMON_DIR into submodule recursion, I believe
> GIT_COMMON_DIR should be added to the local_repo_env array
> (and even if it is passed on later, we might have to
> append "/modules/" to make it point to the
> correct location).
 
Actually, why there should be an _environment_ variable
GIT_COMMON_DIR at all? I mean, gitdir is resolved to some
directory (through link or environment), and it contains the
shared data directly or referes to it with the commondir
link. In which case anyone would want to override that
location?

I searched though tests but they don't cover this.
--
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] [kernel] completion: silence "fatal: Not a git repository" error

2014-10-14 Thread Junio C Hamano
John Szakmeister  writes:

> On Tue, Oct 14, 2014 at 2:29 PM, Junio C Hamano  wrote:
> ...
>> Hmph, do you mean this one?
>>
>> $ cd /var/tmp ;# not a git repository
>> $ git checkout 
>>
>> ->
>>
>> $ git checkout fatal: Not a git repository (or any of the parent 
>> directories): .git
>> HEAD
>>
>> I agree it is ugly, but would it be an improvement for the end user,
>> who did not realize that she was not in a directory where "git checkout"
>> makes sense, not to tell her that she is not in a git repository in
>> some way?
>
> I had thought about that too, but I think--for me--it comes down to two 
> things:
>
> 1) We're not intentionally trying to inform the user anywhere else
> that they are not in a git repo.  We simply fail to complete anything,
> which I think is an established behavior.
> 2) It mingles with the stuff already on the command line, making it
> confusing to know what you typed.  Then you end up ctrl-c'ing your way
> out of it and starting over--which is the frustrating part.

It is not that I am unsympathetic.  It's just it looks to me that
the patch is potentially adding one more failed step by hiding the
error message to further frustrate the user.

$ git checkout 
... completes nothing; puzzled but decides not to be worried for now
$ git checkout master
fatal: not a git repository

As you noticed, however, we do not show the ugly error message by
design.  It is not done consistently, either (happens only when we
try to complete refnames).

I was just hoping that somebody (not necessarily you) could suggest
a way to do better than hide the error message only because it looks
ugly (iow, perhaps show it not in the middle of the command line,
and do so more consistently).  Yes I would imagine it would be a lot
harder, but the end user experience _might_ become so much better to
make it worthwhile.  I dunno.

I am not strongly opposed to queuing the patch.
--
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] checkout: report upstream correctly even with loosely defined branch.*.merge

2014-10-14 Thread Junio C Hamano
When checking out a branch that is set to build on top of another
branch (often, a remote-tracking branch), "git checkout" reports how
your work relates to the other branch, e.g.

Your branch is behind 'origin/master', and can be fast-forwarded.

Back when this feature was introduced, this was only done for
branches that build on remote-tracking branches, but 5e6e2b48 (Make
local branches behave like remote branches when --tracked,
2009-04-01) added support to give the same report for branches that
build on other local branches (i.e. branches whose branch.*.remote
variables are set to '.').  Unlike the support for the branches
building on remote-tracking branches, however, this did not take
into account the fact that branch.*.merge configuration is allowed
to record a shortened branch name.

When branch.*.merge is set to 'master' (not 'refs/heads/master'),
i.e. "my branch builds on the local 'master' branch", this caused
"git checkout" to report:

Your branch is based on 'master', but the upstream is gone.

The upstream is our repository and is definitely not gone, so this
output is nonsense.

The fix is fairly obvious; just like the branch name is DWIMed when
"git pull" merges from the 'master' branch without complaint on such
a branch, the name of the branch the current branch builds upon
needs to be DWIMed the same way.

Signed-off-by: Junio C Hamano 
---
 remote.c | 34 +++---
 t/t2024-checkout-dwim.sh | 18 ++
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/remote.c b/remote.c
index 0e9459c..ecbe363 100644
--- a/remote.c
+++ b/remote.c
@@ -1611,6 +1611,27 @@ void set_ref_status_for_push(struct ref *remote_refs, 
int send_mirror,
}
 }
 
+static void set_merge(struct branch *ret)
+{
+   char *ref;
+   unsigned char sha1[20];
+   int i;
+
+   ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge));
+   for (i = 0; i < ret->merge_nr; i++) {
+   ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
+   ret->merge[i]->src = xstrdup(ret->merge_name[i]);
+   if (!remote_find_tracking(ret->remote, ret->merge[i]) ||
+   strcmp(ret->remote_name, "."))
+   continue;
+   if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),
+sha1, &ref) == 1)
+   ret->merge[i]->dst = ref;
+   else
+   ret->merge[i]->dst = xstrdup(ret->merge_name[i]);
+   }
+}
+
 struct branch *branch_get(const char *name)
 {
struct branch *ret;
@@ -1622,17 +1643,8 @@ struct branch *branch_get(const char *name)
ret = make_branch(name, 0);
if (ret && ret->remote_name) {
ret->remote = remote_get(ret->remote_name);
-   if (ret->merge_nr) {
-   int i;
-   ret->merge = xcalloc(ret->merge_nr, 
sizeof(*ret->merge));
-   for (i = 0; i < ret->merge_nr; i++) {
-   ret->merge[i] = xcalloc(1, 
sizeof(**ret->merge));
-   ret->merge[i]->src = 
xstrdup(ret->merge_name[i]);
-   if (remote_find_tracking(ret->remote, 
ret->merge[i])
-   && !strcmp(ret->remote_name, "."))
-   ret->merge[i]->dst = 
xstrdup(ret->merge_name[i]);
-   }
-   }
+   if (ret->merge_nr)
+   set_merge(ret);
}
return ret;
 }
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 6ecb559..468a000 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -185,4 +185,22 @@ test_expect_success 'checkout  -- succeeds, even 
if a file with the same
test_branch_upstream spam repo_c spam
 '
 
+test_expect_success 'loosely defined local base branch is reported correctly' '
+
+   git checkout master &&
+   git branch strict &&
+   git branch loose &&
+   git commit --allow-empty -m "a bit more" &&
+
+   test_config branch.strict.remote . &&
+   test_config branch.loose.remote . &&
+   test_config branch.strict.merge refs/heads/master &&
+   test_config branch.loose.merge master &&
+
+   git checkout strict | sed -e "s/strict/BRANCHNAME/g" >expect &&
+   git checkout loose | sed -e "s/loose/BRANCHNAME/g" >actual &&
+
+   test_cmp expect actual
+'
+
 test_done
-- 
2.1.2-492-gf8d07d7

--
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] Multiple worktrees vs. submodules fixes

2014-10-14 Thread Max Kirillov
On Tue, Oct 14, 2014 at 09:51:22PM +0200, Jens Lehmann wrote:
> Am 14.10.2014 um 20:34 schrieb Max Kirillov:
>> But here are a lot of nuances. For example, it makes
>> sense to have a superproject checkout without submodules
>> being initialized (so that they don't waste space and
>> machine time for working tree, which often is more than
>> repository data).
> 
> Hmm, I'm not sure if this is a problem. If the
> GIT_COMMON_DIR does have the submodule repo but it isn't
> initialized locally, we shouldn't have a problem (except
> for wasting some disk space if not a single checkout-to
> superproject initializes this submodule).

If initially a repository is clone without submodules, it
will not have anything in the GIT_COMMON_DIR.

> And if GIT_COMMON_DIR does not have the submodule repo
> yet, wouldn't it be cloned the moment we init the
> submodule in the checkout-to? Or would that need extra
> functionality?

I cannot say I like this. Network operations should be
caused only by clone and submodules.

I think the logic can be simple: it a submodule is not
checked-out in the repository "checkout --to" is called
from, then it is not checked-out to the new one also. If it
is, then checkout calls itself recursively in the submodule
and works like being run in standalone repository.

>> Then, a checkout copy of a submodule can be standalone
>> (for example, git and git-html-docs are submodules of
>> msysgit). Or, it can even belong to some other
>> superproject. And in that cases they still should be able
>> to be linked.
> 
> Maybe such configurations would have to be handled
> manually to achieve maximum savings. At least I could live
> with that.

To make manual handling of the cases, and to skip
checking-out a module.

I would think about the following interface:

$ git checkout --to ... - does not checkout submodules,
creates empty directory.

$ git checkout --recursive --to ... - if a submodule is
checked-out in source repository, recursed there and run
"checkout --recursive" again. If a submodule is not
checked-out, does not checkout it, creates an empty
directory.

By the way, I have found your branch
recursive_submodule_checkout. Would you like to revive it?
Then it can be done with the same option.

> Hmm, so I tend towards adding GIT_COMMON_DIR to
> local_repo_env until we figured out how to handle this.
> Without that I fear bad things will happen, at least for a
> superproject with multiple checkout-to work trees where
> the same submodule is initialized more than once ...

I learned about local_repo_env and agree it should include
GIT_COMMON_DIR. Unless it is removed at all...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Oct 2014, #03; Tue, 14)

2014-10-14 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* bc/asciidoc-pretty-formats-fix (2014-10-08) 1 commit
  (merged to 'next' on 2014-10-13 at 8208335)
 + Documentation: fix misrender of pretty-formats in Asciidoctor


* da/completion-show-signature (2014-10-07) 1 commit
  (merged to 'next' on 2014-10-07 at 2467c19)
 + completion: add --show-signature for log and show


* da/include-compat-util-first-in-c (2014-09-15) 1 commit
  (merged to 'next' on 2014-10-07 at ea5bcb4)
 + cleanups: ensure that git-compat-util.h is included first

 Code clean-up.


* dt/cache-tree-repair (2014-09-30) 1 commit
  (merged to 'next' on 2014-10-07 at 923bd93)
 + t0090: avoid passing empty string to printf %d
 (this branch is used by jk/prune-mtime.)

 This fixes a topic that has graduated to 'master'.


* mh/lockfile (2014-10-01) 38 commits
  (merged to 'next' on 2014-10-08 at 39cb6da)
 + lockfile.h: extract new header file for the functions in lockfile.c
 + hold_locked_index(): move from lockfile.c to read-cache.c
 + hold_lock_file_for_append(): restore errno before returning
 + get_locked_file_path(): new function
 + lockfile.c: rename static functions
 + lockfile: rename LOCK_NODEREF to LOCK_NO_DEREF
 + commit_lock_file_to(): refactor a helper out of commit_lock_file()
 + trim_last_path_component(): replace last_path_elm()
 + resolve_symlink(): take a strbuf parameter
 + resolve_symlink(): use a strbuf for internal scratch space
 + lockfile: change lock_file::filename into a strbuf
 + commit_lock_file(): use a strbuf to manage temporary space
 + try_merge_strategy(): use a statically-allocated lock_file object
 + try_merge_strategy(): remove redundant lock_file allocation
 + struct lock_file: declare some fields volatile
 + lockfile: avoid transitory invalid states
 + git_config_set_multivar_in_file(): avoid call to rollback_lock_file()
 + dump_marks(): remove a redundant call to rollback_lock_file()
 + api-lockfile: document edge cases
 + commit_lock_file(): rollback lock file on failure to rename
 + close_lock_file(): if close fails, roll back
 + commit_lock_file(): die() if called for unlocked lockfile object
 + commit_lock_file(): inline temporary variable
 + remove_lock_file(): call rollback_lock_file()
 + lock_file(): exit early if lockfile cannot be opened
 + prepare_index(): declare return value to be (const char *)
 + delete_ref_loose(): don't muck around in the lock_file's filename
 + cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN
 + lockfile.c: document the various states of lock_file objects
 + lock_file(): always initialize and register lock_file object
 + hold_lock_file_for_append(): release lock on errors
 + lockfile: unlock file if lockfile permissions cannot be adjusted
 + rollback_lock_file(): set fd to -1
 + rollback_lock_file(): exit early if lock is not active
 + rollback_lock_file(): do not clear filename redundantly
 + close_lock_file(): exit (successfully) if file is already closed
 + api-lockfile: revise and expand the documentation
 + unable_to_lock_die(): rename function from unable_to_lock_index_die()
 (this branch is used by mh/lockfile-stdio.)

 The lockfile API and its users have been cleaned up.


* mh/lockfile-stdio (2014-10-01) 3 commits
  (merged to 'next' on 2014-10-08 at e56cebc)
 + commit_packed_refs(): reimplement using fdopen_lock_file()
 + dump_marks(): reimplement using fdopen_lock_file()
 + fdopen_lock_file(): access a lockfile using stdio
 (this branch uses mh/lockfile.)


* rs/daemon-fixes (2014-10-01) 3 commits
  (merged to 'next' on 2014-10-07 at 4171e10)
 + daemon: remove write-only variable maxfd
 + daemon: fix error message after bind()
 + daemon: handle gethostbyname() error

 "git daemon" (with NO_IPV6 build configuration) used to incorrectly
 use the hostname even when gethostbyname() reported that the given
 hostname is not found.


* rs/mailsplit (2014-10-07) 1 commit
  (merged to 'next' on 2014-10-08 at 58b053e)
 + mailsplit: remove unnecessary unlink(2) call


* rs/more-uses-of-skip-prefix (2014-10-07) 1 commit
  (merged to 'next' on 2014-10-08 at cd153c0)
 + use skip_prefix() to avoid more magic numbers


* rs/plug-leak-in-bundle (2014-10-07) 1 commit
  (merged to 'next' on 2014-10-08 at 5539cd7)
 + bundle: plug minor memory leak in is_tag_in_date_range()


* rs/sha1-array-test (2014-10-01) 2 commits
  (merged to 'next' on 2014-10-08 at 5960711)
 + sha1-lookup: handle duplicates in sha1_pos()
 + sha1-array: add test-sha1-array and basic tests


* sk/tag-contains-wo-recursion (2014-09-23) 1 commit
  (merged to 'next' on 2014-10-08 at e425f54)
 + t7004: give the test a bit more stack space


* so/rebase-doc-fork-point 

color box, display box, corrugated box, color card, blister card, color sleeve, hang tag, label

2014-10-14 Thread Jinghao Printing - CHINA
Hi, this is David Wu from Shanghai, China.
We are a printing company, we can print color box, corrugated box,
label, hang tag etc.
Please let me know if you need these.

I will send you the website then.

Best regards,
David Wu
--
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 v23 0/25] rs/ref-transaction ("Use ref transactions", part 3)

2014-10-14 Thread Jonathan Nieder
This series by Ronnie was last seen on-list at [1].  It includes some
improvements to the ref-transaction API, improves handling of poorly
named refs (which should make it easier to tighten the refname format
checks in the future), and is a stepping stone toward later series
that use the ref-transaction API more and make it support alternate
backends (yay!).

The changes since (a merge of 'master' and) v22 are very minor and can
be seen below[2].  The more important change is that it's rebased
against current 'master'.

Review comments from Michael and Junio were very helpful in getting
this in shape.  Thanks much to both.

The series can also be found at

  git://repo.or.cz/git/jrn.git tags/rs/ref-transaction

It is based against current 'master' (670a3c1d, 2014-10-14) and
intended for 'next'.

Thoughts welcome, as always.  Improvements preferred in the form of
patches on top of the series.

Jonathan Nieder (6):
  mv test: recreate mod/ directory instead of relying on stale copy
  branch -d: avoid repeated symref resolution
  packed-ref cache: forbid dot-components in refnames
  refs.c: do not permit err == NULL
  lockfile: remove unable_to_lock_error
  ref_transaction_commit: bail out on failure to remove a ref

Junio C Hamano (1):
  reflog test: test interaction with detached HEAD

Ronnie Sahlberg (18):
  wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success
  refs.c: lock_ref_sha1_basic is used for all refs
  wrapper.c: add a new function unlink_or_msg
  refs.c: add an err argument to delete_ref_loose
  refs.c: pass the ref log message to _create/delete/update instead of
_commit
  rename_ref: don't ask read_ref_full where the ref came from
  refs.c: refuse to lock badly named refs in lock_ref_sha1_basic
  refs.c: call lock_ref_sha1_basic directly from commit
  refs.c: pass a list of names to skip to is_refname_available
  refs.c: ref_transaction_commit: distinguish name conflicts from other
errors
  fetch.c: change s_update_ref to use a ref transaction
  refs.c: make write_ref_sha1 static
  refs.c: change resolve_ref_unsafe reading argument to be a flags field
  branch -d: simplify by using RESOLVE_REF_READING
  test: put tests for handling of bad ref names in one place
  refs.c: allow listing and deleting badly named refs
  for-each-ref: skip and warn about broken ref names
  remote rm/prune: print a message when writing packed-refs fails

 branch.c |   6 +-
 builtin/blame.c  |   2 +-
 builtin/branch.c |  22 ++-
 builtin/checkout.c   |   6 +-
 builtin/clone.c  |   2 +-
 builtin/commit.c |   6 +-
 builtin/fetch.c  |  34 ++--
 builtin/fmt-merge-msg.c  |   2 +-
 builtin/for-each-ref.c   |  11 +-
 builtin/fsck.c   |   2 +-
 builtin/log.c|   3 +-
 builtin/merge.c  |   2 +-
 builtin/notes.c  |   2 +-
 builtin/receive-pack.c   |   9 +-
 builtin/remote.c |  20 ++-
 builtin/replace.c|   5 +-
 builtin/show-branch.c|   7 +-
 builtin/symbolic-ref.c   |   2 +-
 builtin/tag.c|   4 +-
 builtin/update-ref.c |  13 +-
 bundle.c |   2 +-
 cache.h  |  44 +++--
 fast-import.c|   8 +-
 git-compat-util.h|  16 +-
 http-backend.c   |   4 +-
 lockfile.c   |  10 --
 lockfile.h   |   1 -
 notes-merge.c|   2 +-
 reflog-walk.c|   5 +-
 refs.c   | 446 ---
 refs.h   |  44 +++--
 remote.c |  11 +-
 sequencer.c  |   8 +-
 t/t1400-update-ref.sh|  62 +++
 t/t1413-reflog-detach.sh |  70 
 t/t1430-bad-ref-name.sh  | 207 ++
 t/t3200-branch.sh|   9 +
 t/t7001-mv.sh|  15 +-
 t/t9300-fast-import.sh   |  30 
 transport-helper.c   |   5 +-
 transport.c  |   5 +-
 upload-pack.c|   2 +-
 walker.c |   5 +-
 wrapper.c|  28 ++-
 wt-status.c  |   2 +-
 45 files changed, 850 insertions(+), 351 deletions(-)
 create mode 100755 t/t1413-reflog-detach.sh
 create mode 100755 t/t1430-bad-ref-name.sh

[1] http://thread.gmane.org/gmane.comp.version-control.git/254501/focus=257771
[2]
 cache.h   | 11 ---
 git-compat-util.h |  4 +--
 refs.c| 96 +--
 refs.h|  6 +++-
 4 files changed, 64 insertions(+), 53 deletions(-)

diff --git a/cache.h b/cache.h
index 209e8ba..0501f7d 100644
--- a/cache.h
+++ b/cache.h
@@ -983,7 +983,8 @@ extern int read_ref(const char *refname, unsigned char 
*sha1);
  * packed references), REF_ISSYMREF (if the initial reference was a
  * symbolic reference), REF_BAD_NAME (if the reference name is ill
  * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN
- * (if the ref is malformed).
+ * (if the ref is malformed or has a bad name). See refs.h for mo

[PATCH 01/25] mv test: recreate mod/ directory instead of relying on stale copy

2014-10-14 Thread Jonathan Nieder
From: Jonathan Nieder 
Date: Wed, 10 Sep 2014 14:01:46 -0700

The tests for 'git mv moves a submodule' functionality often run
commands like

git mv sub mod/sub

to move a submodule into a subdirectory.  Just like plain /bin/mv,
this is supposed to succeed if the mod/ parent directory exists
and fail if it doesn't exist.

Usually these tests mkdir the parent directory beforehand, but some
instead rely on it being left behind by previous tests.

More precisely, when 'git reset --hard' tries to move to a state where
mod/sub is not present any more, it would perform the following
operations:

rmdir("mod/sub")
rmdir("mod")

The first fails with ENOENT because the test script removed mod/sub
with "rm -rf" already, so 'reset --hard' doesn't bother to move on to
the second, and the mod/ directory is kept around.

Better to explicitly remove and re-create the mod/ directory so later
tests don't have to depend on the directory left behind by the earlier
ones at all (making it easier to rearrange or skip some tests in the
file or to tweak 'reset --hard' behavior without breaking unrelated
tests).

Noticed while testing a patch that fixes the reset --hard behavior
described above.

Signed-off-by: Jonathan Nieder 
Reviewed-by: Ronnie Sahlberg 
---
 t/t7001-mv.sh | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 54d7807..69f11bd 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -350,10 +350,11 @@ test_expect_success 'git mv moves a submodule with a .git 
directory and .gitmodu
 '
 
 test_expect_success 'git mv moves a submodule with gitfile' '
-   rm -rf mod/sub &&
+   rm -rf mod &&
git reset --hard &&
git submodule update &&
entry="$(git ls-files --stage sub | cut -f 1)" &&
+   mkdir mod &&
(
cd mod &&
git mv ../sub/ .
@@ -372,11 +373,12 @@ test_expect_success 'git mv moves a submodule with 
gitfile' '
 '
 
 test_expect_success 'mv does not complain when no .gitmodules file is found' '
-   rm -rf mod/sub &&
+   rm -rf mod &&
git reset --hard &&
git submodule update &&
git rm .gitmodules &&
entry="$(git ls-files --stage sub | cut -f 1)" &&
+   mkdir mod &&
git mv sub mod/sub 2>actual.err &&
! test -s actual.err &&
! test -e sub &&
@@ -390,11 +392,12 @@ test_expect_success 'mv does not complain when no 
.gitmodules file is found' '
 '
 
 test_expect_success 'mv will error out on a modified .gitmodules file unless 
staged' '
-   rm -rf mod/sub &&
+   rm -rf mod &&
git reset --hard &&
git submodule update &&
git config -f .gitmodules foo.bar true &&
entry="$(git ls-files --stage sub | cut -f 1)" &&
+   mkdir mod &&
test_must_fail git mv sub mod/sub 2>actual.err &&
test -s actual.err &&
test -e sub &&
@@ -413,13 +416,14 @@ test_expect_success 'mv will error out on a modified 
.gitmodules file unless sta
 '
 
 test_expect_success 'mv issues a warning when section is not found in 
.gitmodules' '
-   rm -rf mod/sub &&
+   rm -rf mod &&
git reset --hard &&
git submodule update &&
git config -f .gitmodules --remove-section submodule.sub &&
git add .gitmodules &&
entry="$(git ls-files --stage sub | cut -f 1)" &&
echo "warning: Could not find section in .gitmodules where path=sub" 
>expect.err &&
+   mkdir mod &&
git mv sub mod/sub 2>actual.err &&
test_i18ncmp expect.err actual.err &&
! test -e sub &&
@@ -433,9 +437,10 @@ test_expect_success 'mv issues a warning when section is 
not found in .gitmodule
 '
 
 test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' 
'
-   rm -rf mod/sub &&
+   rm -rf mod &&
git reset --hard &&
git submodule update &&
+   mkdir mod &&
git mv -n sub mod/sub 2>actual.err &&
test -f sub/.git &&
git diff-index --exit-code HEAD &&
-- 
2.1.0.rc2.206.gedb03e5

--
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 02/25] wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Wed, 16 Jul 2014 11:01:18 -0700

Simplify the function warn_if_unremovable slightly. Additionally, change
behaviour slightly. If we failed to remove the object because the object
does not exist, we can still return success back to the caller since none of
the callers depend on "fail if the file did not exist".

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 git-compat-util.h |  7 +--
 refs.c|  2 +-
 wrapper.c | 14 ++
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index fb41118..d67592f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -777,11 +777,14 @@ void git_qsort(void *base, size_t nmemb, size_t size,
 
 /*
  * Preserves errno, prints a message, but gives no warning for ENOENT.
- * Always returns the return value of unlink(2).
+ * Returns 0 on success, which includes trying to unlink an object that does
+ * not exist.
  */
 int unlink_or_warn(const char *path);
 /*
- * Likewise for rmdir(2).
+ * Preserves errno, prints a message, but gives no warning for ENOENT.
+ * Returns 0 on success, which includes trying to remove a directory that does
+ * not exist.
  */
 int rmdir_or_warn(const char *path);
 /*
diff --git a/refs.c b/refs.c
index a77458f..2dcf6c6 100644
--- a/refs.c
+++ b/refs.c
@@ -2607,7 +2607,7 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag)
char *loose_filename = get_locked_file_path(lock->lk);
int err = unlink_or_warn(loose_filename);
free(loose_filename);
-   if (err && errno != ENOENT)
+   if (err)
return 1;
}
return 0;
diff --git a/wrapper.c b/wrapper.c
index 5b77d2a..8d4be66 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -466,14 +466,12 @@ int xmkstemp_mode(char *template, int mode)
 
 static int warn_if_unremovable(const char *op, const char *file, int rc)
 {
-   if (rc < 0) {
-   int err = errno;
-   if (ENOENT != err) {
-   warning("unable to %s %s: %s",
-   op, file, strerror(errno));
-   errno = err;
-   }
-   }
+   int err;
+   if (!rc || errno == ENOENT)
+   return 0;
+   err = errno;
+   warning("unable to %s %s: %s", op, file, strerror(errno));
+   errno = err;
return rc;
 }
 
-- 
2.1.0.rc2.206.gedb03e5

--
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 03/25] refs.c: lock_ref_sha1_basic is used for all refs

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 2 Oct 2014 07:59:02 -0700

lock_ref_sha1_basic is used to lock refs that sit directly in the .git
dir such as HEAD and MERGE_HEAD in addition to the more ordinary refs
under "refs/".  Remove the note claiming otherwise.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 2dcf6c6..4f2564d 100644
--- a/refs.c
+++ b/refs.c
@@ -2134,7 +2134,7 @@ int dwim_log(const char *str, int len, unsigned char 
*sha1, char **log)
 }
 
 /*
- * Locks a "refs/" ref returning the lock on success and NULL on failure.
+ * Locks a ref returning the lock on success and NULL on failure.
  * On failure errno is set to something meaningful.
  */
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
-- 
2.1.0.rc2.206.gedb03e5

--
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 04/25] wrapper.c: add a new function unlink_or_msg

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Wed, 16 Jul 2014 11:20:36 -0700

This behaves like unlink_or_warn except that on failure it writes the message
to its 'err' argument, which the caller can display in an appropriate way or
ignore.

Signed-off-by: Ronnie Sahlberg 
Reviewed-by: Michael Haggerty 
Signed-off-by: Jonathan Nieder 
---
 git-compat-util.h |  9 +
 wrapper.c | 14 ++
 2 files changed, 23 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index d67592f..59ecf21 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -326,6 +326,8 @@ static inline char *git_find_last_dir_sep(const char *path)
 
 #include "wildmatch.h"
 
+struct strbuf;
+
 /* General helper functions */
 extern void vreportf(const char *prefix, const char *err, va_list params);
 extern void vwritef(int fd, const char *prefix, const char *err, va_list 
params);
@@ -781,6 +783,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,
  * not exist.
  */
 int unlink_or_warn(const char *path);
+ /*
+  * Tries to unlink file.  Returns 0 if unlink succeeded
+  * or the file already didn't exist.  Returns -1 and
+  * appends a message to err suitable for
+  * 'error("%s", err->buf)' on error.
+  */
+int unlink_or_msg(const char *file, struct strbuf *err);
 /*
  * Preserves errno, prints a message, but gives no warning for ENOENT.
  * Returns 0 on success, which includes trying to remove a directory that does
diff --git a/wrapper.c b/wrapper.c
index 8d4be66..007ec0d 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -475,6 +475,20 @@ static int warn_if_unremovable(const char *op, const char 
*file, int rc)
return rc;
 }
 
+int unlink_or_msg(const char *file, struct strbuf *err)
+{
+   int rc = unlink(file);
+
+   assert(err);
+
+   if (!rc || errno == ENOENT)
+   return 0;
+
+   strbuf_addf(err, "unable to unlink %s: %s",
+   file, strerror(errno));
+   return -1;
+}
+
 int unlink_or_warn(const char *file)
 {
return warn_if_unremovable("unlink", file, unlink(file));
-- 
2.1.0.rc2.206.gedb03e5

--
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 05/25] refs.c: add an err argument to delete_ref_loose

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 15 May 2014 08:25:23 -0700

Add an err argument to delete_ref_loose so that we can pass a descriptive
error string back to the caller. Pass the err argument from transaction
commit to this function so that transaction users will have a nice error
string if the transaction failed due to delete_ref_loose.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 4f2564d..430857b 100644
--- a/refs.c
+++ b/refs.c
@@ -2597,7 +2597,7 @@ int repack_without_refs(const char **refnames, int n, 
struct strbuf *err)
return ret;
 }
 
-static int delete_ref_loose(struct ref_lock *lock, int flag)
+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf 
*err)
 {
if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
/*
@@ -2605,9 +2605,9 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag)
 * lockfile name, minus ".lock":
 */
char *loose_filename = get_locked_file_path(lock->lk);
-   int err = unlink_or_warn(loose_filename);
+   int res = unlink_or_msg(loose_filename, err);
free(loose_filename);
-   if (err)
+   if (res)
return 1;
}
return 0;
@@ -3658,7 +3658,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
struct ref_update *update = updates[i];
 
if (update->lock) {
-   ret |= delete_ref_loose(update->lock, update->type);
+   ret |= delete_ref_loose(update->lock, update->type,
+   err);
if (!(update->flags & REF_ISPRUNING))
delnames[delnum++] = update->lock->ref_name;
}
-- 
2.1.0.rc2.206.gedb03e5

--
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 06/25] refs.c: pass the ref log message to _create/delete/update instead of _commit

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Wed, 30 Apr 2014 12:22:42 -0700

Change the ref transaction API so that we pass the reflog message to the
create/delete/update functions instead of to ref_transaction_commit.
This allows different reflog messages for each ref update in a multi-ref
transaction.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 branch.c   |  4 ++--
 builtin/commit.c   |  4 ++--
 builtin/receive-pack.c |  5 +++--
 builtin/replace.c  |  5 +++--
 builtin/tag.c  |  4 ++--
 builtin/update-ref.c   | 13 +++--
 fast-import.c  |  8 
 refs.c | 34 +-
 refs.h | 12 ++--
 sequencer.c|  4 ++--
 walker.c   |  5 ++---
 11 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/branch.c b/branch.c
index 9a2228e..884c62c 100644
--- a/branch.c
+++ b/branch.c
@@ -285,8 +285,8 @@ void create_branch(const char *head,
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, sha1,
-  null_sha1, 0, !forcing, &err) ||
-   ref_transaction_commit(transaction, msg, &err))
+  null_sha1, 0, !forcing, msg, &err) ||
+   ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);
strbuf_release(&err);
diff --git a/builtin/commit.c b/builtin/commit.c
index 81dc622..a7857ab 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1811,8 +1811,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
ref_transaction_update(transaction, "HEAD", sha1,
   current_head
   ? current_head->object.sha1 : NULL,
-  0, !!current_head, &err) ||
-   ref_transaction_commit(transaction, sb.buf, &err)) {
+  0, !!current_head, sb.buf, &err) ||
+   ref_transaction_commit(transaction, &err)) {
rollback_index_files();
die("%s", err.buf);
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f2f6c67..df6c337 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -842,8 +842,9 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, namespaced_name,
-  new_sha1, old_sha1, 0, 1, &err) ||
-   ref_transaction_commit(transaction, "push", &err)) {
+  new_sha1, old_sha1, 0, 1, "push",
+  &err) ||
+   ref_transaction_commit(transaction, &err)) {
ref_transaction_free(transaction);
 
rp_error("%s", err.buf);
diff --git a/builtin/replace.c b/builtin/replace.c
index 8020db8..85d39b5 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -171,8 +171,9 @@ static int replace_object_sha1(const char *object_ref,
 
transaction = ref_transaction_begin(&err);
if (!transaction ||
-   ref_transaction_update(transaction, ref, repl, prev, 0, 1, &err) ||
-   ref_transaction_commit(transaction, NULL, &err))
+   ref_transaction_update(transaction, ref, repl, prev,
+  0, 1, NULL, &err) ||
+   ref_transaction_commit(transaction, &err))
die("%s", err.buf);
 
ref_transaction_free(transaction);
diff --git a/builtin/tag.c b/builtin/tag.c
index a81b9e4..e633f4e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -733,8 +733,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
transaction = ref_transaction_begin(&err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
-  0, 1, &err) ||
-   ref_transaction_commit(transaction, NULL, &err))
+  0, 1, NULL, &err) ||
+   ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);
if (force && !is_null_sha1(prev) && hashcmp(prev, object))
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 54a48c0..6c9be05 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = {
 
 static char line_termination = '\n';
 static int update_flags;
+static const char *msg;
 
 /*
  * Parse one whitespace- or NUL-terminated, possibly C-quoted argument
@@ -198,7 +199,7 @@ static const char *parse_

[PATCH 07/25] rename_ref: don't ask read_ref_full where the ref came from

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Wed, 30 Apr 2014 12:41:04 -0700

We call read_ref_full with a pointer to flags from rename_ref but since
we never actually use the returned flags we can just pass NULL here instead.

Signed-off-by: Ronnie Sahlberg 
Reviewed-by: Michael Haggerty 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index f5d7bd7..3c45615 100644
--- a/refs.c
+++ b/refs.c
@@ -2721,7 +2721,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
goto rollback;
}
 
-   if (!read_ref_full(newrefname, sha1, 1, &flag) &&
+   if (!read_ref_full(newrefname, sha1, 1, NULL) &&
delete_ref(newrefname, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
if (remove_empty_directories(git_path("%s", 
newrefname))) {
-- 
2.1.0.rc2.206.gedb03e5

--
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 09/25] refs.c: call lock_ref_sha1_basic directly from commit

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 1 May 2014 10:43:39 -0700

Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic
directly from the commit function.

Signed-off-by: Ronnie Sahlberg 
Reviewed-by: Michael Haggerty 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index 9c01623..b591b9c 100644
--- a/refs.c
+++ b/refs.c
@@ -3632,12 +3632,12 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
 
-   update->lock = lock_any_ref_for_update(update->refname,
-  (update->have_old ?
-   update->old_sha1 :
-   NULL),
-  update->flags,
-  &update->type);
+   update->lock = lock_ref_sha1_basic(update->refname,
+  (update->have_old ?
+   update->old_sha1 :
+   NULL),
+  update->flags,
+  &update->type);
if (!update->lock) {
if (err)
strbuf_addf(err, "Cannot lock the ref '%s'.",
-- 
2.1.0.rc2.206.gedb03e5

--
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 08/25] refs.c: refuse to lock badly named refs in lock_ref_sha1_basic

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 1 May 2014 10:40:10 -0700

Move the check for check_refname_format from lock_any_ref_for_update to
lock_ref_sha1_basic.  At some later stage we will get rid of
lock_any_ref_for_update completely.  This has no visible impact to callers
except for the inability to lock badly named refs, which is not possible
today already for other reasons.(*)

Keep lock_any_ref_for_update as a no-op wrapper.  It is the public facing
version of this interface and keeping it as a separate function will make
it easier to experiment with the internal lock_ref_sha1_basic signature.

(*) For example, if lock_ref_sha1_basic checks the refname format and
refuses to lock badly named refs, it will not be possible to delete
such refs because the first step of deletion is to lock the ref.  We
currently already fail in that case because these refs are not recognized
to exist:

 $ cp .git/refs/heads/master .git/refs/heads/echo...\*\*
 $ git branch -D .git/refs/heads/echo...\*\*
 error: branch '.git/refs/heads/echo...**' not found.

This has been broken for a while.  Later patches in the series will start
repairing the handling of badly named refs.

Signed-off-by: Ronnie Sahlberg 
Reviewed-by: Michael Haggerty 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 3c45615..9c01623 100644
--- a/refs.c
+++ b/refs.c
@@ -2150,6 +2150,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
int missing = 0;
int attempts_remaining = 3;
 
+   if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+   errno = EINVAL;
+   return NULL;
+   }
+
lock = xcalloc(1, sizeof(struct ref_lock));
lock->lock_fd = -1;
 
@@ -2241,8 +2246,6 @@ struct ref_lock *lock_any_ref_for_update(const char 
*refname,
 const unsigned char *old_sha1,
 int flags, int *type_p)
 {
-   if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
-   return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
 }
 
-- 
2.1.0.rc2.206.gedb03e5

--
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 10/25] refs.c: pass a list of names to skip to is_refname_available

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 1 May 2014 11:16:07 -0700

Change is_refname_available to take a list of strings to exclude when
checking for conflicts instead of just one single name. We can already
exclude a single name for the sake of renames. This generalizes that support.

ref_transaction_commit already tracks a set of refs that are being deleted
in an array.  This array is then used to exclude refs from being written to
the packed-refs file.  At some stage we will want to change this array to a
struct string_list and then we can pass it to is_refname_available via the
call to lock_ref_sha1_basic.  That will allow us to perform transactions
that perform multiple renames as long as there are no conflicts within the
starting or ending state.

For example, that would allow a single transaction that contains two
renames that are both individually conflicting:

   m -> n/n
   n -> m/m

No functional change intended yet.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 50 --
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/refs.c b/refs.c
index b591b9c..a007cf3 100644
--- a/refs.c
+++ b/refs.c
@@ -787,13 +787,13 @@ static void prime_ref_dir(struct ref_dir *dir)
}
 }
 
-static int entry_matches(struct ref_entry *entry, const char *refname)
+static int entry_matches(struct ref_entry *entry, const struct string_list 
*list)
 {
-   return refname && !strcmp(entry->name, refname);
+   return list && string_list_has_string(list, entry->name);
 }
 
 struct nonmatching_ref_data {
-   const char *skip;
+   const struct string_list *skip;
struct ref_entry *found;
 };
 
@@ -817,16 +817,19 @@ static void report_refname_conflict(struct ref_entry 
*entry,
 /*
  * Return true iff a reference named refname could be created without
  * conflicting with the name of an existing reference in dir.  If
- * oldrefname is non-NULL, ignore potential conflicts with oldrefname
- * (e.g., because oldrefname is scheduled for deletion in the same
+ * skip is non-NULL, ignore potential conflicts with refs in skip
+ * (e.g., because they are scheduled for deletion in the same
  * operation).
  *
  * Two reference names conflict if one of them exactly matches the
  * leading components of the other; e.g., "foo/bar" conflicts with
  * both "foo" and with "foo/bar/baz" but not with "foo/bar" or
  * "foo/barbados".
+ *
+ * skip must be sorted.
  */
-static int is_refname_available(const char *refname, const char *oldrefname,
+static int is_refname_available(const char *refname,
+   const struct string_list *skip,
struct ref_dir *dir)
 {
const char *slash;
@@ -840,12 +843,12 @@ static int is_refname_available(const char *refname, 
const char *oldrefname,
 * looking for a conflict with a leaf entry.
 *
 * If we find one, we still must make sure it is
-* not "oldrefname".
+* not in "skip".
 */
pos = search_ref_dir(dir, refname, slash - refname);
if (pos >= 0) {
struct ref_entry *entry = dir->entries[pos];
-   if (entry_matches(entry, oldrefname))
+   if (entry_matches(entry, skip))
return 1;
report_refname_conflict(entry, refname);
return 0;
@@ -878,13 +881,13 @@ static int is_refname_available(const char *refname, 
const char *oldrefname,
/*
 * We found a directory named "refname". It is a
 * problem iff it contains any ref that is not
-* "oldrefname".
+* in "skip".
 */
struct ref_entry *entry = dir->entries[pos];
struct ref_dir *dir = get_ref_dir(entry);
struct nonmatching_ref_data data;
 
-   data.skip = oldrefname;
+   data.skip = skip;
sort_ref_dir(dir);
if (!do_for_each_entry_in_dir(dir, 0, nonmatching_ref_fn, 
&data))
return 1;
@@ -2139,6 +2142,7 @@ int dwim_log(const char *str, int len, unsigned char 
*sha1, char **log)
  */
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
+   const struct string_list *skip,
int flags, int *type_p)
 {
char *ref_file;
@@ -2188,7 +2192,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 * name is a proper prefix of our refname.
 */
if (missing &&
-!is_refname_available(refname, NULL, get_packed_refs(&ref_cache))) 
{
+!is_refname_available(refname, skip, get_packed_refs(&ref_cache)

[PATCH 11/25] refs.c: ref_transaction_commit: distinguish name conflicts from other errors

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Fri, 16 May 2014 14:14:38 -0700

In _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either
when we lstat the new refname or if the name checking function reports that
the same type of conflict happened.  In both cases, it means that we can not
create the new ref due to a name conflict.

Start defining specific return codes for _commit.  TRANSACTION_NAME_CONFLICT
refers to a failure to create a ref due to a name conflict with another ref.
TRANSACTION_GENERIC_ERROR is for all other errors.

When "git fetch" is creating refs, name conflicts differ from other errors in
that they are likely to be resolved by running "git remote prune ".
"git fetch" currently inspects errno to decide whether to give that advice.
Once it switches to the transaction API, it can check for
TRANSACTION_NAME_CONFLICT instead.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 26 --
 refs.h |  9 +++--
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index a007cf3..9d9bbeb 100644
--- a/refs.c
+++ b/refs.c
@@ -3637,9 +3637,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
-   ret = ref_update_reject_duplicates(updates, n, err);
-   if (ret)
+   if (ref_update_reject_duplicates(updates, n, err)) {
+   ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
+   }
 
/* Acquire all locks while verifying old values */
for (i = 0; i < n; i++) {
@@ -3653,10 +3654,12 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
   update->flags,
   &update->type);
if (!update->lock) {
+   ret = (errno == ENOTDIR)
+   ? TRANSACTION_NAME_CONFLICT
+   : TRANSACTION_GENERIC_ERROR;
if (err)
strbuf_addf(err, "Cannot lock the ref '%s'.",
update->refname);
-   ret = 1;
goto cleanup;
}
}
@@ -3666,15 +3669,16 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
struct ref_update *update = updates[i];
 
if (!is_null_sha1(update->new_sha1)) {
-   ret = write_ref_sha1(update->lock, update->new_sha1,
-update->msg);
-   update->lock = NULL; /* freed by write_ref_sha1 */
-   if (ret) {
+   if (write_ref_sha1(update->lock, update->new_sha1,
+  update->msg)) {
+   update->lock = NULL; /* freed by write_ref_sha1 
*/
if (err)
strbuf_addf(err, "Cannot update the ref 
'%s'.",
update->refname);
+   ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
}
+   update->lock = NULL; /* freed by write_ref_sha1 */
}
}
 
@@ -3683,14 +3687,16 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
struct ref_update *update = updates[i];
 
if (update->lock) {
-   ret |= delete_ref_loose(update->lock, update->type,
-   err);
+   if (delete_ref_loose(update->lock, update->type, err))
+   ret = TRANSACTION_GENERIC_ERROR;
+
if (!(update->flags & REF_ISPRUNING))
delnames[delnum++] = update->lock->ref_name;
}
}
 
-   ret |= repack_without_refs(delnames, delnum, err);
+   if (repack_without_refs(delnames, delnum, err))
+   ret = TRANSACTION_GENERIC_ERROR;
for (i = 0; i < delnum; i++)
unlink_or_warn(git_path("logs/%s", delnames[i]));
clear_loose_ref_cache(&ref_cache);
diff --git a/refs.h b/refs.h
index 1a55cab..50b115a 100644
--- a/refs.h
+++ b/refs.h
@@ -326,9 +326,14 @@ int ref_transaction_delete(struct ref_transaction 
*transaction,
 
 /*
  * Commit all of the changes that have been queued in transaction, as
- * atomically as possible.  Return a nonzero value if there is a
- * problem.
+ * atomically as possible.
+ *
+ * Returns 0 for success, or one of the below error codes for errors.
  */
+/* Naming conflict (for example, the ref names A and A/B conflict). */
+#define TRANSACTION_NAME_CONFLICT -1
+/* All other errors. */
+#define TRANSACTION_GENERIC

[PATCH 12/25] fetch.c: change s_update_ref to use a ref transaction

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Mon, 28 Apr 2014 13:49:07 -0700

Change s_update_ref to use a ref transaction for the ref update.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 builtin/fetch.c | 34 --
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 159fb7e..6ffd023 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -404,23 +404,37 @@ static int s_update_ref(const char *action,
 {
char msg[1024];
char *rla = getenv("GIT_REFLOG_ACTION");
-   static struct ref_lock *lock;
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
+   int ret, df_conflict = 0;
 
if (dry_run)
return 0;
if (!rla)
rla = default_rla.buf;
snprintf(msg, sizeof(msg), "%s: %s", rla, action);
-   lock = lock_any_ref_for_update(ref->name,
-  check_old ? ref->old_sha1 : NULL,
-  0, NULL);
-   if (!lock)
-   return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
-   if (write_ref_sha1(lock, ref->new_sha1, msg) < 0)
-   return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
+
+   transaction = ref_transaction_begin(&err);
+   if (!transaction ||
+   ref_transaction_update(transaction, ref->name, ref->new_sha1,
+  ref->old_sha1, 0, check_old, msg, &err))
+   goto fail;
+
+   ret = ref_transaction_commit(transaction, &err);
+   if (ret) {
+   df_conflict = (ret == TRANSACTION_NAME_CONFLICT);
+   goto fail;
+   }
+
+   ref_transaction_free(transaction);
+   strbuf_release(&err);
return 0;
+fail:
+   ref_transaction_free(transaction);
+   error("%s", err.buf);
+   strbuf_release(&err);
+   return df_conflict ? STORE_REF_ERROR_DF_CONFLICT
+  : STORE_REF_ERROR_OTHER;
 }
 
 #define REFCOL_WIDTH  10
-- 
2.1.0.rc2.206.gedb03e5

--
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 13/25] refs.c: make write_ref_sha1 static

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Mon, 28 Apr 2014 15:36:58 -0700

No external users call write_ref_sha1 any more so let's declare it static.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 refs.c | 10 --
 refs.h |  3 ---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index 9d9bbeb..6b4236a 100644
--- a/refs.c
+++ b/refs.c
@@ -2706,6 +2706,9 @@ static int rename_ref_available(const char *oldname, 
const char *newname)
return ret;
 }
 
+static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
+ const char *logmsg);
+
 int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
 {
unsigned char sha1[20], orig_sha1[20];
@@ -2950,8 +2953,11 @@ int is_branch(const char *refname)
return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
 }
 
-/* This function must return a meaningful errno */
-int write_ref_sha1(struct ref_lock *lock,
+/*
+ * Write sha1 into the ref specified by the lock. Make sure that errno
+ * is sane on error.
+ */
+static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
 {
static char term = '\n';
diff --git a/refs.h b/refs.h
index 50b115a..eea1044 100644
--- a/refs.h
+++ b/refs.h
@@ -197,9 +197,6 @@ extern int commit_ref(struct ref_lock *lock);
 /** Release any lock taken but not written. **/
 extern void unlock_ref(struct ref_lock *lock);
 
-/** Writes sha1 into the ref specified by the lock. **/
-extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, 
const char *msg);
-
 /*
  * Setup reflog before using. Set errno to something meaningful on failure.
  */
-- 
2.1.0.rc2.206.gedb03e5

--
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 14/25] refs.c: change resolve_ref_unsafe reading argument to be a flags field

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Tue, 15 Jul 2014 12:59:36 -0700

resolve_ref_unsafe takes a boolean argument for reading (a nonexistent ref
resolves successfully for writing but not for reading).  Change this to be
a flags field instead, and pass the new constant RESOLVE_REF_READING when
we want this behaviour.

While at it, swap two of the arguments in the function to put output
arguments at the end.  As a nice side effect, this ensures that we can
catch callers that were unaware of the new API so they can be audited.

Give the wrapper functions resolve_refdup and read_ref_full the same
treatment for consistency.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 branch.c|  2 +-
 builtin/blame.c |  2 +-
 builtin/branch.c|  9 ++---
 builtin/checkout.c  |  6 ++--
 builtin/clone.c |  2 +-
 builtin/commit.c|  2 +-
 builtin/fmt-merge-msg.c |  2 +-
 builtin/for-each-ref.c  |  6 ++--
 builtin/fsck.c  |  2 +-
 builtin/log.c   |  3 +-
 builtin/merge.c |  2 +-
 builtin/notes.c |  2 +-
 builtin/receive-pack.c  |  4 +--
 builtin/remote.c|  5 +--
 builtin/show-branch.c   |  7 ++--
 builtin/symbolic-ref.c  |  2 +-
 bundle.c|  2 +-
 cache.h | 23 ++--
 http-backend.c  |  4 ++-
 notes-merge.c   |  2 +-
 reflog-walk.c   |  5 +--
 refs.c  | 93 -
 remote.c| 11 +++---
 sequencer.c |  4 +--
 transport-helper.c  |  5 ++-
 transport.c |  5 +--
 upload-pack.c   |  2 +-
 wt-status.c |  2 +-
 28 files changed, 124 insertions(+), 92 deletions(-)

diff --git a/branch.c b/branch.c
index 884c62c..4bab55a 100644
--- a/branch.c
+++ b/branch.c
@@ -170,7 +170,7 @@ int validate_new_branchname(const char *name, struct strbuf 
*ref,
const char *head;
unsigned char sha1[20];
 
-   head = resolve_ref_unsafe("HEAD", sha1, 0, NULL);
+   head = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
if (!is_bare_repository() && head && !strcmp(head, ref->buf))
die(_("Cannot force update the current branch."));
}
diff --git a/builtin/blame.c b/builtin/blame.c
index 3838be2..303e217 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2286,7 +2286,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
commit->date = now;
parent_tail = &commit->parents;
 
-   if (!resolve_ref_unsafe("HEAD", head_sha1, 1, NULL))
+   if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
die("no such ref: HEAD");
 
parent_tail = append_parent(parent_tail, head_sha1);
diff --git a/builtin/branch.c b/builtin/branch.c
index 6785097..6491bac 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -131,7 +131,8 @@ static int branch_merged(int kind, const char *name,
branch->merge[0] &&
branch->merge[0]->dst &&
(reference_name = reference_name_to_free =
-resolve_refdup(branch->merge[0]->dst, sha1, 1, NULL)) != 
NULL)
+resolve_refdup(branch->merge[0]->dst, RESOLVE_REF_READING,
+   sha1, NULL)) != NULL)
reference_rev = lookup_commit_reference(sha1);
}
if (!reference_rev)
@@ -235,7 +236,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
free(name);
 
name = mkpathdup(fmt, bname.buf);
-   target = resolve_ref_unsafe(name, sha1, 0, &flags);
+   target = resolve_ref_unsafe(name, 0, sha1, &flags);
if (!target ||
(!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {
error(remote_branch
@@ -299,7 +300,7 @@ static char *resolve_symref(const char *src, const char 
*prefix)
int flag;
const char *dst;
 
-   dst = resolve_ref_unsafe(src, sha1, 0, &flag);
+   dst = resolve_ref_unsafe(src, 0, sha1, &flag);
if (!(dst && (flag & REF_ISSYMREF)))
return NULL;
if (prefix)
@@ -869,7 +870,7 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
 
track = git_branch_track;
 
-   head = resolve_refdup("HEAD", head_sha1, 0, NULL);
+   head = resolve_refdup("HEAD", 0, head_sha1, NULL);
if (!head)
die(_("Failed to resolve HEAD as a valid ref."));
if (!strcmp(head, "HEAD"))
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b4decd5..5410dac 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -355,7 +355,7 @@ static int checkout_paths(const struct checkout_opts *opts,
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
die(_("unable to write new index file"))

[PATCH 16/25] branch -d: avoid repeated symref resolution

2014-10-14 Thread Jonathan Nieder
Date: Wed, 10 Sep 2014 18:22:48 -0700

If a repository gets in a broken state with too much symref nesting,
it cannot be repaired with "git branch -d":

 $ git symbolic-ref refs/heads/nonsense refs/heads/nonsense
 $ git branch -d nonsense
 error: branch 'nonsense' not found.

Worse, "git update-ref --no-deref -d" doesn't work for such repairs
either:

 $ git update-ref -d refs/heads/nonsense
 error: unable to resolve reference refs/heads/nonsense: Too many levels of 
symbolic links

Fix both by teaching resolve_ref_unsafe a new RESOLVE_REF_NO_RECURSE
flag and passing it when appropriate.

Callers can still read the value of a symref (for example to print a
message about it) with that flag set --- resolve_ref_unsafe will
resolve one level of symrefs and stop there.

Signed-off-by: Jonathan Nieder 
Reviewed-by: Ronnie Sahlberg 
---
 builtin/branch.c  |  3 ++-
 cache.h   |  6 ++
 refs.c| 17 +++--
 refs.h|  2 ++
 t/t1400-update-ref.sh | 34 ++
 t/t3200-branch.sh |  9 +
 6 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 6491bac..2ad2d0b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -236,7 +236,8 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
free(name);
 
name = mkpathdup(fmt, bname.buf);
-   target = resolve_ref_unsafe(name, 0, sha1, &flags);
+   target = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE,
+   sha1, &flags);
if (!target ||
(!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {
error(remote_branch
diff --git a/cache.h b/cache.h
index 7200639..b735f3f 100644
--- a/cache.h
+++ b/cache.h
@@ -973,6 +973,11 @@ extern int read_ref(const char *refname, unsigned char 
*sha1);
  *   "writing" to the ref, the return value is the name of the ref
  *   that will actually be created or changed.
  *
+ * If the RESOLVE_REF_NO_RECURSE flag is passed, only resolves one
+ * level of symbolic reference.  The value stored in sha1 for a symbolic
+ * reference will always be null_sha1 in this case, and the return
+ * value is the reference that the symref refers to directly.
+ *
  * If flags is non-NULL, set the value that it points to the
  * combination of REF_ISPACKED (if the reference was found among the
  * packed references), REF_ISSYMREF (if the initial reference was a
@@ -985,6 +990,7 @@ extern int read_ref(const char *refname, unsigned char 
*sha1);
  * errno is set to something meaningful on error.
  */
 #define RESOLVE_REF_READING 0x01
+#define RESOLVE_REF_NO_RECURSE 0x02
 extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, 
unsigned char *sha1, int *flags);
 extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char 
*sha1, int *flags);
 
diff --git a/refs.c b/refs.c
index f8d59ab..4fe263e 100644
--- a/refs.c
+++ b/refs.c
@@ -1467,6 +1467,10 @@ const char *resolve_ref_unsafe(const char *refname, int 
resolve_flags, unsigned
refname = refname_buffer;
if (flags)
*flags |= REF_ISSYMREF;
+   if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
+   hashclr(sha1);
+   return refname;
+   }
continue;
}
}
@@ -1523,13 +1527,17 @@ const char *resolve_ref_unsafe(const char *refname, int 
resolve_flags, unsigned
buf = buffer + 4;
while (isspace(*buf))
buf++;
+   refname = strcpy(refname_buffer, buf);
+   if (resolve_flags & RESOLVE_REF_NO_RECURSE) {
+   hashclr(sha1);
+   return refname;
+   }
if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
if (flags)
*flags |= REF_ISBROKEN;
errno = EINVAL;
return NULL;
}
-   refname = strcpy(refname_buffer, buf);
}
 }
 
@@ -2170,6 +2178,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 
if (mustexist)
resolve_flags |= RESOLVE_REF_READING;
+   if (flags & REF_NODEREF && flags & REF_DELETING)
+   resolve_flags |= RESOLVE_REF_NO_RECURSE;
 
refname = resolve_ref_unsafe(refname, resolve_flags,
 lock->old_sha1, &type);
@@ -3664,13 +3674,16 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
/* Acquire all locks while verifying old values */
for (i = 0; i < n; i++) {
 

[PATCH 15/25] reflog test: test interaction with detached HEAD

2014-10-14 Thread Jonathan Nieder
From: Junio C Hamano 
Date: Sat, 13 Sep 2014 10:52:25 -0700

A proposed patch produced broken HEAD reflog entries when checking out
anything other than a branch.  The testsuite still passed, so it took
a few days for the bug to be noticed.

Add tests checking the content of the reflog after detaching and
reattaching HEAD so we don't have to rely on manual testing to catch
such problems in the future.

[jn: using 'log -g --format=%H' instead of parsing --oneline output,
 resetting state in each test so they can be safely reordered or
 skipped]

Signed-off-by: Jonathan Nieder 
Reviewed-by: Ronnie Sahlberg 
---
 t/t1413-reflog-detach.sh | 70 
 1 file changed, 70 insertions(+)
 create mode 100755 t/t1413-reflog-detach.sh

diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
new file mode 100755
index 000..c730600
--- /dev/null
+++ b/t/t1413-reflog-detach.sh
@@ -0,0 +1,70 @@
+#!/bin/sh
+
+test_description='Test reflog interaction with detached HEAD'
+. ./test-lib.sh
+
+reset_state () {
+   git checkout master &&
+   cp saved_reflog .git/logs/HEAD
+}
+
+test_expect_success setup '
+   test_tick &&
+   git commit --allow-empty -m initial &&
+   git branch side &&
+   test_tick &&
+   git commit --allow-empty -m second &&
+   cat .git/logs/HEAD >saved_reflog
+'
+
+test_expect_success baseline '
+   reset_state &&
+   git rev-parse master master^ >expect &&
+   git log -g --format=%H >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'switch to branch' '
+   reset_state &&
+   git rev-parse side master master^ >expect &&
+   git checkout side &&
+   git log -g --format=%H >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'detach to other' '
+   reset_state &&
+   git rev-parse master side master master^ >expect &&
+   git checkout side &&
+   git checkout master^0 &&
+   git log -g --format=%H >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'detach to self' '
+   reset_state &&
+   git rev-parse master master master^ >expect &&
+   git checkout master^0 &&
+   git log -g --format=%H >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'attach to self' '
+   reset_state &&
+   git rev-parse master master master master^ >expect &&
+   git checkout master^0 &&
+   git checkout master &&
+   git log -g --format=%H >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'attach to other' '
+   reset_state &&
+   git rev-parse side master master master^ >expect &&
+   git checkout master^0 &&
+   git checkout side &&
+   git log -g --format=%H >actual &&
+   test_cmp expect actual
+'
+
+test_done
-- 
2.1.0.rc2.206.gedb03e5

--
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 18/25] packed-ref cache: forbid dot-components in refnames

2014-10-14 Thread Jonathan Nieder
Date: Fri, 26 Sep 2014 12:22:22 -0700

Since v1.7.9-rc1~10^2 (write_head_info(): handle "extra refs" locally,
2012-01-06), this trick to keep track of ".have" refs that are only
valid on the wire and not on the filesystem is not needed any more.

Simplify by removing support for the REFNAME_DOT_COMPONENT flag.

This means we'll be slightly stricter with invalid refs found in a
packed-refs file or during clone.  read_loose_refs() already checks
for and skips refnames with .components so it is not affected.

Signed-off-by: Jonathan Nieder 
Reviewed-by: Ronnie Sahlberg 
---
 refs.c | 14 +++---
 refs.h |  6 +-
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/refs.c b/refs.c
index 4fe263e..fe1352a 100644
--- a/refs.c
+++ b/refs.c
@@ -70,16 +70,8 @@ static int check_refname_component(const char *refname, int 
flags)
 out:
if (cp == refname)
return 0; /* Component has zero length. */
-   if (refname[0] == '.') {
-   if (!(flags & REFNAME_DOT_COMPONENT))
-   return -1; /* Component starts with '.'. */
-   /*
-* Even if leading dots are allowed, don't allow "."
-* as a component (".." is prevented by a rule above).
-*/
-   if (refname[1] == '\0')
-   return -1; /* Component equals ".". */
-   }
+   if (refname[0] == '.')
+   return -1; /* Component starts with '.'. */
if (cp - refname >= LOCK_SUFFIX_LEN &&
!memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN))
return -1; /* Refname ends with ".lock". */
@@ -290,7 +282,7 @@ static struct ref_entry *create_ref_entry(const char 
*refname,
struct ref_entry *ref;
 
if (check_name &&
-   check_refname_format(refname, 
REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
+   check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
die("Reference has invalid format: '%s'", refname);
len = strlen(refname) + 1;
ref = xmalloc(sizeof(struct ref_entry) + len);
diff --git a/refs.h b/refs.h
index 79802d7..c61b8f4 100644
--- a/refs.h
+++ b/refs.h
@@ -229,7 +229,6 @@ extern int for_each_reflog(each_ref_fn, void *);
 
 #define REFNAME_ALLOW_ONELEVEL 1
 #define REFNAME_REFSPEC_PATTERN 2
-#define REFNAME_DOT_COMPONENT 4
 
 /*
  * Return 0 iff refname has the correct format for a refname according
@@ -237,10 +236,7 @@ extern int for_each_reflog(each_ref_fn, void *);
  * If REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
  * reference names.  If REFNAME_REFSPEC_PATTERN is set in flags, then
  * allow a "*" wildcard character in place of one of the name
- * components.  No leading or repeated slashes are accepted.  If
- * REFNAME_DOT_COMPONENT is set in flags, then allow refname
- * components to start with "." (but not a whole component equal to
- * "." or "..").
+ * components.  No leading or repeated slashes are accepted.
  */
 extern int check_refname_format(const char *refname, int flags);
 
-- 
2.1.0.rc2.206.gedb03e5

--
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 17/25] branch -d: simplify by using RESOLVE_REF_READING

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 11 Sep 2014 10:34:36 -0700

When "git branch -d" reads the branch it is about to delete, it used
to avoid passing the RESOLVE_REF_READING ('treat missing ref as
error') flag because a symref pointing to a nonexistent ref would show
up as missing instead of as something that could be deleted.  To check
if a ref is actually missing, we then check

 - is it a symref?
 - if not, did it resolve to null_sha1?

Now we pass RESOLVE_REF_NO_RECURSE and the correct information is
returned for a symref even when it points to a missing ref.  Simplify
by relying on RESOLVE_REF_READING.

No functional change intended.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 builtin/branch.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 2ad2d0b..0c7aac0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -236,10 +236,11 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
free(name);
 
name = mkpathdup(fmt, bname.buf);
-   target = resolve_ref_unsafe(name, RESOLVE_REF_NO_RECURSE,
+   target = resolve_ref_unsafe(name,
+   RESOLVE_REF_READING
+   | RESOLVE_REF_NO_RECURSE,
sha1, &flags);
-   if (!target ||
-   (!(flags & REF_ISSYMREF) && is_null_sha1(sha1))) {
+   if (!target) {
error(remote_branch
  ? _("remote branch '%s' not found.")
  : _("branch '%s' not found."), bname.buf);
-- 
2.1.0.rc2.206.gedb03e5

--
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 19/25] test: put tests for handling of bad ref names in one place

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 25 Sep 2014 15:02:39 -0700

There's no straightforward way to grep for all tests dealing with
invalid refs.  Put them in a single test script so it is easy to see
what functionality has not been exercised with bad ref names yet.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 t/t1400-update-ref.sh   | 44 --
 t/t1430-bad-ref-name.sh | 84 +
 t/t9300-fast-import.sh  | 30 --
 3 files changed, 84 insertions(+), 74 deletions(-)
 create mode 100755 t/t1430-bad-ref-name.sh

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7c8c41a..7b4707b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -136,14 +136,6 @@ test_expect_success 'update-ref --no-deref -d can delete 
reference to bad ref' '
test_path_is_missing .git/refs/heads/ref-to-bad
 '
 
-test_expect_success 'update-ref --no-deref -d can delete reference to broken 
name' '
-   git symbolic-ref refs/heads/badname refs/heads/broken...ref &&
-   test_when_finished "rm -f .git/refs/heads/badname" &&
-   test_path_is_file .git/refs/heads/badname &&
-   git update-ref --no-deref -d refs/heads/badname &&
-   test_path_is_missing .git/refs/heads/badname
-'
-
 test_expect_success '(not) create HEAD with old sha1' "
test_must_fail git update-ref HEAD $A $B
 "
@@ -408,12 +400,6 @@ test_expect_success 'stdin fails create with no ref' '
grep "fatal: create: missing " err
 '
 
-test_expect_success 'stdin fails create with bad ref name' '
-   echo "create ~a $m" >stdin &&
-   test_must_fail git update-ref --stdin err &&
-   grep "fatal: invalid ref format: ~a" err
-'
-
 test_expect_success 'stdin fails create with no new value' '
echo "create $a" >stdin &&
test_must_fail git update-ref --stdin err &&
@@ -432,12 +418,6 @@ test_expect_success 'stdin fails update with no ref' '
grep "fatal: update: missing " err
 '
 
-test_expect_success 'stdin fails update with bad ref name' '
-   echo "update ~a $m" >stdin &&
-   test_must_fail git update-ref --stdin err &&
-   grep "fatal: invalid ref format: ~a" err
-'
-
 test_expect_success 'stdin fails update with no new value' '
echo "update $a" >stdin &&
test_must_fail git update-ref --stdin err &&
@@ -456,12 +436,6 @@ test_expect_success 'stdin fails delete with no ref' '
grep "fatal: delete: missing " err
 '
 
-test_expect_success 'stdin fails delete with bad ref name' '
-   echo "delete ~a $m" >stdin &&
-   test_must_fail git update-ref --stdin err &&
-   grep "fatal: invalid ref format: ~a" err
-'
-
 test_expect_success 'stdin fails delete with too many arguments' '
echo "delete $a $m $m" >stdin &&
test_must_fail git update-ref --stdin err &&
@@ -734,12 +708,6 @@ test_expect_success 'stdin -z fails create with no ref' '
grep "fatal: create: missing " err
 '
 
-test_expect_success 'stdin -z fails create with bad ref name' '
-   printf $F "create ~a " "$m" >stdin &&
-   test_must_fail git update-ref -z --stdin err &&
-   grep "fatal: invalid ref format: ~a " err
-'
-
 test_expect_success 'stdin -z fails create with no new value' '
printf $F "create $a" >stdin &&
test_must_fail git update-ref -z --stdin err &&
@@ -764,12 +732,6 @@ test_expect_success 'stdin -z fails update with too few 
args' '
grep "fatal: update $a: unexpected end of input when reading 
" err
 '
 
-test_expect_success 'stdin -z fails update with bad ref name' '
-   printf $F "update ~a" "$m" "" >stdin &&
-   test_must_fail git update-ref -z --stdin err &&
-   grep "fatal: invalid ref format: ~a" err
-'
-
 test_expect_success 'stdin -z emits warning with empty new value' '
git update-ref $a $m &&
printf $F "update $a" "" "" >stdin &&
@@ -802,12 +764,6 @@ test_expect_success 'stdin -z fails delete with no ref' '
grep "fatal: delete: missing " err
 '
 
-test_expect_success 'stdin -z fails delete with bad ref name' '
-   printf $F "delete ~a" "$m" >stdin &&
-   test_must_fail git update-ref -z --stdin err &&
-   grep "fatal: invalid ref format: ~a" err
-'
-
 test_expect_success 'stdin -z fails delete with no old value' '
printf $F "delete $a" >stdin &&
test_must_fail git update-ref -z --stdin err &&
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
new file mode 100755
index 000..c7b19b0
--- /dev/null
+++ b/t/t1430-bad-ref-name.sh
@@ -0,0 +1,84 @@
+#!/bin/sh
+
+test_description='Test handling of ref names that check-ref-format rejects'
+. ./test-lib.sh
+
+test_expect_success setup '
+   test_commit one
+'
+
+test_expect_success 'fast-import: fail on invalid branch name 
".badbranchname"' '
+   test_when_finished "rm -f .git/objects/pack_* .git/objects/index_*" &&
+   cat >input <<-INPUT_END &&
+   commit .badbranch

[PATCH 20/25] refs.c: allow listing and deleting badly named refs

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Wed, 3 Sep 2014 11:45:43 -0700

We currently do not handle badly named refs well:

  $ cp .git/refs/heads/master .git/refs/heads/master.@\*@\\.
  $ git branch
fatal: Reference has invalid format: 'refs/heads/master.@*@\.'
  $ git branch -D master.@\*@\\.
error: branch 'master.@*@\.' not found.

Users cannot recover from a badly named ref without manually finding
and deleting the loose ref file or appropriate line in packed-refs.
Making that easier will make it easier to tweak the ref naming rules
in the future, for example to forbid shell metacharacters like '`'
and '"', without putting people in a state that is hard to get out of.

So allow "branch --list" to show these refs and allow "branch -d/-D"
and "update-ref -d" to delete them.  Other commands (for example to
rename refs) will continue to not handle these refs but can be changed
in later patches.

Details:

In resolving functions, refuse to resolve refs that don't pass the
git-check-ref-format(1) check unless the new RESOLVE_REF_ALLOW_BAD_NAME
flag is passed.  Even with RESOLVE_REF_ALLOW_BAD_NAME, refuse to
resolve refs that escape the refs/ directory and do not match the
pattern [A-Z_]* (think "HEAD" and "MERGE_HEAD").

In locking functions, refuse to act on badly named refs unless they
are being deleted and either are in the refs/ directory or match [A-Z_]*.

Just like other invalid refs, flag resolved, badly named refs with the
REF_ISBROKEN flag, treat them as resolving to null_sha1, and skip them
in all iteration functions except for for_each_rawref.

Flag badly named refs (but not symrefs pointing to badly named refs)
with a REF_BAD_NAME flag to make it easier for future callers to
notice and handle them specially.  For example, in a later patch
for-each-ref will use this flag to detect refs whose names can confuse
callers parsing for-each-ref output.

In the transaction API, refuse to create or update badly named refs,
but allow deleting them (unless they try to escape refs/ and don't match
[A-Z_]*).

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 builtin/branch.c|   9 +--
 cache.h |  17 +-
 refs.c  | 148 ++--
 refs.h  |  12 +++-
 t/t1430-bad-ref-name.sh | 125 +++-
 5 files changed, 273 insertions(+), 38 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0c7aac0..7e113d6 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
name = mkpathdup(fmt, bname.buf);
target = resolve_ref_unsafe(name,
RESOLVE_REF_READING
-   | RESOLVE_REF_NO_RECURSE,
+   | RESOLVE_REF_NO_RECURSE
+   | RESOLVE_REF_ALLOW_BAD_NAME,
sha1, &flags);
if (!target) {
error(remote_branch
@@ -248,7 +249,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
continue;
}
 
-   if (!(flags & REF_ISSYMREF) &&
+   if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
force)) {
ret = 1;
@@ -268,8 +269,8 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
   ? _("Deleted remote branch %s (was %s).\n")
   : _("Deleted branch %s (was %s).\n"),
   bname.buf,
-  (flags & REF_ISSYMREF)
-  ? target
+  (flags & REF_ISBROKEN) ? "broken"
+  : (flags & REF_ISSYMREF) ? target
   : find_unique_abbrev(sha1, DEFAULT_ABBREV));
}
delete_branch_config(bname.buf);
diff --git a/cache.h b/cache.h
index b735f3f..0501f7d 100644
--- a/cache.h
+++ b/cache.h
@@ -981,16 +981,29 @@ extern int read_ref(const char *refname, unsigned char 
*sha1);
  * If flags is non-NULL, set the value that it points to the
  * combination of REF_ISPACKED (if the reference was found among the
  * packed references), REF_ISSYMREF (if the initial reference was a
- * symbolic reference) and REF_ISBROKEN (if the ref is malformed).
+ * symbolic reference), REF_BAD_NAME (if the reference name is ill
+ * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN
+ * (if the ref is malformed or has a bad name). See refs.h for more detail
+ * on each flag.
  *
  * If ref is not a properly-formatted, normalized reference, return
 

[PATCH 22/25] remote rm/prune: print a message when writing packed-refs fails

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Thu, 11 Sep 2014 08:42:57 -0700

Until v2.1.0-rc0~22^2~11 (refs.c: add an err argument to
repack_without_refs, 2014-06-20), repack_without_refs forgot to
provide an error message when commit_packed_refs fails.  Even today,
it only provides a message for callers that pass a non-NULL err
parameter.  Internal callers in refs.c pass non-NULL err but
"git remote" does not.

That means that "git remote rm" and "git remote prune" can fail
without printing a message about why.  Fix them by passing in a
non-NULL err parameter and printing the returned message.

This is the last caller to a ref handling function passing err ==
NULL.  A later patch can drop support for err == NULL, avoiding such
problems in the future.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
Reviewed-by: Michael Haggerty 
---
 builtin/remote.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 42a533a..7f28f92 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -749,13 +749,16 @@ static int mv(int argc, const char **argv)
 
 static int remove_branches(struct string_list *branches)
 {
+   struct strbuf err = STRBUF_INIT;
const char **branch_names;
int i, result = 0;
 
branch_names = xmalloc(branches->nr * sizeof(*branch_names));
for (i = 0; i < branches->nr; i++)
branch_names[i] = branches->items[i].string;
-   result |= repack_without_refs(branch_names, branches->nr, NULL);
+   if (repack_without_refs(branch_names, branches->nr, &err))
+   result |= error("%s", err.buf);
+   strbuf_release(&err);
free(branch_names);
 
for (i = 0; i < branches->nr; i++) {
@@ -1332,9 +1335,13 @@ static int prune_remote(const char *remote, int dry_run)
delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
for (i = 0; i < states.stale.nr; i++)
delete_refs[i] = states.stale.items[i].util;
-   if (!dry_run)
-   result |= repack_without_refs(delete_refs,
- states.stale.nr, NULL);
+   if (!dry_run) {
+   struct strbuf err = STRBUF_INIT;
+   if (repack_without_refs(delete_refs, states.stale.nr,
+   &err))
+   result |= error("%s", err.buf);
+   strbuf_release(&err);
+   }
free(delete_refs);
}
 
-- 
2.1.0.rc2.206.gedb03e5

--
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 23/25] refs.c: do not permit err == NULL

2014-10-14 Thread Jonathan Nieder
Date: Thu, 28 Aug 2014 16:42:37 -0700

Some functions that take a strbuf argument to append an error treat
!err as an indication that the message should be suppressed (e.g.,
ref_update_reject_duplicates).  Others write the message to stderr on
!err (e.g., repack_without_refs).  Others crash (e.g.,
ref_transaction_update).

Some of these behaviors are for historical reasons and others were
accidents.  Luckily no callers pass err == NULL any more.  Simplify
by consistently requiring the strbuf argument.

Signed-off-by: Jonathan Nieder 
Reviewed-by: Ronnie Sahlberg 
---
 refs.c | 46 +++---
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/refs.c b/refs.c
index e7000f2..097fb4b 100644
--- a/refs.c
+++ b/refs.c
@@ -2646,6 +2646,8 @@ int repack_without_refs(const char **refnames, int n, 
struct strbuf *err)
struct string_list_item *ref_to_delete;
int i, ret, removed = 0;
 
+   assert(err);
+
/* Look for a packed ref */
for (i = 0; i < n; i++)
if (get_packed_ref(refnames[i]))
@@ -2656,13 +2658,8 @@ int repack_without_refs(const char **refnames, int n, 
struct strbuf *err)
return 0; /* no refname exists in packed refs */
 
if (lock_packed_refs(0)) {
-   if (err) {
-   unable_to_lock_message(git_path("packed-refs"), errno,
-  err);
-   return -1;
-   }
-   unable_to_lock_error(git_path("packed-refs"), errno);
-   return error("cannot delete '%s' from packed refs", 
refnames[i]);
+   unable_to_lock_message(git_path("packed-refs"), errno, err);
+   return -1;
}
packed = get_packed_refs(&ref_cache);
 
@@ -2688,7 +2685,7 @@ int repack_without_refs(const char **refnames, int n, 
struct strbuf *err)
 
/* Write what remains */
ret = commit_packed_refs();
-   if (ret && err)
+   if (ret)
strbuf_addf(err, "unable to overwrite old ref-pack file: %s",
strerror(errno));
return ret;
@@ -2696,6 +2693,8 @@ int repack_without_refs(const char **refnames, int n, 
struct strbuf *err)
 
 static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf 
*err)
 {
+   assert(err);
+
if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
/*
 * loose.  The loose file name is the same as the
@@ -3551,6 +3550,8 @@ struct ref_transaction {
 
 struct ref_transaction *ref_transaction_begin(struct strbuf *err)
 {
+   assert(err);
+
return xcalloc(1, sizeof(struct ref_transaction));
 }
 
@@ -3590,6 +3591,8 @@ int ref_transaction_update(struct ref_transaction 
*transaction,
 {
struct ref_update *update;
 
+   assert(err);
+
if (transaction->state != REF_TRANSACTION_OPEN)
die("BUG: update called for transaction that is not open");
 
@@ -3622,6 +3625,8 @@ int ref_transaction_create(struct ref_transaction 
*transaction,
 {
struct ref_update *update;
 
+   assert(err);
+
if (transaction->state != REF_TRANSACTION_OPEN)
die("BUG: create called for transaction that is not open");
 
@@ -3653,6 +3658,8 @@ int ref_transaction_delete(struct ref_transaction 
*transaction,
 {
struct ref_update *update;
 
+   assert(err);
+
if (transaction->state != REF_TRANSACTION_OPEN)
die("BUG: delete called for transaction that is not open");
 
@@ -3715,13 +3722,14 @@ static int ref_update_reject_duplicates(struct 
ref_update **updates, int n,
struct strbuf *err)
 {
int i;
+
+   assert(err);
+
for (i = 1; i < n; i++)
if (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {
-   const char *str =
-   "Multiple updates for ref '%s' not allowed.";
-   if (err)
-   strbuf_addf(err, str, updates[i]->refname);
-
+   strbuf_addf(err,
+   "Multiple updates for ref '%s' not 
allowed.",
+   updates[i]->refname);
return 1;
}
return 0;
@@ -3735,6 +3743,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
int n = transaction->nr;
struct ref_update **updates = transaction->updates;
 
+   assert(err);
+
if (transaction->state != REF_TRANSACTION_OPEN)
die("BUG: commit called for transaction that is not open");
 
@@ -3771,9 +3781,8 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
ret = (errno == ENOTDIR)
? TRANSACTION_NAME_CONFLICT
: TRANSACTION_GENERIC_ERROR;
-   

[PATCH 21/25] for-each-ref: skip and warn about broken ref names

2014-10-14 Thread Jonathan Nieder
From: Ronnie Sahlberg 
Date: Fri, 5 Sep 2014 14:35:17 -0700

Print a warning message for any bad ref names we find in the repo and
skip them so callers don't have to deal with parsing them.

It might be useful in the future to have a flag where we would not
skip these refs for those callers that want to and are prepared (for
example by using a --format argument with %0 as a delimiter after the
ref name).

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Jonathan Nieder 
---
 builtin/for-each-ref.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 492265d..3ee22b9 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -839,6 +839,11 @@ static int grab_single_ref(const char *refname, const 
unsigned char *sha1, int f
struct refinfo *ref;
int cnt;
 
+   if (flag & REF_BAD_NAME) {
+ warning("ignoring ref with broken name %s", refname);
+ return 0;
+   }
+
if (*cb->grab_pattern) {
const char **pattern;
int namelen = strlen(refname);
-- 
2.1.0.rc2.206.gedb03e5

--
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 25/25] ref_transaction_commit: bail out on failure to remove a ref

2014-10-14 Thread Jonathan Nieder
Date: Thu, 28 Aug 2014 17:01:35 -0700

When removal of a loose or packed ref fails, bail out instead of
trying to finish the transaction.  This way, a single error message
can be printed (instead of multiple messages being concatenated by
mistake) and the operator can try to solve the underlying problem
before there is a chance to muck things up even more.

In particular, when git fails to remove a ref, git goes on to try to
delete the reflog.  Exiting early lets us keep the reflog.

When git succeeds in deleting a ref A and fails to remove a ref B, it
goes on to try to delete both reflogs.  It would be better to just
remove the reflog for A, but that would be a more invasive change.
Failing early means we keep both reflogs, which puts the operator in a
good position to understand the problem and recover.

A long term goal is to avoid these problems altogether and roll back
the transaction on failure.  That kind of transactionality will have
to wait for a later series (the plan for which is to make all
destructive work happen in a single update of the packed-refs file).

Signed-off-by: Jonathan Nieder 
Reviewed-by: Ronnie Sahlberg 
---
Thanks for reading.

 refs.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 097fb4b..0368ed4 100644
--- a/refs.c
+++ b/refs.c
@@ -3809,16 +3809,20 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
struct ref_update *update = updates[i];
 
if (update->lock) {
-   if (delete_ref_loose(update->lock, update->type, err))
+   if (delete_ref_loose(update->lock, update->type, err)) {
ret = TRANSACTION_GENERIC_ERROR;
+   goto cleanup;
+   }
 
if (!(update->flags & REF_ISPRUNING))
delnames[delnum++] = update->lock->ref_name;
}
}
 
-   if (repack_without_refs(delnames, delnum, err))
+   if (repack_without_refs(delnames, delnum, err)) {
ret = TRANSACTION_GENERIC_ERROR;
+   goto cleanup;
+   }
for (i = 0; i < delnum; i++)
unlink_or_warn(git_path("logs/%s", delnames[i]));
clear_loose_ref_cache(&ref_cache);
-- 
2.1.0.rc2.206.gedb03e5

--
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 24/25] lockfile: remove unable_to_lock_error

2014-10-14 Thread Jonathan Nieder
Date: Thu, 28 Aug 2014 16:41:34 -0700

The former caller uses unable_to_lock_message now.

Signed-off-by: Jonathan Nieder 
Reviewed-by: Ronnie Sahlberg 
---
 lockfile.c | 10 --
 lockfile.h |  1 -
 2 files changed, 11 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index d098ade..4f16ee7 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -162,16 +162,6 @@ void unable_to_lock_message(const char *path, int err, 
struct strbuf *buf)
absolute_path(path), strerror(err));
 }
 
-int unable_to_lock_error(const char *path, int err)
-{
-   struct strbuf buf = STRBUF_INIT;
-
-   unable_to_lock_message(path, err, &buf);
-   error("%s", buf.buf);
-   strbuf_release(&buf);
-   return -1;
-}
-
 NORETURN void unable_to_lock_die(const char *path, int err)
 {
struct strbuf buf = STRBUF_INIT;
diff --git a/lockfile.h b/lockfile.h
index dc066d1..cd2ec95 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -71,7 +71,6 @@ struct lock_file {
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NO_DEREF 2
 
-extern int unable_to_lock_error(const char *path, int err);
 extern void unable_to_lock_message(const char *path, int err,
   struct strbuf *buf);
 extern NORETURN void unable_to_lock_die(const char *path, int err);
-- 
2.1.0.rc2.206.gedb03e5

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


meld output check is broken

2014-10-14 Thread Andrey Novoseltsev
Hello,

It seems that support of "--output" by meld is performed by grepping
its help output. However with meld 3.12 I get

$ meld --help
Usage:
  meld [OPTION...]

Help Options:
  -h, --help Show help options
  --help-all Show all help options
  --help-gtk Show GTK+ Options
  --help-gapplicationShow GApplication options

Application Options:
  --display=DISPLAY  X display to use

Yet it still supports --output and can show nice middle instead of
<< HEAD etc. markers.

Thank you,
Andrey
--
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] Allow building Git with Asciidoctor

2014-10-14 Thread brian m. carlson
On Tue, Oct 14, 2014 at 10:08:19AM -0700, Junio C Hamano wrote:
> Jeff King  writes:
> 
> > On Sat, Oct 11, 2014 at 11:37:32PM +, brian m. carlson wrote:
> >
> > Specifically I'm not excited about getting into a state where we have to
> > maintain both an asciidoc.conf file _and_ ruby extensions for
> > asciidoctor. I don't mind if somebody wants to step up and keep the
> > asciidoctor bits in sync with the asciidoc.conf, but I feel like one of
> > them needs to be considered the "master".
> 
> My so-far-unstated inclination, since seeing the patch to fix the
> unbalanced example block separators from Brian (which was outside
> and before this four-patch series), has been to keep our Makefile in
> Documentation/ aware only of AsciiDoc while maintaining *.txt files
> in a state so that AsciiDoctor could also be used to process them,
> if people want to futz with their copies of Documentation/Makefile.
> 
> I do not mind to have the machinery to run AsciiDoctor too much in
> my tree.  It may make it easier for those who use it to spot places
> in *.txt that need (in)compatibility workarounds between the two
> formatters than keeping it outside.

I'd be happy if you simply picked up patch 3 and left out patch 4.  It
gets us most of the way there, which is good enough for most things.
It's even possible to handle the litdd attribute on the command line, so
the only thing we'd really lose is the linkgit links.

Alternately, I'm happy to be responsible for maintaining the
extensions.rb file.  The asciidoc.conf file has not had a substantive
(non-comment) change since 2012, and it has not had a change that would
require an update to the extensions since 2010.  I don't anticipate that
keeping it up-to-date will require a significant amount of work.  We can
even drop it into contrib if you think that's a better place.

It's really up to you which you'd prefer.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v4 0/1] Use absolute paths of lockfiles

2014-10-14 Thread Yue Lin Ho
Hi Michael:

Good Morning. :)

I see mh/lockfile and mh/lockfile-stdio were ​graduated to "master".
So. does this patch continue?
On top of mh/lockfile-stdio?

Thank you. ^_^

Yue Lin Ho



--
View this message in context: 
http://git.661346.n2.nabble.com/What-s-cooking-in-git-git-Aug-2014-02-Fri-8-tp7616651p7619900.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-prompt.sh: Option to hide prompt for ignored pwd

2014-10-14 Thread Jess Austin
Optionally set __git_ps1 to display nothing when present working
directory is ignored, triggered by the new environmental variable
GIT_PS1_HIDE_IF_PWD_IGNORED. This environmental variable may be
overridden on any repository by setting bash.hideIfPwdIgnored to
"false". In the absence of GIT_PS1_HIDE_IF_PWD_IGNORED this change
has no effect.

Many people manage e.g. dotfiles in their home directory with git.
This causes the prompt generated by __git_ps1 to refer to that "top
level" repo while working in any descendant directory. That can be
distracting, so this patch helps one shut off that noise.

Signed-off-by: Jess Austin 
---
On Tue, Oct 14, 2014 at 2:08 PM, Richard Hansen  wrote:
> On 2014-10-14 14:47, Johannes Sixt wrote:
>> Ahem, no. Please do not punish users who are not interested in the new
>> feature with two new processes every time __git_ps() is run. Think of
>> Windows where fork() is really, *really* expensive.
> Regardless, it would be nice if the behavior matched the other bash.*
> variables (only check the bash.* variable if the corresponding
> environment variable is set, and default to true).  The following should
> fix it:
>
> if [ -n "${GIT_PS1_HIDE_ON_IGNORED_PWD}" ] &&
>[ "$(git config --bool bash.hideOnIgnoredPwd)" != "false" ] &&
>[ "$(git check-ignore .)" ]
> then
Thanks for helping me understand this! I think I have it correct now.


On Tue, Oct 14, 2014 at 2:21 PM, Richard Hansen  wrote:
> I do prefer the new names.  They are long, but how often will someone
> have to type it?  In this case it's better to be descriptive than to be
> short.  (I wonder if adding two letters would improve readability
> further:  GIT_PS1_HIDE_WHEN_PWD_IGNORED and bash.hideWhenPwdIgnored.)

We got those two letters back with GIT_PS1_HIDE_IF_PWD_IGNORED and
bash.hideIfPwdIgnored.

> To avoid scaring people who might not want this feature enabled, I
> recommend changing the subject line to something like this:
>
> git-prompt.sh: Option to hide prompt for ignored pwd

Good idea!


 contrib/completion/git-prompt.sh | 12 
 t/t9903-bash-prompt.sh   | 42 
 2 files changed, 54 insertions(+)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c5473dc..151218b 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -84,6 +84,11 @@
 # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
 # the colored output of "git status -sb" and are available only when
 # using __git_ps1 for PROMPT_COMMAND or precmd.
+#
+# If you would like __git_ps1 to do nothing in the case when the current
+# directory is set up to be ignored by git, then set
+# GIT_PS1_HIDE_IF_PWD_IGNORED to a nonempty value. Override this on the
+# repository level by setting bash.hideIfPwdIgnored to "false".
 
 # check whether printf supports -v
 __git_printf_supports_v=
@@ -501,6 +506,13 @@ __git_ps1 ()
local f="$w$i$s$u"
local gitstring="$c$b${f:+$z$f}$r$p"
 
+   if [ -n "${GIT_PS1_HIDE_IF_PWD_IGNORED}" ] &&
+  [ "$(git config --bool bash.hideIfPwdIgnored)" != "false" ] &&
+  git check-ignore -q .
+   then
+   printf_format=""
+   fi
+
if [ $pcmode = yes ]; then
if [ "${__git_printf_supports_v-}" != yes ]; then
gitstring=$(printf -- "$printf_format" "$gitstring")
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 9150984..88a75cf 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -35,6 +35,8 @@ test_expect_success 'setup for prompt tests' '
git commit -m "another b2" file &&
echo 000 >file &&
git commit -m "yet another b2" file &&
+   mkdir ignored_dir &&
+   echo "ignored_dir/" >> .gitignore &&
git checkout master
 '
 
@@ -588,4 +590,44 @@ test_expect_success 'prompt - zsh color pc mode' '
test_cmp expected "$actual"
 '
 
+test_expect_success 'prompt - hide if pwd ignored - shell variable unset with 
config disabled' '
+   printf " (master)" >expected &&
+   test_config bash.hideIfPwdIgnored false &&
+   (
+   cd ignored_dir &&
+   __git_ps1 >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - shell variable unset with 
config unset' '
+   printf " (master)" >expected &&
+   (
+   cd ignored_dir &&
+   __git_ps1 >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - hide if pwd ignored - shell variable set with 
config disabled' '
+   printf " (master)" >expected &&
+   test_config bash.hideIfPwdIgnored false &&
+   (
+   cd ignored_dir &&
+   GIT_PS1_HIDE_IF_PWD_IGNORED=y &&
+   __git_ps1 >"$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt -

Expected bug with reset --hard

2014-10-14 Thread Nicholas Chmielewski
git version 1.9.1 & version 2.1.2

I had some changes in a tracked file that I wanted to discard.
Instead of using checkout, I instead used `git reset --hard HEAD` to reset.

Git returned the message `HEAD is now at  `
I then went `git status` and it showed me that I still have changes to commit

I expected there to be no changes in my working directory as a result of
doing a `git reset --hard`

I did a little more fiddling around and still yielded the same results.

- `$ touch file.md`
- `$ git add file.md`
- `$ git commit -m 'empty file'`
- `$ echo 'text' > file.md`

At this point git reported that I had uncommited changes for both file.md and
the troublesome file in question (jquery.datatables.js)

`$ git reset --hard HEAD`

Git now no longer reports and untracked changes for file.md but still
for jquery.datatables.js

`$ git reset --hard HEAD~1`

Git still reports untracked changes for jquery.datatables.js

I have included a copy of the diff of the jquery.datatables.js
diff --git a/vendor/assets/javascripts/jquery.datatables.js 
b/vendor/assets/javascripts/jquery.datatables.js
index b9044f1..4aab04b 100644
--- a/vendor/assets/javascripts/jquery.datatables.js
+++ b/vendor/assets/javascripts/jquery.datatables.js
@@ -1,11 +1,11 @@
-/*! DataTables 1.10.1
- * ©2008-2014 SpryMedia Ltd - datatables.net/license
+/*! DataTables 1.10.3
+ * ©2008-2014 SpryMedia Ltd - datatables.net/license
  */
 
 /**
  * @summary DataTables
  * @description Paginate, search and order HTML tables
- * @version 1.10.1
+ * @version 1.10.3
  * @filejquery.dataTables.js
  * @author  SpryMedia Ltd (www.sprymedia.co.uk)
  * @contact www.sprymedia.co.uk/contact
@@ -113,7 +113,7 @@

// U+2009 is thin space and U+202F is narrow no-break space, both used 
in many
// standards as thousands separators
-   var _re_formatted_numeric = /[',$£€¥%\u2009\u202F]/g;
+   var _re_formatted_numeric = /[',$£€¥%\u2009\u202F]/g;


var _empty = function ( d ) {
@@ -133,7 +133,7 @@
if ( ! _re_dic[ decimalPoint ] ) {
_re_dic[ decimalPoint ] = new RegExp( _fnEscapeRegex( 
decimalPoint ), 'g' );
}
-   return typeof num === 'string' ?
+   return typeof num === 'string' && decimalPoint !== '.' ?
num.replace( /\./g, '' ).replace( _re_dic[ decimalPoint 
], '.' ) :
num;
};
@@ -310,7 +310,6 @@
newKey = key.replace( match[0], 
match[2].toLowerCase() );
map[ newKey ] = key;

-   //console.log( key, match );
if ( match[1] === 'o' )
{
_fnHungarianMap( o[key] );
@@ -673,6 +672,12 @@
return _fnSetObjectDataFn( mDataSrc )( rowData, val, 
meta );
};

+   // Indicate if DataTables should read DOM data as an object or 
array
+   // Used in _fnGetRowElements
+   if ( typeof mDataSrc !== 'number' ) {
+   oSettings._rowReadObject = true;
+   }
+   
/* Feature sorting overrides column specific when off */
if ( !oSettings.oFeatures.bSort )
{
@@ -1498,19 +1503,22 @@
function _fnGetRowElements( settings, row )
{
var
-   d = [],
tds = [],
td = row.firstChild,
name, col, o, i=0, contents,
-   columns = settings.aoColumns;
+   columns = settings.aoColumns,
+   objectRead = settings._rowReadObject;

-   var attr = function ( str, data, td  ) {
+   var d = objectRead ? {} : [];
+   
+   var attr = function ( str, td  ) {
if ( typeof str === 'string' ) {
var idx = str.indexOf('@');

if ( idx !== -1 ) {
-   var src = str.substring( idx+1 );
-   o[ '@'+src ] = td.getAttribute( src );
+   var attr = str.substring( idx+1 );
+   var setter = _fnSetObjectDataFn( str );
+   setter( d, td.getAttribute( attr ) );
}
}
};
@@ -1520,18 +1528,26 @@
contents = $.trim(cell.innerHTML);

if ( col && col._bAttrSrc ) {
-   o = {
-   display: contents
-   };
+   var setter = _fnSetOb

Re: Expected bug with reset --hard

2014-10-14 Thread Nicholas Chmielewski
I'd like to rescind my bug report.


A bit more further investigation revealed that git was detecting the
file with changes as jquery.datatables.js but the file in my directory
was reported as being named jquery.dataTables.js .

I'm currently working on OS X 10.9, and this issue is probably related
more to the case-preservation of the file system rather than git
itself.

On 15 October 2014 14:34, Nicholas Chmielewski
 wrote:
> git version 1.9.1 & version 2.1.2
>
> I had some changes in a tracked file that I wanted to discard.
> Instead of using checkout, I instead used `git reset --hard HEAD` to reset.
>
> Git returned the message `HEAD is now at  `
> I then went `git status` and it showed me that I still have changes to commit
>
> I expected there to be no changes in my working directory as a result of
> doing a `git reset --hard`
>
> I did a little more fiddling around and still yielded the same results.
>
> - `$ touch file.md`
> - `$ git add file.md`
> - `$ git commit -m 'empty file'`
> - `$ echo 'text' > file.md`
>
> At this point git reported that I had uncommited changes for both file.md and
> the troublesome file in question (jquery.datatables.js)
>
> `$ git reset --hard HEAD`
>
> Git now no longer reports and untracked changes for file.md but still
> for jquery.datatables.js
>
> `$ git reset --hard HEAD~1`
>
> Git still reports untracked changes for jquery.datatables.js
>
> I have included a copy of the diff of the jquery.datatables.js
--
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 2/3] mergetool: don't require a work tree for --tool-help

2014-10-14 Thread David Aguilar
On Mon, Oct 13, 2014 at 12:16:55PM -0700, Junio C Hamano wrote:
> David Aguilar  writes:
> 
> > From: Charles Bailey 
> >
> > Signed-off-by: Charles Bailey 
> > Signed-off-by: David Aguilar 
> > ---
> > Changes since v2:
> >
> > This now uses the new git_dir_init function.
> >
> >  git-mergetool.sh | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/git-mergetool.sh b/git-mergetool.sh
> > index 96a61ba..cddb533 100755
> > --- a/git-mergetool.sh
> > +++ b/git-mergetool.sh
> > @@ -10,11 +10,11 @@
> >  
> >  USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to 
> > merge] ...'
> >  SUBDIRECTORY_OK=Yes
> > +NONGIT_OK=Yes
> >  OPTIONS_SPEC=
> >  TOOL_MODE=merge
> >  . git-sh-setup
> >  . git-mergetool--lib
> > -require_work_tree
> >  
> >  # Returns true if the mode reflects a symlink
> >  is_symlink () {
> > @@ -378,6 +378,9 @@ prompt_after_failed_merge () {
> > done
> >  }
> >  
> > +require_work_tree
> > +git_dir_init
> 
> This is somewhat curious.  Shouldn't the order of these swapped?

Yes.  I'll send a replacement patch for 2/3 only.
-- 
David
--
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] mergetool: add an option for writing to a temporary directory

2014-10-14 Thread David Aguilar
On Mon, Oct 13, 2014 at 12:24:41PM -0700, Junio C Hamano wrote:
> David Aguilar  writes:
> 
> > Teach mergetool to write files in a temporary directory when
> > 'mergetool.writeToTemp' is true.
> >
> > This is helpful for tools such as Eclipse which cannot cope with
> > multiple copies of the same file in the worktree.
> 
> With this can we drop the "change the temporary file name" patch by
> Robin Rosenberg?
> 
> http://thread.gmane.org/gmane.comp.version-control.git/255457/focus=255599
> 
> Message-Id: <1408607240-11369-1-git-send-email-robin.rosenb...@dewire.com>

I would think so but I'm biased ;-)
-- 
David
--
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 2/3] mergetool: don't require a work tree for --tool-help

2014-10-14 Thread David Aguilar
On Tue, Oct 14, 2014 at 11:35:11PM -0700, David Aguilar wrote:
> On Mon, Oct 13, 2014 at 12:16:55PM -0700, Junio C Hamano wrote:
> > David Aguilar  writes:
> > 
> > > From: Charles Bailey 
> > >
> > > Signed-off-by: Charles Bailey 
> > > Signed-off-by: David Aguilar 
> > > ---
> > > Changes since v2:
> > >
> > > This now uses the new git_dir_init function.
> > >
> > >  git-mergetool.sh | 5 -
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/git-mergetool.sh b/git-mergetool.sh
> > > index 96a61ba..cddb533 100755
> > > --- a/git-mergetool.sh
> > > +++ b/git-mergetool.sh
> > > @@ -10,11 +10,11 @@
> > >  
> > >  USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to 
> > > merge] ...'
> > >  SUBDIRECTORY_OK=Yes
> > > +NONGIT_OK=Yes
> > >  OPTIONS_SPEC=
> > >  TOOL_MODE=merge
> > >  . git-sh-setup
> > >  . git-mergetool--lib
> > > -require_work_tree
> > >  
> > >  # Returns true if the mode reflects a symlink
> > >  is_symlink () {
> > > @@ -378,6 +378,9 @@ prompt_after_failed_merge () {
> > >   done
> > >  }
> > >  
> > > +require_work_tree
> > > +git_dir_init
> > 
> > This is somewhat curious.  Shouldn't the order of these swapped?
> 
> Yes.  I'll send a replacement patch for 2/3 only.

Nevermind, I noticed you already fixed this up in pu.

Thank you,
-- 
David
--
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