[PATCH v4 2/2] test/send-email: to-cover, cc-cover tests

2014-04-29 Thread Michael S. Tsirkin
Add tests for the new feature.

Signed-off-by: Michael S. Tsirkin 
---
 t/t9001-send-email.sh | 45 +
 1 file changed, 45 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1ecdacb..97cc094 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1334,6 +1334,51 @@ test_expect_success $PREREQ '--force sends cover letter 
template anyway' '
test -n "$(ls msgtxt*)"
 '
 
+test_cover_addresses () {
+   header="$1"
+   shift
+   clean_fake_sendmail &&
+   rm -fr outdir &&
+   git format-patch --cover-letter -2 -o outdir &&
+   cover=`echo outdir/-*.patch` &&
+   mv $cover cover-to-edit.patch &&
+   sed "s/^From:/$header: ex...@address.com\nFrom:/" cover-to-edit.patch > 
$cover &&
+   git send-email \
+ --force \
+ --from="Example " \
+ --no-to --no-cc \
+ "$@" \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ outdir/-*.patch \
+ outdir/0001-*.patch \
+ outdir/0002-*.patch \
+ 2>errors >out &&
+   grep "^$header: ex...@address.com" msgtxt1 > to1 &&
+   grep "^$header: ex...@address.com" msgtxt2 > to2 &&
+   grep "^$header: ex...@address.com" msgtxt3 > to3 &&
+   test_line_count = 1 to1 &&
+   test_line_count = 1 to2 &&
+   test_line_count = 1 to3
+}
+
+test_expect_success $PREREQ 'to-cover adds To to all mail' '
+   test_cover_addresses "To" --to-cover
+'
+
+test_expect_success $PREREQ 'cc-cover adds Cc to all mail' '
+   test_cover_addresses "Cc" --cc-cover
+'
+
+test_expect_success $PREREQ 'tocover adds To to all mail' '
+   test_config sendemail.tocover true &&
+   test_cover_addresses "To"
+'
+
+test_expect_success $PREREQ 'cccover adds Cc to all mail' '
+   test_config sendemail.cccover true &&
+   test_cover_addresses "Cc"
+'
+
 test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' '
clean_fake_sendmail &&
echo "alias sbd  someb...@example.org" >.mailrc &&
-- 
MST

--
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: Tagging a branch as "not fitted for branching" ?

2014-04-29 Thread Jean-Noël Avila

Le 29/04/2014 01:34, Junio C Hamano a écrit :

Jean-Noël Avila  writes:


Most manuals on git state that it is bad practice to push -f a branch
after have meddled with its history, because this would risk to upset
the repositories of the coworkers. On the other hand, some workflows
use branches to propose modifications, and need some rewritting of the
history after some review steps. In this case, the branch should only
be seen as a mere pile of patches. Having this two-sided discourse is
often misunderstood by casual git users.

The proposed solution would be to be able to flag the branches with a
special tag "not fitted for branching" that a collaborator could use
when pushing it. This tag would be passed on to any pulled
remote. When another collaborator would then issue a "git checkout
-b", the command would fail with a warning message, unless forced with
-f'.

Is this feature already present? If not, would it be of any interest?

Since nobody seems to be responding...

I do not think there is anything like that.  I am not personally
interested in it very much without seeing much details on how we go
about implementing such a feature.  Note that I am not speaking on
behalf of the project, or as its maintainer, but just as one of the
active contributors to the project, so my "not interested" is in no
way final.

There are ways other than "checkout -b" to end up having your
commits on top of a forbidden commit.  Are you going to cover all of
them and at what point?  You may rebase your work based on 'master'
(which is not one of these forbidden branches) onto it.  You may
start your WIP on top of 'master', realize that you need something
that is cooking only in 'pu' (which is a forbidden-to-be-built-on
branch), and then do a "git checkout -m pu^0" in order to further
experiment with it in an ideal world in which that prerequiste of
yours already has graduated, and then decide to keep the WIP on a
branch that you are not going to publish with "git branch wip" after
commiting it on a detached HEAD.  Requiring "-f" during such an
exploratory, idea-forming programming exercise might be a bit too
much, and I cannot offhand see where the good place is to require
"-f" in the first place.  At the final "git branch wip" step is too
late, as you have already expended a lot of effort to build that
WIP, and your saying "git branch wip" should be an enough clue for
Git that you do mean to save it.

At the first glance, I do agree (and to only this part I can say "I
am interested in") that it might be a good idea if we had a bit more
formal way to convey that branch 'pu' is not something you may want
to base your final work on, but I am not sure if such a tag would
help very much in practice or would be seen as a mere unnecessary
roadblock that prevents them from freer experiments once the
developers get used to the convention of their projects.


Thank you for your reply.

I had not looked at other scenarios and the use cases that you bring up 
show that this feature would be far more complex than first estimated. I 
was more focused on the simplest case that is the broadest in the github 
fashion. Basing a work on a remote 'pu' branch when using advanced 
branch management commands may not raise any warning, or these warnings 
could be muffled with a config property.


After thinking a bit more on this, the initial idea encompassed another 
feature: enable people to push without "-f" when they have created these 
kind of branches. In your daily management of the pu branch for git, do 
you have to use the -f flag a lot? Would it be helpful to just tell git 
"I know what I am doing on this branch"? In this feature, the tagging 
would only be local.


--
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] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-04-29 Thread Torsten Bögershausen

On 04/29/2014 05:23 AM, Jeff King wrote:

On Mon, Apr 28, 2014 at 10:49:30PM +0200, Torsten Bögershausen wrote:


OK, thanks for the description.
In theory we can make Git "composition ignoring" by changing
index_file_exists() in name-hash.c.
(Both names must be precomposed first and compared then)

Yeah, we could perhaps get away without storing the extra precomposed
form if we just stored the entries under their precomposed hash, and
then taught same_name to use a slower precompose-aware comparison. But I
also see that we do binary searches in index_name_pos (called by
index_name_is_other). I don't know if we'd have to deal with this
problem there, too.

Just loud thinking:
We precompose whenever we read file names from disc, that's done in 
readdir()
We precompose whenever we get an argv into Git, that's done in 
precompose_argv()
We precompose every time we read file names from the index file on 
disc(?) into memory.
That we don't do today, and my suggestion to hack name-hash.c isn't a 
good one.


Probably we need to go into read-cache.c, or more places. I'm not an 
expert here knowing

all Git internal details.
Basically all places where strings containing file names are put into 
memory are effected,

and I wouldn't be too concerned about CPU cycles.


I don't know how much people are using Git before 1.7.12 (the
first version supporting precomposed unicode).

Could we simply ask them to upgrade ?

I'm not sure what you mean here. Upgrading won't help, because the
values are baked into existing history created with the old versions
forever. Any time I "git checkout v1.0" on the broken project, a modern
git will complain about the ghost untracked file.

It depends if all file names in a certain repo are stored decomposed,
(in this case everybody can set core.precomposeunicode false)
or if there is a mixture having precomposed and decomposed
in different comits/directories...
In this case we can normalize the master branch.
For older commit the users need to wait for an updated Git version,
until that they need to live with the ghosts as good as they can.




The next problem is that people need to agree if the repo should store
names in pre- or decomposed form.
(My voice is for precomposed)
Unfortunatly the core.precomposeunicode is repo-local, so everybody
needs to "agree globally" and "configure locally".

Yeah, that was sort of my "point 1" from the patch. I'm a bit worried
that it creates problems for people on other systems, though. Linux
people do not need to care about precomposed/decomposed at all, and any
attempt we make to automatically handle it is going to run afoul of
non-utf8 encodings. Not to mention that it does not solve the "git
checkout v1.0" problem above.

Not sure what is meant by non-utf8 encodings.
Mac OS X can only handle Unicode filenames,
and a single ISO-8859-1 will be returned as "%XY" from readdir().
So if you want to share a repo with Mac OS X (and/or Windows)
Unicode should be used.
Are you saying that there is a Linux station feeding in file names in  
e.g. 8859-1, EUC ?

My experience/knowledge is that you can not do that (in a useful way).



Side note:
I which we had this config variable travelling with the repo, like 
.gitattributes does
for text dealing with CRLF-LF.

Yeah, I guess if we wanted to enforce it everywhere, you would have to
use .gitattributes since we cannot safely turn on such a feature by
default.


I don't know how many reports you have, reading all this it feels as if the 
effected users
could "normalize" their repos and run "git config core.precomposeunicode true", 
followed
by "git config --global core.precomposeunicode true".
Does that sound like a possible way forward ?

I have just a handful of reports. Maybe 3-4? I didn't dig them all up
today, as it would be a non-trivial effort. But I have no idea how good
a sampling that is.

The following could help, may be:
git -c core.quotepath=false ls-files | iconv -f UTF-8-MAC -t UTF-8 >expected
git -c core.quotepath=false ls-files >actual
diff expected actual


-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 10/12] MINGW: config.mak.uname: drop USE_NED_ALLOCATOR

2014-04-29 Thread Marat Radchenko
On Mon, Apr 28, 2014 at 05:23:25PM +0200, Erik Faye-Lund wrote:
> On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko  
> wrote:
> > nedalloc was initially added in f0ed82 to fix slowness of standard WinXP
> > memory allocator. Since WinXP is EOLed, this point is no longer valid.
> >
> > The actual reason behind this commit is incompatibility of malloc.c.h
> > with MinGW-W64 headers. Alternative solution implies updating nedalloc
> > to something newer.
> 
> Did you measure that malloc on newer Windows-versions are actually
> faster? AFAIK, malloc does a lot more inside the CRT than in the
> kernel...

Windows 8, msysGit.

git repack -adf on msysgit/git (best of 3 runs)

+ nedalloc: 10.5s
- nedalloc: 11s

git repack -adf on torvalds/linux (best of 3 runs)

+ nedalloc: 3m 24s
- nedalloc: 3m 47s

We need to make a decision: drop nedalloc, update nedalloc to later release,
patch nedalloc to make it work under MinGW-W64 or disable nedalloc under
MinGW-W64 (still leaving it enabled under MinGW).

P.S. Waiting for "Resolving deltas" when cloning torvalds/linux is a pain,
perhaps someone should run gprof on it.
--
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: Reference to a commit inside a commit message

2014-04-29 Thread Felipe Contreras
Jeff King wrote:

> [1] I do not know about others, but I typically cut and paste from
> another terminal, and use the following alias in my config:
> 
>   [alias]
>   ll = "!git --no-pager log -1 --pretty='tformat:%h (%s, %ad)' 
> --date=short"

I have:

  [alias]
  short = show --quiet --format='%C(auto)%h (%s)%C(reset)'

-- 
Felipe Contreras
--
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 10/12] MINGW: config.mak.uname: drop USE_NED_ALLOCATOR

2014-04-29 Thread Felipe Contreras
Marat Radchenko wrote:

> We need to make a decision: drop nedalloc, update nedalloc to later release,
> patch nedalloc to make it work under MinGW-W64 or disable nedalloc under
> MinGW-W64 (still leaving it enabled under MinGW).

I say go for the latter (disable for mingw-264). It can be fixed later, and in
the meantime nobody gets affected negatively.

-- 
Felipe Contreras
--
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: Recording the current branch on each commit?

2014-04-29 Thread Robin Rosenberg


- Ursprungligt meddelande -
> Från: "Felipe Contreras" 
> Till: "James Denholm" , "Felipe Contreras" 
> 
> Kopia: "David Kastrup" , "Jeremy Morton" 
> , "Johan Herland" ,
> "Git mailing list" 
> Skickat: tisdag, 29 apr 2014 5:32:29
> Ämne: Re: Recording the current branch on each commit?
> 
> James Denholm wrote:
> > Felipe Contreras  wrote:
> > > James Denholm wrote:
> > >> It's not anybody else's job to take your patches and drizzle them in the
> > >> honey of respectable discourse.
> > >
> > > It's nobody's job to do anything. This a collaborative effort and in a
> > > collaborative effort everbody chimes in to do different things.
[...]

> In the Git project though, we choose to starve to death. Neither were my
> patches picked, nor did anybody else step up with a different proposal, we
> just
> did nothing, which is what we always do.

Just because you are starving, the others may not be. I'll skip dinner today.

Not all people view the world the same way you do. Sometimes they don't "see 
it" 
because they don't share your experience. A year later other people may have 
come to the
same conclusion as you (or not) and whatever the idea you had may come
from someone else, when the world is ready. 

Whining won't help, it will just reduce your credibility, perhaps to the point
that people won't even read a improved proposal if you come up with one.

Remember this is a high volume list, so you don't get much time to explain an 
idea. It's
a matter of karma.

-- robin
--
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: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
James Denholm wrote:

> You cannot expect that anybody but yourself is willing to propose,
> debate the merits of and otherwise defend patches that you have
> authored (herein "your patches", implying authorship, not
> ownership).

This is the original comment:

> David Kastrup wrote:
> > It becomes easier to actually change things when communicating in
> > a less abrasive and destructive manner.

Which is demonstrably false, as I already explained nobody else could
get these patches in, regarldless of the abrasiveness, or lack
thereof.

My point was that my abrasiveness is not an excuse not to do the
changes, as somebody else could get them in (or a similar proposal).
But they couldn't, because it's a change.

Your point about me not expecting somebody else to defend my patches
is irrelevant; it doesn't have anything to do with the topic, and it's
not relevant in general either.

I didn't ask or expect anybody to defend my patches, my point was that
David Kastrup was wrong; it wouldn't be easier to change things;
because change is simply not welcome.

> Ultimately, the only person who can ensure that a patch is
> championed, and the only person who need feel a responsibility to,
> is the author, and that responsibility is only ever to themselves.

Contributors don't have any responsibility to champion their patches.
It is pro bono work.

I should champion my patches because I want to improve Git, not
because I have a responsibility. And nobody else has any
responsibility either, but if somebody else want to improve Git as
well, they should chamption the patches (or others of their own) as
well.

In the meantime the problem still remains.

> > It doesn't matter if you want to go hunting and I want to buy
> > bread, either one of those is better than starving to death.
> 
> Not at all. Hunting may necessitate a negative side effect, such as
> betraying vegetarianism,  having to go out into the jungle for five
> days,  risk life and limb,  and (worse yet) sleep in a tent. This is
> an especially poor decision if we honestly would prefer a loaf of
> bread, and we just need to find a way across the street.

You obviously didn't read what I said.

> And again, I'm referring to the general case here, but of your
> views of what the solution should be clash with what the
> community view is, you're not going to be able to convince
> the community to go hunting.

I'm not going to convince them to buy bread either.

The community wants to starve to death, and you couldn't convince them
otherwise either.

-- 
Felipe Contreras
--
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: Recording the current branch on each commit?

2014-04-29 Thread David Kastrup
Felipe Contreras  writes:

> Contributors don't have any responsibility to champion their patches.
> It is pro bono work.

No, that's just the appearance that should be upheld in the higher
society.  It's ok to get paid for work on Git as long as you don't
mention it in public.  It's also ok to get paid for _promises_ of work
if you can make people believe you.  Open Source is not much different
from how politics and society in general work in the U.S.A.  To get the
real wads of money, you first need to get the means not to have to talk
about money (it's ok if you do it by means totally opposed to "the
political cause" as long as you don't talk about it), then you have to
prefinance people's trust in you not being there for the money, and then
you are in a position to get paid for your work.

Anyway, I digress.  Even without all that not so "pro bono" background
to "pro bono work", there is still a difference between "pro bono" work
ending up in the wastebin and "pro bono" work ending up in a product.

Even while the ones getting the benefits from your work will not feel an
obligation to make it worth your while, there is a difference in
satisfaction between getting your work trashed and getting it used.

The satisfaction by exploding in self-righteousness tends to be a poor
substitute and is comparatively short-lived.

Yes, it may mean that you have to carry your child the last yards rather
than shout it across the finishing line.  Even though it should have
legs perfectly suited to get it across the track on its own.

Only that way you get to pat it on its head.

-- 
David Kastrup
--
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 01/12] MINGW: compat/mingw.h: do not attempt to redefine lseek on mingw-w64

2014-04-29 Thread Marat Radchenko
Unlike MinGW, MinGW-W64 has lseek already properly defined in io.h.

Signed-off-by: Marat Radchenko 
Acked-by: Eric Faye-Lund 
---
 compat/mingw.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/compat/mingw.h b/compat/mingw.h
index e033e72..262b300 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -265,7 +265,9 @@ static inline int getrlimit(int resource, struct rlimit 
*rlp)
  * Use mingw specific stat()/lstat()/fstat() implementations on Windows.
  */
 #define off_t off64_t
+#ifndef lseek
 #define lseek _lseeki64
+#endif
 
 /* use struct stat with 64 bit st_size */
 #ifdef stat
-- 
1.9.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


[PATCH v2] MinGW(-W64) cross-compilation

2014-04-29 Thread Marat Radchenko
Differences with v1:
 - Dropped "MINGW: compat/bswap.h: include stdint.h", it isn't needed after
   "MINGW: git-compat-util.h: use inttypes.h for printf macros"
 - Split "MINGW: config.mak.uname allow using CURL for non-msysGit builds"
   into "MINGW: config.mak.uname: allow using cURL for non-msysGit builds"
   and "MINGW: fix main() signature in http-fetch.c and remote-curl.c"
 - Reworded "MINGW: git-compat-util.h: use inttypes.h for printf macros"
 - Reworded "MINGW: config.mak.uname: reorganize MINGW settings"
 - Rewrote "MINGW: config.mak.uname: drop -DNOGDI" into
   "MINGW: compat/poll/poll.c: undef NOGDI"
 - Rewrote "MINGW: config.mak.uname: drop USE_NED_ALLOCATOR" into
   "compat/nedmalloc/malloc.c.h: fix compilation under MinGW-W64"
 - Reworded "Makefile: introduce CROSS_COMPILE variable"
 - Reordeder commits (1-5 are Acked by: Eric Faye-Lund )

=

This patch series fixes building on modern MinGW and (32bit only yet) MinGW-W64.

*Compilation* tested on:
 - MSVC (via WinGit environment)
 - msysGit environment
 - Linux cross-toolchain i686-pc-mingw32 (4.8.2) with mingw-runtime-3.20.2
 - Linux cross-toolchain i686-w64-mingw32 (4.8.2) with mingw64-runtime-3.1.0

Stuff still required to make Git build with x86_64 MinGW-W64 toolchain:

1. Drop -D_USE_32BIT_TIME_T that was added in fa93bb to config.mak.uname
because time_t cannot be 32bit on x86_64. I haven't yet figured out what
should break if this define is removed (pointers are welcome) and why it was
added in the first place.

2. Stop passing --large-address-aware to linker. I wonder if it does anything
for 32bit MinGW builds.

3. Fix several places with mismatched pointer size casts.

Building it from Gentoo Linux:

MinGW:

  crossdev -t i686-pc-mingw32
  ARCH=x86 emerge-i686-pc-mingw32 -u dev-libs/libiconv sys-libs/zlib 
net-misc/curl sys-devel/gettext expat
  cd 
  make CROSS_COMPILE=i686-pc-mingw32- NO_OPENSSL=1 MINGW=1 
CURLDIR=/usr/i686-pc-mingw32/usr

MinGW-W64 (32 bit):

  crossdev -t i686-w64-mingw32
  ARCH=x86 emerge-i686-w64-mingw32 -u dev-libs/libiconv sys-libs/zlib 
net-misc/curl sys-devel/gettext expat
  cd 
  make CROSS_COMPILE=i686-w64-mingw32- NO_OPENSSL=1 MINGW=1 
CURLDIR=/usr/i686-w64-mingw32/usr

--
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/12] Makefile: introduce CROSS_COMPILE variable

2014-04-29 Thread Marat Radchenko
To ease cross-compilation process, introduce a single variable
with the prefix to all compiler-related executables.

Define CROSS_COMPILE=foo- if your compiler and binary utilities
are foo-cc, foo-ar, foo-strip, etc.  More specific variables
override this, so if you set CC=gcc CROSS_COMPILE=ia64-linux-gnu-
then the compiler will be 'gcc', not 'ia64-linux-gnu-gcc'.

Signed-off-by: Marat Radchenko 
---
 Makefile | 19 +--
 config.mak.uname |  2 +-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 74a929b..8406b94 100644
--- a/Makefile
+++ b/Makefile
@@ -350,6 +350,11 @@ all::
 #
 # Define GMTIME_UNRELIABLE_ERRORS if your gmtime() function does not
 # return NULL when it receives a bogus time_t.
+#
+# Define CROSS_COMPILE=foo- if your compiler and binary utilities
+# are foo-cc, foo-ar, foo-strip, etc.  More specific variables
+# override this, so if you set CC=gcc CROSS_COMPILE=ia64-linux-gnu-
+# then the compiler will be 'gcc', not 'ia64-linux-gnu-gcc'.
 
 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -361,7 +366,6 @@ CFLAGS = -g -O2 -Wall
 LDFLAGS =
 ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
-STRIP ?= strip
 
 # Among the variables below, these:
 #   gitexecdir
@@ -401,8 +405,12 @@ htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
 
 export prefix bindir sharedir sysconfdir gitwebdir localedir
 
-CC = cc
-AR = ar
+AR = $(CROSS_COMPILE)ar
+CC = $(CROSS_COMPILE)cc
+GCOV = $(CROSS_COMPILE)gcov
+RC = $(CROSS_COMPILE)windres
+STRIP = $(CROSS_COMPILE)strip
+
 RM = rm -f
 DIFF = diff
 TAR = tar
@@ -415,13 +423,12 @@ XGETTEXT = xgettext
 MSGFMT = msgfmt
 PTHREAD_LIBS = -lpthread
 PTHREAD_CFLAGS =
-GCOV = gcov
 
 export TCL_PATH TCLTK_PATH
 
 SPARSE_FLAGS =
 
-
+RCFLAGS =
 
 ### --- END CONFIGURATION SECTION ---
 
@@ -1796,7 +1803,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
mv $@+ $@
 
 git.res: git.rc GIT-VERSION-FILE
-   $(QUIET_RC)$(RC) \
+   $(QUIET_RC)$(RC) $(RCFLAGS) \
  $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., 
,$(GIT_VERSION) \
  -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
 
diff --git a/config.mak.uname b/config.mak.uname
index b68a7d1..d5f7953 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -507,7 +507,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
EXTLIBS += -lws2_32
GITLIBS += git.res
PTHREAD_LIBS =
-   RC = windres -O coff
+   RCFLAGS += -O coff
NATIVE_CRLF = YesPlease
X = .exe
SPARSE_FLAGS = -Wno-one-bit-signed-bitfield
-- 
1.9.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


[PATCH 02/12] MSVC: config.mak.uname: drop -D__USE_MINGW_ACCESS from CFLAGS

2014-04-29 Thread Marat Radchenko
-D__USE_MINGW_ACCESS only affects MinGW and does nothing when
MSVC is used.

Signed-off-by: Marat Radchenko 
Acked-by: Eric Faye-Lund 
---
 config.mak.uname | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 23a8803..faddb82 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -357,7 +357,7 @@ ifeq ($(uname_S),Windows)
COMPAT_OBJS = compat/msvc.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
compat/win32/dirent.o
-   COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H 
-DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 
-DSTRIP_EXTENSION=\".exe\"
+   COMPAT_CFLAGS = -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat 
-Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE 
-NODEFAULTLIB:MSVCRT.lib
EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib 
invalidcontinue.obj
PTHREAD_LIBS =
-- 
1.9.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


[PATCH 03/12] MINGW: compat/mingw.h: drop fork() definition

2014-04-29 Thread Marat Radchenko
fork() is not used in MinGW builds but causes a compiler warning
on x86_64 MinGW-W64: conflicting types for built-in function 'fork'

Signed-off-by: Marat Radchenko 
Acked-by: Eric Faye-Lund 
---
 compat/mingw.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 262b300..87d58ba 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -90,8 +90,6 @@ static inline int symlink(const char *oldpath, const char 
*newpath)
 { errno = ENOSYS; return -1; }
 static inline int fchmod(int fildes, mode_t mode)
 { errno = ENOSYS; return -1; }
-static inline pid_t fork(void)
-{ errno = ENOSYS; return -1; }
 static inline unsigned int alarm(unsigned int seconds)
 { return 0; }
 static inline int fsync(int fd)
-- 
1.9.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


[PATCH 08/12] MINGW: fix main() signature in http-fetch.c and remote-curl.c

2014-04-29 Thread Marat Radchenko
On MinGW, compat/mingw.h defines a 'mingw_main' wrapper function.
Fix `warning: passing argument 2 of 'mingw_main' from incompatible
pointer type` in http-fetch.c and remote-curl.c by dropping 'const'.

Signed-off-by: Marat Radchenko 
---
 http-fetch.c  | 5 +++--
 remote-curl.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/http-fetch.c b/http-fetch.c
index ba3ea10..a6a9a2f 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -6,7 +6,7 @@
 static const char http_fetch_usage[] = "git http-fetch "
 "[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin] commit-id url";
 
-int main(int argc, const char **argv)
+int main(int argc, char **argv)
 {
struct walker *walker;
int commits_on_stdin = 0;
@@ -38,7 +38,8 @@ int main(int argc, const char **argv)
} else if (argv[arg][1] == 'v') {
get_verbosely = 1;
} else if (argv[arg][1] == 'w') {
-   write_ref = &argv[arg + 1];
+   const char *ref = argv[arg + 1];
+   write_ref = &ref;
arg++;
} else if (argv[arg][1] == 'h') {
usage(http_fetch_usage);
diff --git a/remote-curl.c b/remote-curl.c
index 52c2d96..565b6c9 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -938,7 +938,7 @@ static void parse_push(struct strbuf *buf)
free(specs);
 }
 
-int main(int argc, const char **argv)
+int main(int argc, char **argv)
 {
struct strbuf buf = STRBUF_INIT;
int nongit;
-- 
1.9.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


[PATCH 04/12] MINGW: do not fail at redefining pid_t on MinGW-W64

2014-04-29 Thread Marat Radchenko
pid_t is available in sys/types.h on both MinGW and MinGW-W64

Signed-off-by: Marat Radchenko 
Acked-by: Eric Faye-Lund 
---
 compat/mingw.h | 1 -
 compat/msvc.h  | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 87d58ba..7e3d038 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -5,7 +5,6 @@
  * things that are not available in header files
  */
 
-typedef int pid_t;
 typedef int uid_t;
 typedef int socklen_t;
 #define hstrerror strerror
diff --git a/compat/msvc.h b/compat/msvc.h
index 580bb55..a63d878 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -15,6 +15,8 @@
 #define strtoull _strtoui64
 #define strtoll  _strtoi64
 
+typedef int pid_t;
+
 static __inline int strcasecmp (const char *s1, const char *s2)
 {
int size1 = strlen(s1);
-- 
1.9.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


[PATCH 11/12] compat/nedmalloc/malloc.c.h: fix compilation under MinGW-W64

2014-04-29 Thread Marat Radchenko
1. Unlike MinGW, MinGW-W64 already provides _ReadWriteBarrier macro,
   so don't try to redefine it.

2. MinGW-W64 has a strange definition FORCEINLINE as
   extern __inline__ __attribute__((__always_inline__,__gnu_inline__))

   'extern' doesn't work together with 'static', so #undef MinGW-W64
   version of FORCEINLINE.

Signed-off-by: Marat Radchenko 
---
 compat/nedmalloc/malloc.c.h | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
index f216a2a..a6c8cac 100644
--- a/compat/nedmalloc/malloc.c.h
+++ b/compat/nedmalloc/malloc.c.h
@@ -715,6 +715,10 @@ struct mallinfo {
 #endif /* HAVE_USR_INCLUDE_MALLOC_H */
 #endif /* NO_MALLINFO */
 
+#ifdef __MINGW64_VERSION_MAJOR
+  #undef FORCEINLINE
+#endif
+
 /*
   Try to persuade compilers to inline. The most critical functions for
   inlining are defined as macros, so these aren't used for them.
@@ -1382,7 +1386,9 @@ LONG __cdecl _InterlockedExchange(LONG volatile *Target, 
LONG Value);
 
   /*** Atomic operations ***/
   #if (__GNUC__ * 1 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) > 40100
-#define _ReadWriteBarrier() __sync_synchronize()
+#ifndef _ReadWriteBarrier
+  #define _ReadWriteBarrier() __sync_synchronize()
+#endif
   #else
 static __inline__ __attribute__((always_inline)) long 
__sync_lock_test_and_set(volatile long * const Target, const long Value)
 {
-- 
1.9.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


[PATCH 12/12] MINGW: config.mak.uname: add explicit way to request MinGW-build

2014-04-29 Thread Marat Radchenko
When crosscompiling, one cannot rely on `uname` from host system.
Thus, add an option to use `make MINGW=1` for building MinGW build
from non-MinGW host (Linux, for example).

Signed-off-by: Marat Radchenko 
---
 config.mak.uname | 5 +
 1 file changed, 5 insertions(+)

diff --git a/config.mak.uname b/config.mak.uname
index d5f7953..c7f3d74 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -13,6 +13,11 @@ ifdef MSVC
uname_O := Windows
 endif
 
+ifdef MINGW
+   uname_S := MINGW
+   uname_O := MINGW
+endif
+
 # We choose to avoid "if .. else if .. else .. endif endif"
 # because maintaining the nesting to match is a pain.  If
 # we had "elif" things would have been much nicer...
-- 
1.9.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


[PATCH 10/12] MINGW: compat/poll/poll.c: undef NOGDI

2014-04-29 Thread Marat Radchenko
On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI.

Removal -DNOGDI=1 from config.mak.uname has an undesirable effect of
bringing in wingdi.h with weird #define ERROR 0 that conflicts with
internal Git enums. So, just #undef NOGDI in compat/poll/poll.c.

Signed-off-by: Marat Radchenko 
---
 compat/poll/poll.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index 31163f2..e38cba8 100644
--- a/compat/poll/poll.c
+++ b/compat/poll/poll.c
@@ -38,6 +38,7 @@
 #include 
 
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+# undef NOGDI
 # define WIN32_NATIVE
 # if defined (_MSC_VER) && !defined(_WIN32_WINNT)
 #  define _WIN32_WINNT 0x0502
-- 
1.9.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


[PATCH 07/12] MINGW: config.mak.uname: reorganize MINGW settings

2014-04-29 Thread Marat Radchenko
HAVE_LIBCHARSET_H and NO_R_TO_GCC_LINKER are not specific to
msysGit, they're general MinGW settings.

Logic behind HAVE_LIBCHARSET_H: if user is on MinGW and has iconv,
we expect him to have libcharset.h. If user doesn't have iconv,
he has to explicitly say so via NO_ICONV=1.

Signed-off-by: Marat Radchenko 
---
 config.mak.uname | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index 50c1114..b68a7d1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -516,11 +516,11 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
prefix =
INSTALL = /bin/install
EXTLIBS += /mingw/lib/libz.a
-   NO_R_TO_GCC_LINKER = YesPlease
INTERNAL_QSORT = YesPlease
-   HAVE_LIBCHARSET_H = YesPlease
NO_GETTEXT = YesPlease
 endif
+   HAVE_LIBCHARSET_H = YesPlease
+   NO_R_TO_GCC_LINKER = YesPlease
 endif
 ifeq ($(uname_S),QNX)
COMPAT_CFLAGS += -DSA_RESTART=0
-- 
1.9.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


[PATCH 05/12] MINGW: config.mak.uname: allow using cURL for non-msysGit builds

2014-04-29 Thread Marat Radchenko
Is it absolutely valid and possible to have cURL in generic
MinGW environment. Building Git without cURL is still possible
by passing NO_CURL=1

Signed-off-by: Marat Radchenko 
Acked-by: Eric Faye-Lund 
---
 config.mak.uname | 2 --
 1 file changed, 2 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index faddb82..a626410 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -519,8 +519,6 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
INTERNAL_QSORT = YesPlease
HAVE_LIBCHARSET_H = YesPlease
NO_GETTEXT = YesPlease
-else
-   NO_CURL = YesPlease
 endif
 endif
 ifeq ($(uname_S),QNX)
-- 
1.9.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


[PATCH 06/12] MINGW: git-compat-util.h: use inttypes.h for printf macros

2014-04-29 Thread Marat Radchenko
All MinGW flavors have inttypes.h, so just include it.

However, we need to pass -D__USE_MINGW_ANSI_STDIO=0 to select
MSVCRT-compatible macro definitions on MinGW-W64:
http://sourceforge.net/apps/trac/mingw-w64/wiki/gnu%20printf

As a side-effect, Git no longer builds with MSVC < 2010 due to
its lack of stdint.h but hopefully that is not a problem.

Signed-off-by: Marat Radchenko 
---
 compat/mingw.h|  2 --
 compat/msvc.h |  3 +++
 config.mak.uname  |  3 ++-
 git-compat-util.h | 11 ++-
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 7e3d038..2fbc8ea 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -339,8 +339,6 @@ static inline char *mingw_find_last_dir_sep(const char 
*path)
 }
 #define find_last_dir_sep mingw_find_last_dir_sep
 #define PATH_SEP ';'
-#define PRIuMAX "I64u"
-#define PRId64 "I64d"
 
 void mingw_open_html(const char *path);
 #define open_html mingw_open_html
diff --git a/compat/msvc.h b/compat/msvc.h
index a63d878..84a03f9 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -17,6 +17,9 @@
 
 typedef int pid_t;
 
+#define PRIuMAX "I64u"
+#define PRId64 "I64d"
+
 static __inline int strcasecmp (const char *s1, const char *s2)
 {
int size1 = strlen(s1);
diff --git a/config.mak.uname b/config.mak.uname
index a626410..50c1114 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -316,6 +316,7 @@ ifeq ($(uname_S),Windows)
NO_PREAD = YesPlease
NEEDS_CRYPTO_WITH_SSL = YesPlease
NO_LIBGEN_H = YesPlease
+   NO_INTTYPES_H = UnfortunatelyYes
NO_POLL = YesPlease
NO_SYMLINK_HEAD = YesPlease
NO_IPV6 = YesPlease
@@ -497,7 +498,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_INET_NTOP = YesPlease
NO_POSIX_GOODIES = UnfortunatelyYes
DEFAULT_HELP_FORMAT = html
-   COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI 
-Icompat -Icompat/win32
+   COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -D__USE_MINGW_ACCESS 
-D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..65498a2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -95,6 +95,12 @@
 #define GIT_WINDOWS_NATIVE
 #endif
 
+#ifndef NO_INTTYPES_H
+#include 
+#else
+#include 
+#endif
+
 #include 
 #include 
 #include 
@@ -146,11 +152,6 @@
 #include 
 #include 
 #include 
-#ifndef NO_INTTYPES_H
-#include 
-#else
-#include 
-#endif
 #ifdef NO_INTPTR_T
 /*
  * On I16LP32, ILP32 and LP64 "long" is the save bet, however
-- 
1.9.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


[PATCH 0/8] remote-helper: fixes

2014-04-29 Thread Felipe Contreras
A few trivial updates, a few important fixes.


Daniel Liew (1):
  git-remote-hg: use internal clone's hgrc

Felipe Contreras (7):
  remote-hg: fix parsing of custom committer
  remote-hg: update to 'public' phase when pushing
  remote-{hg,bzr}: store marks only on success
  remote-hg: properly detect missing contexts
  t: remote-hg: split into setup test
  remote-hg: make sure we omit multiple heads
  remote-hg: trivial cleanups

 git-remote-bzr.py|  8 +++---
 git-remote-hg.py | 72 ++--
 t/t5810-remote-hg.sh | 65 +++
 3 files changed, 70 insertions(+), 75 deletions(-)

-- 
1.9.2+fc1.3.gade8541

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


[PATCH 6/8] git-remote-hg: use internal clone's hgrc

2014-04-29 Thread Felipe Contreras
From: Daniel Liew 

Use the hgrc configuration file in the internal mercurial repository in
addition to the other system wide hgrc files. This is done by using the
'ui' object from the 'repository' object which will have loaded the
repository hgrc file if it exists.

Signed-off-by: Dan Liew 
Signed-off-by: Felipe Contreras 
---
 git-remote-hg.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 1972f7f..cd3d79e 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -422,7 +422,7 @@ def get_repo(url, alias):
 
 repo = hg.repository(myui, local_path)
 try:
-peer = hg.peer(myui, {}, url)
+peer = hg.peer(repo.ui, {}, url)
 except:
 die('Repository error')
 repo.pull(peer, heads=None, force=True)
-- 
1.9.2+fc1.3.gade8541

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


[PATCH 1/8] remote-hg: fix parsing of custom committer

2014-04-29 Thread Felipe Contreras
Other tools use the 'committer' extra field differently, so let's make
the parsing more reliable and don't assume it's in a certain format.

Reported-by: Kevin Cox 
Signed-off-by: Felipe Contreras 
---
 git-remote-hg.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 34cda02..c849abb 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -461,8 +461,12 @@ def export_ref(repo, name, kind, head):
 
 author = "%s %d %s" % (fixup_user(user), time, gittz(tz))
 if 'committer' in extra:
-user, time, tz = extra['committer'].rsplit(' ', 2)
-committer = "%s %s %s" % (user, time, gittz(int(tz)))
+try:
+cuser, ctime, ctz = extra['committer'].rsplit(' ', 2)
+committer = "%s %s %s" % (cuser, ctime, gittz(int(ctz)))
+except ValueError:
+cuser = extra['committer']
+committer = "%s %d %s" % (fixup_user(cuser), time, gittz(tz))
 else:
 committer = author
 
-- 
1.9.2+fc1.3.gade8541

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


[PATCH 2/8] remote-hg: update to 'public' phase when pushing

2014-04-29 Thread Felipe Contreras
This is what Mercurial does.

Reported-by: Nathan Palmer
Signed-off-by: Felipe Contreras 
---
 git-remote-hg.py | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index c849abb..204ceeb 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -995,9 +995,17 @@ def push_unsafe(repo, remote, parsed_refs, p_revs):
 if unbundle:
 if force:
 remoteheads = ['force']
-return remote.unbundle(cg, remoteheads, 'push')
+ret = remote.unbundle(cg, remoteheads, 'push')
 else:
-return remote.addchangegroup(cg, 'push', repo.url())
+ret = remote.addchangegroup(cg, 'push', repo.url())
+
+phases = remote.listkeys('phases')
+if phases:
+for head in p_revs:
+# update to public
+remote.pushkey('phases', hghex(head), '1', '0')
+
+return ret
 
 def push(repo, remote, parsed_refs, p_revs):
 if hasattr(remote, 'canpush') and not remote.canpush():
-- 
1.9.2+fc1.3.gade8541

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


[PATCH 5/8] t: remote-hg: split into setup test

2014-04-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras 
---
 t/t5810-remote-hg.sh | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/t/t5810-remote-hg.sh b/t/t5810-remote-hg.sh
index 594a0a1..ba8b2d8 100755
--- a/t/t5810-remote-hg.sh
+++ b/t/t5810-remote-hg.sh
@@ -105,17 +105,18 @@ setup () {
 
 setup
 
-test_expect_success 'cloning' '
-   test_when_finished "rm -rf gitrepo*" &&
-
+test_expect_success 'setup' '
(
hg init hgrepo &&
cd hgrepo &&
echo zero >content &&
hg add content &&
hg commit -m zero
-   ) &&
+   )
+'
 
+test_expect_success 'cloning' '
+   test_when_finished "rm -rf gitrepo*" &&
git clone "hg::hgrepo" gitrepo &&
check gitrepo HEAD zero
 '
-- 
1.9.2+fc1.3.gade8541

--
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 8/8] remote-hg: trivial cleanups

2014-04-29 Thread Felipe Contreras
Cleanup 51be46e (remote-hg: do not fail on invalid bookmarks).

Having a 40-characters string is not ideal, and having three tests for
basically the same relatively rare situation is overkill.

Signed-off-by: Felipe Contreras 
---
 git-remote-hg.py |  2 +-
 t/t5810-remote-hg.sh | 56 
 2 files changed, 5 insertions(+), 53 deletions(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 402b92f..74f2a2e 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -677,7 +677,7 @@ def do_list(parser):
 print "? refs/heads/branches/%s" % gitref(branch)
 
 for bmark in bmarks:
-if  bmarks[bmark].hex() == '':
+if  bmarks[bmark].hex() == '0' * 40:
 warn("Ignoring invalid bookmark '%s'", bmark)
 else:
 print "? refs/heads/%s" % gitref(bmark)
diff --git a/t/t5810-remote-hg.sh b/t/t5810-remote-hg.sh
index ba8b2d8..9946f57 100755
--- a/t/t5810-remote-hg.sh
+++ b/t/t5810-remote-hg.sh
@@ -772,7 +772,7 @@ test_expect_success 'remote double failed push' '
)
 '
 
-test_expect_success 'clone remote with master null bookmark, then push to the 
bookmark' '
+test_expect_success 'clone remote with null bookmark, then push' '
test_when_finished "rm -rf gitrepo* hgrepo*" &&
 
hg init hgrepo &&
@@ -781,67 +781,19 @@ test_expect_success 'clone remote with master null 
bookmark, then push to the bo
echo a >a &&
hg add a &&
hg commit -m a &&
-   hg bookmark -r null master
+   hg bookmark -r null bookmark
) &&
 
git clone "hg::hgrepo" gitrepo &&
check gitrepo HEAD a &&
(
cd gitrepo &&
-   git checkout --quiet -b master &&
-   echo b >b &&
-   git add b &&
-   git commit -m b &&
-   git push origin master
-   )
-'
-
-test_expect_success 'clone remote with default null bookmark, then push to the 
bookmark' '
-   test_when_finished "rm -rf gitrepo* hgrepo*" &&
-
-   hg init hgrepo &&
-   (
-   cd hgrepo &&
-   echo a >a &&
-   hg add a &&
-   hg commit -m a &&
-   hg bookmark -r null -f default
-   ) &&
-
-   git clone "hg::hgrepo" gitrepo &&
-   check gitrepo HEAD a &&
-   (
-   cd gitrepo &&
-   git checkout --quiet -b default &&
-   echo b >b &&
-   git add b &&
-   git commit -m b &&
-   git push origin default
-   )
-'
-
-test_expect_success 'clone remote with generic null bookmark, then push to the 
bookmark' '
-   test_when_finished "rm -rf gitrepo* hgrepo*" &&
-
-   hg init hgrepo &&
-   (
-   cd hgrepo &&
-   echo a >a &&
-   hg add a &&
-   hg commit -m a &&
-   hg bookmark -r null bmark
-   ) &&
-
-   git clone "hg::hgrepo" gitrepo &&
-   check gitrepo HEAD a &&
-   (
-   cd gitrepo &&
-   git checkout --quiet -b bmark &&
+   git checkout --quiet -b bookmark &&
git remote -v &&
echo b >b &&
git add b &&
git commit -m b &&
-   git push origin bmark
+   git push origin bookmark
)
 '
 
-- 
1.9.2+fc1.3.gade8541

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


[PATCH 4/8] remote-hg: properly detect missing contexts

2014-04-29 Thread Felipe Contreras
This can happen when there's a synchronization issue between marks-git
and marks-hg; a key is missing in marks-hg, and when we receive a reset
command the value of ctx basically comes from None.

Signed-off-by: Felipe Contreras 
---
 git-remote-hg.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 8aca6dd..1972f7f 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -917,6 +917,11 @@ def checkheads_bmark(repo, ref, ctx):
 
 ctx_old = bmarks[bmark]
 ctx_new = ctx
+
+if not ctx.rev():
+print "error %s unknown" % ref
+return False
+
 if not repo.changelog.descendant(ctx_old.rev(), ctx_new.rev()):
 if force_push:
 print "ok %s forced update" % ref
-- 
1.9.2+fc1.3.gade8541

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


[PATCH 3/8] remote-{hg,bzr}: store marks only on success

2014-04-29 Thread Felipe Contreras
Commit 2594a79 (remote-hg: fix bad state issue) originally introduced
this code in order to avoid synchronization issues while pushing,
because `git fast-export` might end up writing the marks before a crash
in the remote helper occurs.

However, the problem is in `git fast-export`; the marks should only be
written after both have finished successfully.

Signed-off-by: Felipe Contreras 
---
 git-remote-bzr.py | 8 +++-
 git-remote-hg.py  | 8 +++-
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/git-remote-bzr.py b/git-remote-bzr.py
index 9abb58e..71b138e 100755
--- a/git-remote-bzr.py
+++ b/git-remote-bzr.py
@@ -971,12 +971,10 @@ def main(args):
 die('unhandled command: %s' % line)
 sys.stdout.flush()
 
+marks.store()
+
 def bye():
-if not marks:
-return
-if not is_tmp:
-marks.store()
-else:
+if is_tmp:
 shutil.rmtree(dirname)
 
 atexit.register(bye)
diff --git a/git-remote-hg.py b/git-remote-hg.py
index 204ceeb..8aca6dd 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -1258,12 +1258,10 @@ def main(args):
 die('unhandled command: %s' % line)
 sys.stdout.flush()
 
+marks.store()
+
 def bye():
-if not marks:
-return
-if not is_tmp:
-marks.store()
-else:
+if is_tmp:
 shutil.rmtree(dirname)
 
 atexit.register(bye)
-- 
1.9.2+fc1.3.gade8541

--
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 7/8] remote-hg: make sure we omit multiple heads

2014-04-29 Thread Felipe Contreras
We want to ignore secondary heads, otherwise we will import revisions
that won't have any ref pointing to them and might eventually be pruned,
which would cause problems with the synchronization of marks.

This can only be expressed properly as '::b - ::a', but that's not
efficient, specially in older versions of Mercurial.
'ancestor(a,b)::b - ancestor(a,b)::a' might be better, but it's not
particularly pretty. Mercurial v3.0 will have a new 'only(b, a)' that
does the same, but that hasn't even been released yet. Either way all of
these require repo.revs() which is only available after Mercurial v2.1.

Also, we would need special considerations for when there's no base
revision (importing from root).

It's much better to implement our own function to get a range of
revisions.

The new gitrange() is inspired by Mercurial's revset.missingancestors().

Signed-off-by: Felipe Contreras 
---
 git-remote-hg.py | 35 ---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index cd3d79e..402b92f 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -437,16 +437,45 @@ def rev_to_mark(rev):
 def mark_to_rev(mark):
 return marks.to_rev(mark)
 
+# Get a range of revisions in the form of a..b (git committish)
+def gitrange(repo, a, b):
+positive = []
+pending = set([int(b)])
+negative = set([int(a)])
+for cur in xrange(b, -1, -1):
+if not pending:
+break
+
+parents = [p for p in repo.changelog.parentrevs(cur) if p >= 0]
+
+if cur in pending:
+positive.append(cur)
+pending.remove(cur)
+for p in parents:
+if not p in negative:
+pending.add(p)
+elif cur in negative:
+negative.remove(cur)
+for p in parents:
+if not p in pending:
+negative.add(p)
+else:
+pending.discard(p)
+
+positive.reverse()
+return positive
+
 def export_ref(repo, name, kind, head):
 ename = '%s/%s' % (kind, name)
 try:
 tip = marks.get_tip(ename)
-tip = repo[tip].rev()
+tip = repo[tip]
 except:
-tip = 0
+tip = repo[-1]
 
-revs = xrange(tip, head.rev() + 1)
+revs = gitrange(repo, tip, head)
 total = len(revs)
+tip = tip.rev()
 
 for rev in revs:
 
-- 
1.9.2+fc1.3.gade8541

--
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 19/19] refs.c: pass **transaction to commit and have it clear the pointer

2014-04-29 Thread Michael Haggerty
On 04/28/2014 07:59 PM, Ronnie Sahlberg wrote:
> On Fri, Apr 25, 2014 at 6:31 PM, Michael Haggerty  
> wrote:
>> On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote:
>>> Change ref_transaction_commit to take a pointer to a pointer for the
>>> transaction. This allows us to clear the transaction pointer from within
>>> ref_transaction_commit so that it becomes NULL in the caller.
>>>
>>> This makes transaction handling in the callers much nicer since instead of
>>> having to write horrible code like :
>>>   t = ref_transaction_begin();
>>>   if ((!t ||
>>>   ref_transaction_update(t, refname, sha1, oldval, flags,
>>>  !!oldval)) ||
>>>   (ref_transaction_commit(t, action, &err) && !(t = NULL))) {
>>>   ref_transaction_rollback(t);
>>>
>>> we can now just do the much nicer
>>>   t = ref_transaction_begin();
>>>   if (!t ||
>>>   ref_transaction_update(t, refname, sha1, oldval, flags,
>>>  !!oldval) ||
>>>   ref_transaction_commit(&t, action, &err)) {
>>>   ref_transaction_rollback(t);
>>
>> I understand the motivation for this change, but passing
>> pointer-to-pointer is unconventional in a case like this.  Unfortunately
>> I ran out of steam for the night before I could think about alternatives.
> 
> I see.
> Yes passing a pointer to pointer is not ideal.
> But I still want to be able to use the pattern
>t = ref_transaction_begin();
>if (!t ||
>ref_transaction_update(t, ...) ||
>ref_transaction_commit(t, ...)) {
>ref_transaction_rollback(t);
> 
> Maybe the problem is that ref_transaction_commit() implicitely also
> frees the transaction.
> 
> 
> What about changing ref_transaction_commit() would NOT free the
> transaction and thus a caller would
> always have to explicitely free the transaction afterwards?
> 
> Something like this :
>t = ref_transaction_begin();
>if (!t ||
>ref_transaction_update(t, ...) ||
>ref_transaction_commit(&t, ...)) {

You wouldn't need the "&" here then, right?

>ref_transaction_rollback(t);
>}
>ref_transaction_free(t);

That sounds like a better solution.  We would want to make sure that
ref_transaction_commit() / ref_transaction_rollback() leaves the
ref_transaction in a state that if it is accidentally passed to
ref_transaction_update() or its friends, the function calls die("BUG: ...").

Unless we want to make ref_transaction objects reusable.  But then we
would need an explicit "allocation" step in the boilerplate code:

t = ref_transaction_alloc();
while (something) {
if (ref_transaction_begin(t) ||
 ref_transaction_update(t, ...) ||
 ref_transaction_commit(t, ...)) {
ref_transaction_rollback(t);
}
}
ref_transaction_free(t);

Note that ref_transaction_begin() should in this case be converted to
return 0-on-OK, negative-on-error for consistency.

This would bring us back to the familiar pattern alloc...use...free.

I was going to say that the extra boilerplate is not worth it, and
anyway reusing ref_transaction objects won't save any significant work.
 But then it occurred to me that ref_transaction_alloc() might be a
place to do more expensive work, like creating a connection to a
database, so reuse could potentially be a bigger win.

All in all, either way is OK with me.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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


Re: [PATCH v3 16/19] branch.c: use ref transaction for all ref updates

2014-04-29 Thread Michael Haggerty
On 04/28/2014 09:16 PM, Ronnie Sahlberg wrote:
> On Fri, Apr 25, 2014 at 4:16 PM, Michael Haggerty  
> wrote:
>> On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote:
>>> Change create_branch to use a ref transaction when creating the new branch.
>>> ref_transaction_create will check that the ref does not already exist and 
>>> fail
>>> otherwise meaning that we no longer need to keep a lock on the ref during 
>>> the
>>> setup_tracking. This simplifies the code since we can now do the transaction
>>> in one single step.
>>>
>>> If the forcing flag is false then use ref_transaction_create since this will
>>> fail if the ref already exist. Otherwise use ref_transaction_update.
>>>
>>> This also fixes a race condition in the old code where two concurrent
>>> create_branch could race since the lock_any_ref_for_update/write_ref_sha1
>>> did not protect against the ref already existsing. I.e. one thread could 
>>> end up
>>> overwriting a branch even if the forcing flag is false.
>>>
>>> Signed-off-by: Ronnie Sahlberg 
>>> ---
>>>  branch.c | 39 +--
>>>  1 file changed, 25 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/branch.c b/branch.c
>>> index 660097b..23cde1e 100644
>>> --- a/branch.c
>>> +++ b/branch.c
>>> @@ -226,7 +226,6 @@ void create_branch(const char *head,
>>>  int force, int reflog, int clobber_head,
>>>  int quiet, enum branch_track track)
>>>  {
>>> - struct ref_lock *lock = NULL;
>>>   struct commit *commit;
>>>   unsigned char sha1[20];
>>>   char *real_ref, msg[PATH_MAX + 20];
>>> @@ -285,15 +284,6 @@ void create_branch(const char *head,
>>>   die(_("Not a valid branch point: '%s'."), start_name);
>>>   hashcpy(sha1, commit->object.sha1);
>>>
>>> - if (!dont_change_ref) {
>>> - lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
>>> - if (!lock)
>>> - die_errno(_("Failed to lock ref for update"));
>>> - }
>>> -
>>> - if (reflog)
>>> - log_all_ref_updates = 1;
>>> -
>>>   if (forcing)
>>>   snprintf(msg, sizeof msg, "branch: Reset to %s",
>>>start_name);
>>> @@ -301,13 +291,34 @@ void create_branch(const char *head,
>>>   snprintf(msg, sizeof msg, "branch: Created from %s",
>>>start_name);
>>>
>>> + if (reflog)
>>> + log_all_ref_updates = 1;
>>> +
>>> + if (!dont_change_ref) {
>>> + struct ref_transaction *transaction;
>>> + char *err = NULL;
>>> +
>>> + transaction = ref_transaction_begin();
>>> + if (forcing) {
>>> + if (!transaction ||
>>> + ref_transaction_update(transaction, ref.buf, sha1,
>>> +NULL, 0, 0) ||
>>> + ref_transaction_commit(transaction, msg, &err))
>>> +   die_errno(_("%s: failed to write ref: %s"),
>>> + ref.buf, err);
>>> + } else {
>>> + if (!transaction ||
>>> + ref_transaction_create(transaction, ref.buf, sha1,
>>> +0) ||
>>> + ref_transaction_commit(transaction, msg, &err))
>>> +   die_errno(_("%s: failed to write ref: %s"),
>>> + ref.buf, err);
>>> + }
>>
>> You've got some indentation problems above.
>>
>> But actually, there seems like a lot of duplicated code here.  Couldn't
>> you instead do a single block with have_old set based on forcing:
>>
>> ref_transaction_update(transaction, ref.buf, sha1,
>>null_sha1, 0, !forcing)
>>
>> ?
> 
> Done, thanks.
> 
> 
> I am not sure how I feel about using _update to create new refs
> since we already have ref_transaction_create for that purpose.
> 
> ref_transaction_update can either be used to update an existing ref
> or it can be used to create new refs, either by passing have_old==0
> or by passing old_sha1==null_sha1 and have_old==1

Hold onto your socks then, because I think in the future update() should
get a have_new parameter too.  That way it can also be used to verify
the current value of a reference by passing have_old=1, have_new=0
without also re-setting the reference unnecessarily like now.  Though I
admit, have_old=have_new=0 might *not* be so useful :-)

> Maybe the api would be cleaner if we would change it so that update
> and create does
> not overlap and thus change _update so that it can only modify refs
> that must already exist ?

I have no compunctions about using update() to create or delete a
reference.  My point of view is that update() is the general case, and
create() and delete() are special-cases that exist only for the
convenience of callers.  For example, our future pluggable backends
might only have 

Re: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
David Kastrup wrote:
> Felipe Contreras  writes:
> 
> > Contributors don't have any responsibility to champion their
> > patches.  It is pro bono work.
> 
> No, that's just the appearance that should be upheld in the higher
> society.  It's ok to get paid for work on Git as long as you don't
> mention it in public.

The word "contribute" implies doing something that was not necessary.
If somebody is paying you to do something, then it's not a
contribution, it is simply your duty.

That's why I used the word "contributors", to separate the people that
don't have a responsability, and the ones that do.

> Even while the ones getting the benefits from your work will not
> feel an obligation to make it worth your while, there is a
> difference in satisfaction between getting your work trashed and
> getting it used.

I don't know why this keeps poping up in the thread, but it is
starting to seem to me that you are under the impression that I'm
somehow unable to get my patches merged.

Look at the list of contributors of the past year, see who is #2:

https://www.ohloh.net/p/git/contributors?query=&sort=commits_12_mo

I know what kind of patches can get in, and what patches can't (the
ones that do any kind of relevant change). I know that from
experience.

-- 
Felipe Contreras
--
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: Recording the current branch on each commit?

2014-04-29 Thread David Kastrup
Felipe Contreras  writes:

> David Kastrup wrote:
>
>> Even while the ones getting the benefits from your work will not
>> feel an obligation to make it worth your while, there is a
>> difference in satisfaction between getting your work trashed and
>> getting it used.
>
> I don't know why this keeps poping up in the thread, but it is
> starting to seem to me that you are under the impression that I'm
> somehow unable to get my patches merged.
>
> Look at the list of contributors of the past year, see who is #2:
>
> https://www.ohloh.net/p/git/contributors?query=&sort=commits_12_mo
>
> I know what kind of patches can get in, and what patches can't (the
> ones that do any kind of relevant change). I know that from
> experience.

Well, there you have it.  The ones that do any kind of relevant change
are the ones that need thinking about and consideration.  And when you
are so verbose about them that

a) you are getting on people's nerves
b) nobody else finds something worth saying that you did not already say

then the net effect is that it feels to the person in question he's
mainly doing you (and not all that many others) a favor by investing the
work for properly considering it and its consequences.

Which is not much of an incentive.  At any rate, we are in a phase
supposed to be shortly before the release of 2.0.  So it is actually
quite by design that patches doing any kind of relevant changes are not
currently going through.  You can be as nice or ugly about it as you
want right now and it will not affect 2.0 any more.

But it might do so regarding 2.1.

-- 
David Kastrup
--
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: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
David Kastrup wrote:
> Felipe Contreras  writes:
> 
> > David Kastrup wrote:
> >
> >> Even while the ones getting the benefits from your work will not
> >> feel an obligation to make it worth your while, there is a
> >> difference in satisfaction between getting your work trashed and
> >> getting it used.
> >
> > I don't know why this keeps poping up in the thread, but it is
> > starting to seem to me that you are under the impression that I'm
> > somehow unable to get my patches merged.
> >
> > Look at the list of contributors of the past year, see who is #2:
> >
> > https://www.ohloh.net/p/git/contributors?query=&sort=commits_12_mo
> >
> > I know what kind of patches can get in, and what patches can't (the
> > ones that do any kind of relevant change). I know that from
> > experience.
> 
> Well, there you have it.  The ones that do any kind of relevant change
> are the ones that need thinking about and consideration.  And when you
> are so verbose about them that
> 
> a) you are getting on people's nerves
> b) nobody else finds something worth saying that you did not already say
> 
> then the net effect is that it feels to the person in question he's
> mainly doing you (and not all that many others) a favor by investing the
> work for properly considering it and its consequences.

This is the last time I say it: this is demonstrably false.

You claim that relevant changes can be made if the submitter is not so verbose
(and less aggressive and what not).

This is obviously not the case. Show me any change of importance done in the
last two years, hell, make it four. And by change I mean something that was one
way before, and was another way after.

There is nothing. It doesn't matter how these changes are presented. Changes
are not welcome, doesn't matter who is championing them, or how. Period.

> At any rate, we are in a phase supposed to be shortly before the release of
> 2.0.

This is a red herring. All the patches I'm talking about were sent well before
2.0 was imminent, six months to one year ago.

Even more, I'm challenging you to find an important change since even four years
ago. You won't find any.

> But it might do so regarding 2.1.

No, it won't. Neither 3.0.

-- 
Felipe Contreras
--
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: Recording the current branch on each commit?

2014-04-29 Thread David Kastrup
Felipe Contreras  writes:

> David Kastrup wrote:
>
>> Well, there you have it.  The ones that do any kind of relevant change
>> are the ones that need thinking about and consideration.  And when you
>> are so verbose about them that
>> 
>> a) you are getting on people's nerves
>> b) nobody else finds something worth saying that you did not already say
>> 
>> then the net effect is that it feels to the person in question he's
>> mainly doing you (and not all that many others) a favor by investing
>> the work for properly considering it and its consequences.
>
> This is the last time I say it: this is demonstrably false.

Feelings are not categorizable as "demonstrably false".

> You claim that relevant changes can be made if the submitter is not so
> verbose (and less aggressive and what not).
>
> This is obviously not the case. Show me any change of importance done
> in the last two years, hell, make it four. And by change I mean
> something that was one way before, and was another way after.

The default behavior of "git push".  Colorized diffs.  "git add dir/"
can now remove files.  "git gc --aggressive" has been sanitized.

-- 
David Kastrup
--
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] MSVC: link dynamically to the CRT

2014-04-29 Thread Stepan Kasal
From: Karsten Blees 
Date: Fri, 7 Jan 2011 17:20:21 +0100

Dynamic linking is generally preferred over static linking, and MSVCRT.dll
has been integral part of Windows for a long time.

This also fixes linker warnings for _malloc and _free in zlib.lib, which
seems to be compiled for MSVCRT.dll already.

The DLL version also exports some of the CRT initialization functions,
which are hidden in the static libcmt.lib (e.g. __wgetmainargs, required by
subsequent Unicode patches).

Signed-off-by: Karsten Blees 
Signed-off-by: Stepan Kasal 
---

Another patch from msysgit.
Cheers,
  Stepan

 config.mak.uname | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index efaed94..eebc847 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -365,16 +365,16 @@ ifeq ($(uname_S),Windows)
compat/win32/pthread.o compat/win32/syslog.o \
compat/win32/dirent.o
COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H 
-DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 
-DSTRIP_EXTENSION=\".exe\"
-   BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE 
-NODEFAULTLIB:MSVCRT.lib
+   BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE
EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
PTHREAD_LIBS =
lib =
 ifndef DEBUG
-   BASIC_CFLAGS += -GL -Os -MT
+   BASIC_CFLAGS += -GL -Os -MD
BASIC_LDFLAGS += -LTCG
AR += -LTCG
 else
-   BASIC_CFLAGS += -Zi -MTd
+   BASIC_CFLAGS += -Zi -MDd
 endif
X = .exe
 endif
-- 
1.9.2.msysgit.0.158.g6070cee

--
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: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
David Kastrup wrote:
> Felipe Contreras  writes:
> 
> > David Kastrup wrote:
> >
> >> Well, there you have it.  The ones that do any kind of relevant change
> >> are the ones that need thinking about and consideration.  And when you
> >> are so verbose about them that
> >> 
> >> a) you are getting on people's nerves
> >> b) nobody else finds something worth saying that you did not already say
> >> 
> >> then the net effect is that it feels to the person in question he's
> >> mainly doing you (and not all that many others) a favor by investing
> >> the work for properly considering it and its consequences.
> >
> > This is the last time I say it: this is demonstrably false.
> 
> Feelings are not categorizable as "demonstrably false".

It's demonsrable by the challenge below.

> > You claim that relevant changes can be made if the submitter is not so
> > verbose (and less aggressive and what not).
> >
> > This is obviously not the case. Show me any change of importance done
> > in the last two years, hell, make it four. And by change I mean
> > something that was one way before, and was another way after.
> 
> The default behavior of "git push".

This is a minor change that not many people would notice, and it has not
actually happend. But fine, let's count it as one.

> Colorized diffs.

That's not a change.

> "git add dir/"

That doesn't count as an important change.

> can now remove files.

Irrelevant.

> "git gc --aggressive" has been sanitized.

Irrelevant. Nobody did notice.


That's all you could list for *four* years? None of that would even be noticed
by most of our users, maybe push.default (when it actually happens), but that's
*one*.

*One* important change in *four* years.

That's demonstration that change just does not happen. And if you disagree,
then we'll agree to disagree.

-- 
Felipe Contreras
--
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: Recording the current branch on each commit?

2014-04-29 Thread David Kastrup
Felipe Contreras  writes:

> David Kastrup wrote:
>
>> The default behavior of "git push".
>
> This is a minor change that not many people would notice, and it has not
> actually happend. But fine, let's count it as one.

Shrug.  Your diatribe is to a good part about the default behavior of
"git pull".  The "minor" change affects multiple branches in upstream,
while your "important change" affects a single local branch.

With that sort of bias, it's easy to convince yourself of anything.

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


[GIT GUI PATCH] git-gui: unconditionally use rev-parse --show-toplevel

2014-04-29 Thread Chris Packham
Previously 'git rev-parse --show-toplevel' was used to determine the
canonical work-tree only when the installed git version was detected to
be 1.7.0 or better. The fall-back logic used the core.worktree config
variable which in the case of a submodule is a relative path from the
submodule's $GIT_DIR. Unfortunately vsatisfies doesn't handle versions
like v2.0.0.rc0 so the fall-back logic is triggered.

Given the fact that git 1.7.0 was released over 4 years ago rather than
fixing the fall-back logic it seems reasonable to drop the version
check.

Signed-off-by: Chris Packham 
---
So I'm not sure if vsatisfies is failing because the version has .rc0 or
because it thinks v2.0.0 < 1.7.0. Regardless I think it's reasonably safe to
assume that people who are using the newer versions of git-gui are running new
enough versions of git.

There is also a similar section in rescan_stage2 that is checking for the
version being 1.6.3 or newer. Again I think it's probably safe to assume that
no-one is running a version of git that old (or at least no-one that wants to
run this version of git-gui). I'm not quite sure how to excercise that bit of
code so I haven't attempted to fix that.

 git-gui.sh | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index cf2209b..9ded5b9 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1282,23 +1282,11 @@ if {![file isdirectory $_gitdir]} {
 load_config 0
 apply_config
 
-# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
-   if { [is_Cygwin] } {
-   catch {set _gitworktree [exec cygpath --windows [git rev-parse 
--show-toplevel]]}
-   } else {
-   set _gitworktree [git rev-parse --show-toplevel]
-   }
+# Determine the canonical work-tree
+if { [is_Cygwin] } {
+   catch {set _gitworktree [exec cygpath --windows [git rev-parse 
--show-toplevel]]}
 } else {
-   # try to set work tree from environment, core.worktree or use
-   # cdup to obtain a relative path to the top of the worktree. If
-   # run from the top, the ./ prefix ensures normalize expands pwd.
-   if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
-   set _gitworktree [get_config core.worktree]
-   if {$_gitworktree eq ""} {
-   set _gitworktree [file normalize ./[git rev-parse 
--show-cdup]]
-   }
-   }
+   set _gitworktree [git rev-parse --show-toplevel]
 }
 
 if {$_prefix ne {}} {
-- 
1.8.2.rc1.4.g27db5a0

--
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: Recording the current branch on each commit?

2014-04-29 Thread James Denholm
I've no right to say this, given that I've no contributions
thus far to the project, little history in open source at all,
and have only been following the list for less than a week,
but I'll say it anyway, mayhaps.

And I don't mean this to cause offence, or inspire outrage,
or any similar sort of thing. I mean this only with good
intentions.

But Felipe, if you honestly feel that git has stagnated, and
that your contributions aren't wanted because we'd
rather starve, then perhaps git isn't the right project for you.

I'm not saying that you shouldn't work on the git codebase,
you could very easily fork it and make the innovative SCMS
none of us can see, and kill git. Can be done, if hunting really
is the best choice as you say.

--
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 v10 11/12] Documentation: add documentation for 'git interpret-trailers'

2014-04-29 Thread Jeremy Morton

On 28/04/2014 17:37, Junio C Hamano wrote:

Christian Couder  writes:


From: Junio C Hamano


Christian Couder  writes:
...



+   trailer. After some alphanumeric characters, it can contain
+   some non alphanumeric characters like ':', '=' or '#' that will
+   be used instead of ':' to separate the token from the value in
+   the trailer, though the default ':' is more standard.


I assume that this is for things like

bug #538

and the configuration would say something like:

[trailer "bug"]
key = "bug #"

For completeness (of this example), the bog-standard s-o-b would
look like

Signed-off-by: Christian Couder

and the configuration for it that spell the redundant "key" would
be:

[trailer "Signed-off-by"]
key = "Signed-off-by: "


Yeah, but you can use the following instead:

[trailer "s-o-b"]
key = "Signed-off-by: "


One thing I'm not quite understanding is where the "Christian 
Couder" bit comes from.  So you've defined the 
trailer token and key, but interpret-trailers then needs to get the 
value it will give for the key from somewhere.  Does it have to just be 
hardcoded in?  We probably want some way to get various variables like 
current branch name, current git version, etc.  So in the case of always 
adding a trailer for the branch that the commit was checked in to at the 
time (Developed-on, Made-on-branch, Author-branch, etc. [I think my 
favourite is Made-on-branch]), you'd want something like:


[trailer "m-o-b"]
key = "Made-on-branch: "
value = "$currentBranch"

... resulting in the trailer (for example):
Made-on-branch: pacman-minigame

Also, if there were no current branch name because you're committing in 
a detached head state, it would be nice if you could have some logic to 
determine that, and instead write the trailer as:

Made-on-branch: (detached HEAD: AB12CD34)

... or whatever.  And also how about some logic to be able to say that 
if you're committing to the "master" branch, the trailer doesn't get 
inserted at all?


--
Best regards,
Jeremy Morton (Jez)
--
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 v5 3/6] pull: refactor $rebase variable into $mode

2014-04-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras 
---
 git-pull.sh | 65 -
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index d4e25f1..3dfd856 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -57,16 +57,11 @@ then
mode=$(git config pull.mode)
 fi
 case "$mode" in
-merge)
-   rebase="false"
-   ;;
-rebase)
-   rebase="true"
+merge|rebase|'')
;;
 rebase-preserve)
-   rebase="preserve"
-   ;;
-'')
+   mode="rebase"
+   rebase_args="--preserve-merges"
;;
 *)
echo "Invalid value for 'mode'"
@@ -74,7 +69,8 @@ rebase-preserve)
exit 1
;;
 esac
-if test -z "$rebase"
+
+if test -z "$mode"
 then
rebase=$(bool_or_string_config branch.$curr_branch_short.rebase)
if test -z "$rebase"
@@ -153,10 +149,10 @@ do
rebase="${1#*=}"
;;
-r|--r|--re|--reb|--reba|--rebas|--rebase)
-   rebase=true
+   mode=rebase
;;
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
-   rebase=false
+   mode=
;;
--recurse-submodules)
recurse_submodules=--recurse-submodules
@@ -187,19 +183,26 @@ do
shift
 done
 
-case "$rebase" in
-preserve)
-   rebase=true
-   rebase_args=--preserve-merges
-   ;;
-true|false|'')
-   ;;
-*)
-   echo "Invalid value for --rebase, should be true, false, or preserve"
-   usage
-   exit 1
-   ;;
-esac
+if test -n "$rebase"
+then
+   case "$rebase" in
+   true)
+   mode="rebase"
+   ;;
+   false)
+   mode="merge"
+   ;;
+   preserve)
+   mode="rebase"
+   rebase_args=--preserve-merges
+   ;;
+   *)
+   echo "Invalid value for --rebase, should be true, false, or 
preserve"
+   usage
+   exit 1
+   ;;
+   esac
+fi
 
 error_on_no_merge_candidates () {
exec >&2
@@ -213,7 +216,7 @@ error_on_no_merge_candidates () {
esac
done
 
-   if test true = "$rebase"
+   if test "$mode" = rebase
then
op_type=rebase
op_prep=against
@@ -226,7 +229,7 @@ error_on_no_merge_candidates () {
remote=$(git config "branch.$curr_branch_short.remote")
 
if [ $# -gt 1 ]; then
-   if [ "$rebase" = true ]; then
+   if [ "$mode" = rebase ]; then
printf "There is no candidate for rebasing against "
else
printf "There are no candidates for merging "
@@ -249,7 +252,7 @@ error_on_no_merge_candidates () {
exit 1
 }
 
-test true = "$rebase" && {
+test "$mode" = rebase && {
if ! git rev-parse -q --verify HEAD >/dev/null
then
# On an unborn branch
@@ -306,7 +309,7 @@ case "$merge_head" in
then
die "$(gettext "Cannot merge multiple branches into empty 
head")"
fi
-   if test true = "$rebase"
+   if test "$mode" = rebase
then
die "$(gettext "Cannot rebase onto multiple branches")"
fi
@@ -327,7 +330,7 @@ then
exit
 fi
 
-if test true = "$rebase"
+if test "$mode" = rebase
 then
o=$(git show-branch --merge-base $curr_branch $merge_head $oldremoteref)
if test "$oldremoteref" = "$o"
@@ -337,8 +340,8 @@ then
 fi
 
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
-case "$rebase" in
-true)
+case "$mode" in
+rebase)
eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args 
$verbosity"
eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
;;
-- 
1.9.2+fc1.3.gade8541

--
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 v5 2/6] pull: migrate all the tests to pull.mode

2014-04-29 Thread Felipe Contreras
And branch.$name.pullmode.

Signed-off-by: Felipe Contreras 
---
 t/t5505-remote.sh |  2 +-
 t/t5520-pull.sh   | 54 +++---
 2 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index ac79dd9..76376e4 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -181,7 +181,7 @@ test_expect_success 'show' '
git branch -d -r origin/master &&
git config --add remote.two.url ../two &&
git config --add remote.two.pushurl ../three &&
-   git config branch.rebase.rebase true &&
+   git config branch.rebase.pullmode rebase &&
git config branch.octopus.merge "topic-a topic-b topic-c" &&
(
cd ../one &&
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 227d293..01ad17a 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -123,26 +123,26 @@ test_expect_success '--rebase' '
test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
test new = $(git show HEAD:file2)
 '
-test_expect_success 'pull.rebase' '
+test_expect_success 'pull.mode=rebase' '
git reset --hard before-rebase &&
-   test_config pull.rebase true &&
+   test_config pull.mode rebase &&
git pull . copy &&
test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
test new = $(git show HEAD:file2)
 '
 
-test_expect_success 'branch.to-rebase.rebase' '
+test_expect_success 'branch.to-rebase.pullmode=rebase' '
git reset --hard before-rebase &&
-   test_config branch.to-rebase.rebase true &&
+   test_config branch.to-rebase.pullmode rebase &&
git pull . copy &&
test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
test new = $(git show HEAD:file2)
 '
 
-test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
+test_expect_success 'branch.to-rebase.pullmode should override pull.mode' '
git reset --hard before-rebase &&
-   test_config pull.rebase true &&
-   test_config branch.to-rebase.rebase false &&
+   test_config pull.mode merge &&
+   test_config branch.to-rebase.pullmode merge &&
git pull . copy &&
test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
test new = $(git show HEAD:file2)
@@ -150,7 +150,7 @@ test_expect_success 'branch.to-rebase.rebase should 
override pull.rebase' '
 
 # add a feature branch, keep-merge, that is merged into master, so the
 # test can try preserving the merge commit (or not) with various
-# --rebase flags/pull.rebase settings.
+# --rebase flags/pull.mode settings.
 test_expect_success 'preserve merge setup' '
git reset --hard before-rebase &&
git checkout -b keep-merge second^ &&
@@ -160,48 +160,40 @@ test_expect_success 'preserve merge setup' '
git tag before-preserve-rebase
 '
 
-test_expect_success 'pull.rebase=false create a new merge commit' '
+test_expect_success 'pull.mode=merge create a new merge commit' '
git reset --hard before-preserve-rebase &&
-   test_config pull.rebase false &&
+   test_config pull.mode merge &&
git pull . copy &&
test $(git rev-parse HEAD^1) = $(git rev-parse before-preserve-rebase) 
&&
test $(git rev-parse HEAD^2) = $(git rev-parse copy) &&
test file3 = $(git show HEAD:file3.t)
 '
 
-test_expect_success 'pull.rebase=true flattens keep-merge' '
+test_expect_success 'pull.mode=rebase flattens keep-merge' '
git reset --hard before-preserve-rebase &&
-   test_config pull.rebase true &&
+   test_config pull.mode rebase &&
git pull . copy &&
test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
test file3 = $(git show HEAD:file3.t)
 '
 
-test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' 
'
+test_expect_success 'pull.mode=rebase-preserve rebases and merges keep-merge' '
git reset --hard before-preserve-rebase &&
-   test_config pull.rebase 1 &&
-   git pull . copy &&
-   test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
-   test file3 = $(git show HEAD:file3.t)
-'
-
-test_expect_success 'pull.rebase=preserve rebases and merges keep-merge' '
-   git reset --hard before-preserve-rebase &&
-   test_config pull.rebase preserve &&
+   test_config pull.mode rebase-preserve &&
git pull . copy &&
test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)
 '
 
-test_expect_success 'pull.rebase=invalid fails' '
+test_expect_success 'pull.mode=invalid fails' '
git reset --hard before-preserve-rebase &&
-   test_config pull.rebase invalid &&
+   test_config pull.mode invalid &&
! git pull . copy
 '
 
 test_expect_success '--rebase=false create a new merge commit' '
git reset --hard before-preserve-rebase &&
-   test_config pull.rebase true

[PATCH v5 4/6] pull: add --merge option

2014-04-29 Thread Felipe Contreras
Also, deprecate --no-rebase since there's no need for it any more.

Signed-off-by: Felipe Contreras 
---
 Documentation/git-pull.txt | 8 ++--
 git-pull.sh| 6 +-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 9a91b9f..767bca3 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -127,8 +127,12 @@ It rewrites history, which does not bode well when you
 published that history already.  Do *not* use this option
 unless you have read linkgit:git-rebase[1] carefully.
 
---no-rebase::
-   Override earlier --rebase.
+-m::
+--merge::
+   Force a merge.
++
+See `pull.mode`, `branch..pullmode` in linkgit:git-config[1] if you want
+to make `git pull` always use `--merge`.
 
 Options related to fetching
 ~~~
diff --git a/git-pull.sh b/git-pull.sh
index 3dfd856..26e4e55 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -151,8 +151,12 @@ do
-r|--r|--re|--reb|--reba|--rebas|--rebase)
mode=rebase
;;
+   -m|--m|--me|--mer|--merg|--merge)
+   mode=merge
+   ;;
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
-   mode=
+   mode=merge
+   warn "$(gettext "--no-rebase is deprecated, please use --merge 
instead")"
;;
--recurse-submodules)
recurse_submodules=--recurse-submodules
-- 
1.9.2+fc1.3.gade8541

--
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 v5 6/6] pull: only allow ff merges by default

2014-04-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras 
---
 Documentation/git-pull.txt   | 18 ++
 git-pull.sh  |  2 +-
 t/t4013-diff-various.sh  |  2 +-
 t/t5500-fetch-pack.sh|  2 +-
 t/t5524-pull-msg.sh  |  2 +-
 t/t5700-clone-reference.sh   |  4 ++--
 t/t6022-merge-rename.sh  | 20 ++--
 t/t6026-merge-attr.sh|  2 +-
 t/t6029-merge-subtree.sh |  6 +++---
 t/t6037-merge-ours-theirs.sh | 10 +-
 10 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 767bca3..ca8e951 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -23,6 +23,7 @@ More precisely, 'git pull' runs 'git fetch' with the given
 parameters and calls 'git merge' to merge the retrieved branch
 heads into the current branch.
 With `--rebase`, it runs 'git rebase' instead of 'git merge'.
+With `--merge`, it forces the merge, even if it's non-fast forward.
 
  should be the name of a remote repository as
 passed to linkgit:git-fetch[1].   can name an
@@ -41,11 +42,28 @@ Assume the following history exists and the current branch 
is
 
  A---B---C master on origin
 /
+D---E master
+
+
+Then `git pull` will merge in a fast-foward way up to the new master.
+
+
+D---E---A---B---C master, origin/master
+
+
+However, a non-fast-foward case looks very different.
+
+
+ A---B---C origin/master
+/
 D---E---F---G master
^
origin/master in your repository
 
 
+By default `git pull` will fail on these situations, however, most likely
+you would want to force a merge, which you can do with `git pull --merge`.
+
 Then "`git pull`" will fetch and replay the changes from the remote
 `master` branch since it diverged from the local `master` (i.e., `E`)
 until its current commit (`C`) on top of `master` and record the
diff --git a/git-pull.sh b/git-pull.sh
index 946cbbe..8cf8f68 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -83,7 +83,7 @@ then
warn "$(gettext "Please use pull.mode and 
branch..pullmode instead.")"
fi
 fi
-test -z "$mode" && mode=merge
+test -z "$mode" && mode=merge-ff-only
 dry_run=
 while :
 do
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index e77c09c..1840767 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -64,7 +64,7 @@ test_expect_success setup '
export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
 
git checkout master &&
-   git pull -s ours . side &&
+   git pull --merge -s ours . side &&
 
GIT_AUTHOR_DATE="2006-06-26 00:05:00 +" &&
GIT_COMMITTER_DATE="2006-06-26 00:05:00 +" &&
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 5b2b1c2..f735cfe 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -259,7 +259,7 @@ test_expect_success 'clone shallow object count' '
 test_expect_success 'pull in shallow repo with missing merge base' '
(
cd shallow &&
-   test_must_fail git pull --depth 4 .. A
+   test_must_fail git pull --merge --depth 4 .. A
)
 '
 
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index 8cccecc..ec9f413 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 test_expect_success pull '
 (
cd cloned &&
-   git pull --log &&
+   git pull --merge --log &&
git log -2 &&
git cat-file commit HEAD >result &&
grep Dollar result
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 6537911..306badf 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -94,7 +94,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd C &&
-git pull origin'
+git pull --merge origin'
 
 cd "$base_dir"
 
@@ -109,7 +109,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd D &&
-git pull origin'
+git pull --merge origin'
 
 cd "$base_dir"
 
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index a89dfbe..f63300f 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -100,7 +100,7 @@ git checkout master'
 test_expect_success 'pull renaming branch into unrenaming one' \
 '
git show-branch &&
-   test_expect_code 1 git pull . white &&
+   test_expect_code 1 git pull --merge . white &&
git ls-files -s &&
git ls-files -u B >b.stages &&
test_line_count = 3 b.stages &&
@@ -118,7 +118,7 @@ test_expect_success 'pull renaming branch into another 
renaming one' \
rm -f B &&
git reset --hard &&
git checkout red &&
-   test_expect_code 1 git pull . white &&
+   test_expect_code 1 git pull --merge . white &&
git ls-files -u B >b.stages &&
test_line_count = 3 b.stages &&
git ls-files -s N >n.stag

[PATCH v5 0/6] Reject non-ff pulls by default

2014-04-29 Thread Felipe Contreras
It is very typical for Git newcomers to inadvertently create merges and worst:
inadvertently pushing them. This is one of the reasons many experienced users
prefer to avoid 'git pull', and recommend newcomers to avoid it as well.

To avoid these problems and keep 'git pull' useful, it has been agreed that
'git pull' should barf by default if the merge is non-fast-forward.
Unfortunately this breaks backwards-compatibility, so we need to be careful
about the error messages we give, and that we provide enough information to our
users to move forward without distrupting their workflow too much.

With the proper error messages and documentation, it has been agreed that the
new behavior is OK.

These are the steps needed to achieve this:

4) Only allow fast-forward merges by default

We could pass --ff-only to `git merge`, however, if we do that we'll get an 
error like this:

  Not possible to fast-forward, aborting.

This is not friendly; we want an error that is user-friendly:

  The pull was not fast-forward, please either merge or rebase.
  If unsure, run 'git pull --merge'.

When we do this we want to give the users the option to go back to the previous
behavior, so a new configuration is needed.

3) Add merge-ff-only config

This option would trigger a check inside `git pull` itself, and error out with
the aforementioned message if it's not possible to do a fast-forward merge.

However, this option conflicts with --rebase, and --no-rebase. Solution below.

2) Add --merge option

Since we have a message that says "If unsure, run 'git pull --merge'", which is
more friendly than 'git pull --no-rebase', we should add this option, and
deprecate --no-rebase.

However, the documentation would become confusing if --merge is configured in
pull.rebase, instead, we want something like this:

  See `pull.mode`, `branch..pullmode` in linkgit:git-config[1] if you want
  to make `git pull` always use `--merge`.

1) Rename pull.rename to pull.mode and
   branch..rebase to branch..pullmode

This way the configurations and options remain consistent:

  git pull --merge
  pull.mode = merge
  branch..pullmode = merge

  git pull --rebase
  pull.mode = rebase
  branch..pullmode = rebase

  git pull --rebase=preserve
  pull.mode = rebase-preserve
  branch..pullmode = rebase-preserve

  git pull
  pull.mode = merge-ff-only
  branch..pullmode = merge-ff-only
 
This patch series does all the steps mentioned, but in reverse order, and in
addition updates the tests to use the new configurations instead.


Felipe Contreras (6):
  pull: rename pull.rename to pull.mode
  pull: migrate all the tests to pull.mode
  pull: refactor $rebase variable into $mode
  pull: add --merge option
  pull: add merge-ff-only option
  pull: only allow ff merges by default

 Documentation/config.txt |  37 ---
 Documentation/git-pull.txt   |  28 ++--
 branch.c |   4 +-
 builtin/remote.c |  14 +-
 git-pull.sh  | 105 ---
 t/t3200-branch.sh|  40 -
 t/t4013-diff-various.sh  |   2 +-
 t/t5500-fetch-pack.sh|   2 +-
 t/t5505-remote.sh|   2 +-
 t/t5520-pull.sh  |  90 -
 t/t5524-pull-msg.sh  |   2 +-
 t/t5601-clone.sh |   4 +-
 t/t5700-clone-reference.sh   |   4 +-
 t/t6022-merge-rename.sh  |  20 -
 t/t6026-merge-attr.sh|   2 +-
 t/t6029-merge-subtree.sh |   6 +--
 t/t6037-merge-ours-theirs.sh |  10 ++---
 17 files changed, 245 insertions(+), 127 deletions(-)

-- 
1.9.2+fc1.3.gade8541

--
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 v5 1/6] pull: rename pull.rename to pull.mode

2014-04-29 Thread Felipe Contreras
Also 'branch..rebase' to 'branch..pullmode'.

This way 'pull.mode' can be set to 'merge', and the default can be
something else.

The old configurations still work, but get deprecated.

Signed-off-by: Felipe Contreras 
---
 Documentation/config.txt   | 34 +-
 Documentation/git-pull.txt |  2 +-
 branch.c   |  4 ++--
 builtin/remote.c   | 14 --
 git-pull.sh| 39 +--
 t/t3200-branch.sh  | 40 
 t/t5601-clone.sh   |  4 ++--
 7 files changed, 91 insertions(+), 46 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c26a7c8..5978d35 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -708,7 +708,7 @@ branch.autosetupmerge::
 branch.autosetuprebase::
When a new branch is created with 'git branch' or 'git checkout'
that tracks another branch, this variable tells Git to set
-   up pull to rebase instead of merge (see "branch..rebase").
+   up pull to rebase instead of merge (see "branch..pullmode").
When `never`, rebase is never automatically set to true.
When `local`, rebase is set to true for tracked branches of
other local branches.
@@ -764,15 +764,15 @@ branch..mergeoptions::
option values containing whitespace characters are currently not
supported.
 
-branch..rebase::
-   When true, rebase the branch  on top of the fetched branch,
-   instead of merging the default branch from the default remote when
-   "git pull" is run. See "pull.rebase" for doing this in a non
-   branch-specific manner.
+branch..pullmode::
+   When "git pull" is run, this determines if it would either merge or
+   rebase the fetched branch. The possible values are 'merge',
+   'rebase', and 'rebase-preserve'. See "pull.mode" for doing this in a
+   non branch-specific manner.
 +
-   When preserve, also pass `--preserve-merges` along to 'git rebase'
-   so that locally committed merge commits will not be flattened
-   by running 'git pull'.
+   When 'rebase-preserve', also pass `--preserve-merges` along to
+   'git rebase' so that locally committed merge commits will not be
+   flattened by running 'git pull'.
 +
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]
@@ -1881,15 +1881,15 @@ pretty.::
Note that an alias with the same name as a built-in format
will be silently ignored.
 
-pull.rebase::
-   When true, rebase branches on top of the fetched branch, instead
-   of merging the default branch from the default remote when "git
-   pull" is run. See "branch..rebase" for setting this on a
-   per-branch basis.
+pull.mode::
+   When "git pull" is run, this determines if it would either merge or
+   rebase the fetched branch. The possible values are 'merge',
+   'rebase', and 'rebase-preserve'. See "branch..pullmode" for doing
+   this in a non branch-specific manner.
 +
-   When preserve, also pass `--preserve-merges` along to 'git rebase'
-   so that locally committed merge commits will not be flattened
-   by running 'git pull'.
+   When 'rebase-preserve', also pass `--preserve-merges` along to
+   'git rebase' so that locally committed merge commits will not be
+   flattened by running 'git pull'.
 +
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 200eb22..9a91b9f 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -117,7 +117,7 @@ locally created merge commits will not be flattened.
 +
 When false, merge the current branch into the upstream branch.
 +
-See `pull.rebase`, `branch..rebase` and `branch.autosetuprebase` in
+See `pull.mode`, `branch..pullmode` and `branch.autosetuprebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `--rebase` instead of merging.
 +
diff --git a/branch.c b/branch.c
index 723a36b..63ce671 100644
--- a/branch.c
+++ b/branch.c
@@ -71,8 +71,8 @@ void install_branch_config(int flag, const char *local, const 
char *origin, cons
 
if (rebasing) {
strbuf_reset(&key);
-   strbuf_addf(&key, "branch.%s.rebase", local);
-   git_config_set(key.buf, "true");
+   strbuf_addf(&key, "branch.%s.pullmode", local);
+   git_config_set(key.buf, "rebase");
}
strbuf_release(&key);
 
diff --git a/builtin/remote.c b/builtin/remote.c
index b3ab4cf..46d3c4d 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -264,7 +264,7 @@ static int config_read_branches(const char *key, const char 
*value, void *cb)
char *name;
stru

[PATCH v5 5/6] pull: add merge-ff-only option

2014-04-29 Thread Felipe Contreras
It is very typical for Git newcomers to inadvertently create merges and
worst; inadvertently pushing them. This is one of the reasons many
experienced users prefer to avoid 'git pull', and recommend newcomers to
avoid it as well.

To avoid these problems and keep 'git pull' useful, it has been
suggested that 'git pull' barfs by default if the merge is
non-fast-forward, which unfortunately would break backwards
compatibility.

This patch leaves everything in place to enable this new mode, but it
only gets enabled if the user specifically configures it; pull.mode =
merge-ff-only.

Later on this mode can be enabled by default (e.g. in v2.0).

For the full discussion you can read:

http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225305

Signed-off-by: Felipe Contreras 
---
 Documentation/config.txt |  7 +--
 git-pull.sh  | 15 ++-
 t/t5520-pull.sh  | 36 
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5978d35..4ebd3b5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1884,8 +1884,11 @@ pretty.::
 pull.mode::
When "git pull" is run, this determines if it would either merge or
rebase the fetched branch. The possible values are 'merge',
-   'rebase', and 'rebase-preserve'. See "branch..pullmode" for doing
-   this in a non branch-specific manner.
+   'rebase', 'merge-ff-only,' and 'rebase-preserve'.
+   If 'merge-ff-only' is specified, the merge will only succeed if it's
+   fast-forward.
+   See "branch..pullmode" for doing this in a non branch-specific
+   manner.
 +
When 'rebase-preserve', also pass `--preserve-merges` along to
'git rebase' so that locally committed merge commits will not be
diff --git a/git-pull.sh b/git-pull.sh
index 26e4e55..946cbbe 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -57,7 +57,7 @@ then
mode=$(git config pull.mode)
 fi
 case "$mode" in
-merge|rebase|'')
+merge|rebase|merge-ff-only|'')
;;
 rebase-preserve)
mode="rebase"
@@ -83,6 +83,7 @@ then
warn "$(gettext "Please use pull.mode and 
branch..pullmode instead.")"
fi
 fi
+test -z "$mode" && mode=merge
 dry_run=
 while :
 do
@@ -318,6 +319,18 @@ case "$merge_head" in
die "$(gettext "Cannot rebase onto multiple branches")"
fi
;;
+*)
+   # check if a non-fast-foward merge would be needed
+   merge_head=${merge_head% }
+   if test "$mode" = merge-ff-only -a -z 
"$no_ff$ff_only${squash#--no-squash}" &&
+   test -n "$orig_head" &&
+   ! git merge-base --is-ancestor "$orig_head" "$merge_head" &&
+   ! git merge-base --is-ancestor "$merge_head" "$orig_head"
+   then
+   die "$(gettext "The pull was not fast-forward, please either 
merge or rebase.
+If unsure, run 'git pull --merge'.")"
+   fi
+   ;;
 esac
 
 # Pulling into unborn branch: a shorthand for branching off
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 01ad17a..2e2b476 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -365,4 +365,40 @@ test_expect_success 'git pull --rebase against local 
branch' '
test file = "$(cat file2)"
 '
 
+test_expect_success 'git pull fast-forward' '
+   test_when_finished "git checkout master && git branch -D other test" &&
+   test_config pull.mode merge-ff-only &&
+   git checkout -b other master &&
+   >new &&
+   git add new &&
+   git commit -m new &&
+   git checkout -b test -t other &&
+   git reset --hard master &&
+   git pull
+'
+
+test_expect_success 'git pull non-fast-forward' '
+   test_when_finished "git checkout master && git branch -D other test" &&
+   test_config pull.mode merge-ff-only &&
+   git checkout -b other master^ &&
+   >new &&
+   git add new &&
+   git commit -m new &&
+   git checkout -b test -t other &&
+   git reset --hard master &&
+   test_must_fail git pull
+'
+
+test_expect_success 'git pull non-fast-forward (merge)' '
+   test_when_finished "git checkout master && git branch -D other test" &&
+   test_config pull.mode merge-ff-only &&
+   git checkout -b other master^ &&
+   >new &&
+   git add new &&
+   git commit -m new &&
+   git checkout -b test -t other &&
+   git reset --hard master &&
+   git pull --merge
+'
+
 test_done
-- 
1.9.2+fc1.3.gade8541

--
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 v10 11/12] Documentation: add documentation for 'git interpret-trailers'

2014-04-29 Thread Christian Couder
On Tue, Apr 29, 2014 at 1:05 PM, Jeremy Morton  wrote:
> On 28/04/2014 17:37, Junio C Hamano wrote:
>>
>> Christian Couder  writes:
>>
>>> From: Junio C Hamano


 Christian Couder  writes:
 ...
>>>
>>>
> +   trailer. After some alphanumeric characters, it can contain
> +   some non alphanumeric characters like ':', '=' or '#' that will
> +   be used instead of ':' to separate the token from the value in
> +   the trailer, though the default ':' is more standard.


 I assume that this is for things like

 bug #538

 and the configuration would say something like:

 [trailer "bug"]
 key = "bug #"

 For completeness (of this example), the bog-standard s-o-b would
 look like

 Signed-off-by: Christian Couder

 and the configuration for it that spell the redundant "key" would
 be:

 [trailer "Signed-off-by"]
 key = "Signed-off-by: "
>>>
>>>
>>> Yeah, but you can use the following instead:
>>>
>>> [trailer "s-o-b"]
>>> key = "Signed-off-by: "
>
>
> One thing I'm not quite understanding is where the "Christian
> Couder" bit comes from.  So you've defined the
> trailer token and key, but interpret-trailers then needs to get the value it
> will give for the key from somewhere.  Does it have to just be hardcoded in?
> We probably want some way to get various variables like current branch name,
> current git version, etc.  So in the case of always adding a trailer for the
> branch that the commit was checked in to at the time (Developed-on,
> Made-on-branch, Author-branch, etc. [I think my favourite is
> Made-on-branch]), you'd want something like:
>
> [trailer "m-o-b"]
> key = "Made-on-branch: "
> value = "$currentBranch"
>
> ... resulting in the trailer (for example):
> Made-on-branch: pacman-minigame

In the documentation patch, there is:

trailer..command::
   This option can be used to specify a shell command that will
   be used to automatically add or modify a trailer with the
   specified 'token'.

   When this option is specified, it is like if a special 'token=value'
   argument is added at the end of the command line, where 'value' will
   be given by the standard output of the specified command.

   If the command contains the `$ARG` string, this string will be
   replaced with the 'value' part of an existing trailer with the same
   token, if any, before the command is launched.

That's why Something like the following should work if "git commit"
automitically runs "git interpret-trailers":

 [trailer "m-o-b"]
 key = "Made-on-branch: "
 command = "git name-rev --name-only HEAD"


> Also, if there were no current branch name because you're committing in a
> detached head state, it would be nice if you could have some logic to
> determine that, and instead write the trailer as:
> Made-on-branch: (detached HEAD: AB12CD34)

You may need to write a small script for that.
Then you just need the "trailer.m-o-b.command" config value to point
to your script.

> ... or whatever.  And also how about some logic to be able to say that if
> you're committing to the "master" branch, the trailer doesn't get inserted
> at all?

You can script that too.

Best,
Christian.
--
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: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
David Kastrup wrote:
> Felipe Contreras  writes:
> 
> > David Kastrup wrote:
> >
> >> The default behavior of "git push".
> >
> > This is a minor change that not many people would notice, and it has not
> > actually happend. But fine, let's count it as one.
> 
> Shrug.  Your diatribe is to a good part about the default behavior of
> "git pull".

> The "minor" change affects multiple branches in upstream,

This is what most people see by default since two years ago:

  warning: push.default is unset; its implicit value is changing in
  Git 2.0 from 'matching' to 'simple'. To squelch this message
  and maintain the current behavior after the default changes, use:

git config --global push.default matching

  To squelch this message and adopt the new behavior now, use:

git config --global push.default simple

  When push.default is set to 'matching', git will push local branches
  to the remote branches that already exist with the same name.

  In Git 2.0, Git will default to the more conservative 'simple'
  behavior, which only pushes the current branch to the corresponding
  remote branch that 'git pull' uses to update the current branch.

  See 'git help config' and search for 'push.default' for further information.
  (the 'simple' mode was introduced in Git 1.7.11. Use the similar mode
  'current' instead of 'simple' if you sometimes use older versions of Git)

Do you honestly believe that there's *anybody* out there is OK with seeing this
message _every_ _single_ _time_ he pushes? No. Everybody has already configured
push.default to one thing or the other. They won't see the change when 2.0 is
released.

And no, if by some miracle somebody hasn't configured that, it still won't
affect the branches upstream, if anything changes is that the `git pull` will
error out warning the user that the upstream branch doesn't have the same name.
It won't affect the branches you actually push.

> while your "important change" affects a single local branch.

My change does actually affect upstream branches, more than that, it affects
the upstream topology, because Git newcomers make merges by mistake when they
call `git pull` without knowing what the hell is going on. Everybody knows
that.

And this is a red herring. I never said my change was important, we are talking
about the changes that have actually happened in the last four years, which is
none.

> With that sort of bias, it's easy to convince yourself of anything.

I'm done discussing with you. I already demonstrated that your claim is wrong.
Change just doesn't happen.

-- 
Felipe Contreras
--
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: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
James Denholm wrote:
> I've no right to say this, given that I've no contributions I'm not
> saying that you shouldn't work on the git codebase, you could very
> easily fork it and make the innovative SCMS none of us can see, and
> kill git. Can be done, if hunting really is the best choice as you
> say.

I already made a fork:

http://felipec.wordpress.com/2013/10/28/git-fc/

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


Advarsel

2014-04-29 Thread Webmaster
Postkassen har overskredet grænsen for opbevaring. re-validere din postkasse 
ved hjælp af nedenstående link.

https://knlhymiopiojda.typeform.com/to/HDCcIw

Tak,
Webmail System Administrator
--
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 v5 1/6] pull: rename pull.rename to pull.mode

2014-04-29 Thread Marat Radchenko
Felipe Contreras wrote
> [PATCH v5 1/6] pull: rename pull.rename to pull.mode

s/pull.rename/pull.rebase/



--
View this message in context: 
http://git.661346.n2.nabble.com/PATCH-v5-0-6-Reject-non-ff-pulls-by-default-tp7609118p7609129.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


Re: Recording the current branch on each commit?

2014-04-29 Thread James Denholm
On 29 April 2014 21:47:42 GMT+10:00, Felipe Contreras 
 wrote:
>James Denholm wrote:
>> I've no right to say this, given that I've no contributions I'm not
>> saying that you shouldn't work on the git codebase, you could very
>> easily fork it and make the innovative SCMS none of us can see, and
>> kill git. Can be done, if hunting really is the best choice as you
>> say.
>
>I already made a fork:
>
>http://felipec.wordpress.com/2013/10/28/git-fc/

Sweet. So now you're going to get open source journalism
interested in git-fc and gain a groundswell of support, right?
So that we can all have egg on our faces when it takes off
and is proven superior... Right?

--
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] PAGER_ENV: remove 'S' from $LESS by default

2014-04-29 Thread Matthieu Moy
Junio C Hamano  writes:

> Jeff King  writes:
>
>> On Mon, Apr 28, 2014 at 02:22:21PM +0200, Matthieu Moy wrote:
>>
>> I'd be OK with doing the moral equivalent for now (perhaps just taking
>> Junio's proposal[1]), and I can deal with the refactoring later when
>> re-rolling the Makefile series.
>>
>> -Peff
>>
>> [1] http://article.gmane.org/gmane.comp.version-control.git/240637
>
> I doubt we would want to use the patch verbatim in that message; it
> served its purpose well to illustrate that there may be other ways
> to address the issue, but I agreed with the flaw in it you pointed
> out in the thread [*1*]
>
> I also agree that droppage of S does not have to wait for that
> topic.

So, shall I rewrite my patch on top of master? (not hard, but there will
be a minor conflict to resolve when merging with Peff's cooking series).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Feature request: Provide porcelain to manage symbolic references as branch aliases

2014-04-29 Thread Phil Sainty
Most of the plumbing for having branch name aliases already exists
in the form of symbolic references, and people do use them for this
purpose; but I get the impression that it's not really supported
officially, and I'm not aware of any porcelain features to
facilitate this use-case.

I'd like to propose that such porcelain be added. I feel that a new
argument to 'git branch' would make the most sense:

git branch --alias  []

For reference/testing, I'm attaching a wrapper script I wrote to
implement the same functionality (as a separate command). I did this
primarily to provide the error-checking I felt was needed to make it
practical to use branch aliases -- git symbolic-ref will happily
trash your branch references if you make a mistake, whereas it's
pretty difficult to mess anything up with my git-branch-alias script.

Thus far it's worked nicely for me. Examples:

$ git branch-alias short some-overly-long-branch-name # creates alias
$ git branch-alias short # creates alias for current branch
$ git log short
$ git checkout short
$ git push origin short # pushes the branch, not the alias/reference
$ git branch-alias --delete short

n.b. For my proposed porcelain change, these examples would become:
$ git branch --alias short some-overly-long-branch-name # creates alias
$ git branch --alias short # creates alias for current branch
$ git branch --delete short # works since 1.8.0.1, but see below.


Since using this script, the only thing I've spotted that I'd like
to be different is the commit message if I "git merge ". The
commit message indicates that I've merged  rather than the
 that it points at. I'd prefer that it was dereferenced
when the message was generated, so that the real branch name was
printed instead.


That niggle aside, significant things I noted along the way were:

1. Git 1.8.0.1 fixed the problem whereby git branch -d 
   used to dereference  and therefore delete the branch
   it pointed at, rather than the reference.

2. HOWEVER if you have  checked out at the time you
   delete it, you're in trouble -- git allows you to do it, and
   you're then left with an invalid HEAD reference.

   (I think this ought to be considered a current bug in git.)

3. I resolved that situation (2) by using "git symbolic-ref HEAD"
   to find the target ref, and setting HEAD to that target. Nothing
   changes for the user, but we can now delete the reference safely.

   HOWEVER, there's a problem with detecting that situation (2)
   in the first place:

4. Chains of references are de-referenced atomically -- the entire
   reference chain is followed to its end; and I could find no
   facility to obtain ONLY the "next link of the chain".

   This means we can't use "git symbolic-ref HEAD" to check whether
   it points to another reference. In my script I had to resort to
   inspecting HEAD manually, which obviously isn't desirable.

   I think a new argument is warranted here, perhaps something like:
   "git symbolic-ref --max-deref-count=1"

   I'll justify that on the assumption that (2) needs fixing in git
   regardless, either by:

   (i) Not allowing the user to delete the checked-out symref (which
   would be consistent with the behaviour if the user attempts to
   "git branch -d " (for an actual branch name) when that
   is the currently checked-out branch.

   or,
   (ii) Using the approach I've taken: silently setting HEAD to the
branch to which the symref points before deleting that symref.
(I couldn't see any reason not to use this approach.)

   But as in both cases we need to detect that HEAD is the symref
   being deleted, which means that we need the ability to explicitly
   dereference only a single step of a reference chain.


-Phil


#!/bin/sh
# git branch-alias
# Version 1.07
# Author: Phil S.

# Creates branch aliases, so that you can refer to a long branch name
# by a convenient short alias. This is just a "do what I mean" wrapper
# around git symbolic-ref, but without the (considerable) risk of
# trashing a branch if you get your arguments wrong

# Examples:
# git branch-alias short some-overly-long-branch-name # creates alias
# git branch-alias short # creates alias for current branch
# git log short
# git checkout short
# git push origin short # pushes the branch, not the alias/reference
# git branch-alias --delete short

# Caveats:
# Although everything else I've tried works seamlessly, I note that
# git merge  will cause the alias name to be mentioned in the
# commit message, rather than the real branch. It would be nicer if
# the branch name appeared.

# Compatibility:
# Developed with git version 1.7.12.4
# Tested with git version 1.9.0
#
# Related git changes between versions 1.7.12.4 and 1.9.0:
#
# 1.8.0.1
#  * A symbolic ref refs/heads/SYM was not correctly removed with "git
#branch -d SYM"; the command removed the ref pointed by SYM
#instead.
#
# 1.8.1
#  * "git symbolic-ref" learned the "-d $symref" option to delete the
#named symbol

Re: [PATCH v10 11/12] Documentation: add documentation for 'git interpret-trailers'

2014-04-29 Thread Jeremy Morton

On 29/04/2014 12:47, Christian Couder wrote:

Also, if there were no current branch name because you're committing in a
detached head state, it would be nice if you could have some logic to
determine that, and instead write the trailer as:
 Made-on-branch: (detached HEAD: AB12CD34)


You may need to write a small script for that.
Then you just need the "trailer.m-o-b.command" config value to point
to your script.


... or whatever.  And also how about some logic to be able to say that if
you're committing to the "master" branch, the trailer doesn't get inserted
at all?


You can script that too.


But it would be nicer if the logic were built-in, then you wouldn't have 
to share some script with your work colleagues. :-)


--
Best regards,
Jeremy Morton (Jez)
--
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 v10 11/12] Documentation: add documentation for 'git interpret-trailers'

2014-04-29 Thread Jeremy Morton

On 29/04/2014 12:47, Christian Couder wrote:

Also, if there were no current branch name because you're committing in a
detached head state, it would be nice if you could have some logic to
determine that, and instead write the trailer as:
 Made-on-branch: (detached HEAD: AB12CD34)


You may need to write a small script for that.
Then you just need the "trailer.m-o-b.command" config value to point
to your script.


... or whatever.  And also how about some logic to be able to say that if
you're committing to the "master" branch, the trailer doesn't get inserted
at all?


You can script that too.


But it would be nicer if the logic were built-in, then you wouldn't have 
to share some script with your work colleagues. :-)


--
Best regards,
Jeremy Morton (Jez)
--
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: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
James Denholm wrote:
> So that we can all have egg on our faces when it takes off and is
> proven superior... Right?

I don't know what you mean by "we", but it certainly doesn't include
you.

  % git log --author=nod.h...@gmail.com master
  empty

-- 
Felipe Contreras
--
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 v5 1/6] pull: rename pull.rename to pull.mode

2014-04-29 Thread Felipe Contreras
Marat Radchenko wrote:
> Felipe Contreras wrote
> > [PATCH v5 1/6] pull: rename pull.rename to pull.mode
> 
> s/pull.rename/pull.rebase/

Right. Will fix.

-- 
Felipe Contreras
--
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 16/19] branch.c: use ref transaction for all ref updates

2014-04-29 Thread Ronnie Sahlberg
On Tue, Apr 29, 2014 at 2:35 AM, Michael Haggerty  wrote:
> On 04/28/2014 09:16 PM, Ronnie Sahlberg wrote:
>> On Fri, Apr 25, 2014 at 4:16 PM, Michael Haggerty  
>> wrote:
>>> On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote:
 Change create_branch to use a ref transaction when creating the new branch.
 ref_transaction_create will check that the ref does not already exist and 
 fail
 otherwise meaning that we no longer need to keep a lock on the ref during 
 the
 setup_tracking. This simplifies the code since we can now do the 
 transaction
 in one single step.

 If the forcing flag is false then use ref_transaction_create since this 
 will
 fail if the ref already exist. Otherwise use ref_transaction_update.

 This also fixes a race condition in the old code where two concurrent
 create_branch could race since the lock_any_ref_for_update/write_ref_sha1
 did not protect against the ref already existsing. I.e. one thread could 
 end up
 overwriting a branch even if the forcing flag is false.

 Signed-off-by: Ronnie Sahlberg 
 ---
  branch.c | 39 +--
  1 file changed, 25 insertions(+), 14 deletions(-)

 diff --git a/branch.c b/branch.c
 index 660097b..23cde1e 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -226,7 +226,6 @@ void create_branch(const char *head,
  int force, int reflog, int clobber_head,
  int quiet, enum branch_track track)
  {
 - struct ref_lock *lock = NULL;
   struct commit *commit;
   unsigned char sha1[20];
   char *real_ref, msg[PATH_MAX + 20];
 @@ -285,15 +284,6 @@ void create_branch(const char *head,
   die(_("Not a valid branch point: '%s'."), start_name);
   hashcpy(sha1, commit->object.sha1);

 - if (!dont_change_ref) {
 - lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
 - if (!lock)
 - die_errno(_("Failed to lock ref for update"));
 - }
 -
 - if (reflog)
 - log_all_ref_updates = 1;
 -
   if (forcing)
   snprintf(msg, sizeof msg, "branch: Reset to %s",
start_name);
 @@ -301,13 +291,34 @@ void create_branch(const char *head,
   snprintf(msg, sizeof msg, "branch: Created from %s",
start_name);

 + if (reflog)
 + log_all_ref_updates = 1;
 +
 + if (!dont_change_ref) {
 + struct ref_transaction *transaction;
 + char *err = NULL;
 +
 + transaction = ref_transaction_begin();
 + if (forcing) {
 + if (!transaction ||
 + ref_transaction_update(transaction, ref.buf, 
 sha1,
 +NULL, 0, 0) ||
 + ref_transaction_commit(transaction, msg, &err))
 +   die_errno(_("%s: failed to write ref: %s"),
 + ref.buf, err);
 + } else {
 + if (!transaction ||
 + ref_transaction_create(transaction, ref.buf, 
 sha1,
 +0) ||
 + ref_transaction_commit(transaction, msg, &err))
 +   die_errno(_("%s: failed to write ref: %s"),
 + ref.buf, err);
 + }
>>>
>>> You've got some indentation problems above.
>>>
>>> But actually, there seems like a lot of duplicated code here.  Couldn't
>>> you instead do a single block with have_old set based on forcing:
>>>
>>> ref_transaction_update(transaction, ref.buf, sha1,
>>>null_sha1, 0, !forcing)
>>>
>>> ?
>>
>> Done, thanks.
>>
>>
>> I am not sure how I feel about using _update to create new refs
>> since we already have ref_transaction_create for that purpose.
>>
>> ref_transaction_update can either be used to update an existing ref
>> or it can be used to create new refs, either by passing have_old==0
>> or by passing old_sha1==null_sha1 and have_old==1
>
> Hold onto your socks then, because I think in the future update() should
> get a have_new parameter too.  That way it can also be used to verify
> the current value of a reference by passing have_old=1, have_new=0
> without also re-setting the reference unnecessarily like now.  Though I
> admit, have_old=have_new=0 might *not* be so useful :-)
>
>> Maybe the api would be cleaner if we would change it so that update
>> and create does
>> not overlap and thus change _update so that it can only modify refs
>> that must already exist ?
>
> I have no compunctions about using update() to create or delete a
> reference.  My poi

Re: [PATCH v4 08/27] refs.c: change ref_transaction_update() to do error checking and return status

2014-04-29 Thread Ronnie Sahlberg
On Mon, Apr 28, 2014 at 5:51 PM, Eric Sunshine  wrote:
> On Mon, Apr 28, 2014 at 6:54 PM, Ronnie Sahlberg  wrote:
>> Update ref_transaction_update() do some basic error checking and return
>> true on error. Update all callers to check ref_transaction_update() for 
>> error.
>>
>> Signed-off-by: Ronnie Sahlberg 
>> ---
>>  builtin/update-ref.c | 10 ++
>>  refs.c   |  9 +++--
>>  refs.h   | 10 +-
>>  3 files changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
>> index 2bef2a0..59c4d6b 100644
>> --- a/builtin/update-ref.c
>> +++ b/builtin/update-ref.c
>> @@ -197,8 +197,9 @@ static const char *parse_cmd_update(struct strbuf 
>> *input, const char *next)
>> if (*next != line_termination)
>> die("update %s: extra input: %s", refname, next);
>>
>> -   ref_transaction_update(transaction, refname, new_sha1, old_sha1,
>> -  update_flags, have_old);
>> +   if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
>> +  update_flags, have_old))
>> +   die("update %s: failed", refname);
>>
>> update_flags = 0;
>> free(refname);
>> @@ -286,8 +287,9 @@ static const char *parse_cmd_verify(struct strbuf 
>> *input, const char *next)
>> if (*next != line_termination)
>> die("verify %s: extra input: %s", refname, next);
>>
>> -   ref_transaction_update(transaction, refname, new_sha1, old_sha1,
>> -  update_flags, have_old);
>> +   if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
>> +  update_flags, have_old))
>> +   die("failed transaction update for %s", refname);
>>
>> update_flags = 0;
>> free(refname);
>> diff --git a/refs.c b/refs.c
>> index 308e13e..1a903fb 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -,19 +,24 @@ static struct ref_update *add_update(struct 
>> ref_transaction *transaction,
>> return update;
>>  }
>>
>> -void ref_transaction_update(struct ref_transaction *transaction,
>> +int ref_transaction_update(struct ref_transaction *transaction,
>> const char *refname,
>> const unsigned char *new_sha1,
>> const unsigned char *old_sha1,
>> int flags, int have_old)
>>  {
>> -   struct ref_update *update = add_update(transaction, refname);
>> +   struct ref_update *update;
>> +
>> +   if (have_old && !old_sha1)
>> +   die("have_old is true but old_sha1 is NULL");
>>
>> +   update = add_update(transaction, refname);
>> hashcpy(update->new_sha1, new_sha1);
>> update->flags = flags;
>> update->have_old = have_old;
>> if (have_old)
>> hashcpy(update->old_sha1, old_sha1);
>> +   return 0;
>
> Am I misreading? The commit message talks about returning true on
> error, but this code seems to only ever die() or return 0 (false).
>

I have updated the commit message to be more precise.
The function returns true on error but there are currently no checks
that actually
do return failure in this function. Such checks will be added in the future.
Also having it return an int instead of void allows us to use the pattern

if(!transaction ||
  ref_transaction_update(transaction, ...) |||
  ref_transaction_commit(transaction, ...)) {

eventhough we do not yet have any conditions where _update will fail.




>>  }
>>
>>  void ref_transaction_create(struct ref_transaction *transaction,
>> diff --git a/refs.h b/refs.h
>> index bc7715e..0364a3e 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -237,11 +237,11 @@ void ref_transaction_rollback(struct ref_transaction 
>> *transaction);
>>   * that the reference should have had before the update, or zeros if
>>   * it must not have existed beforehand.
>>   */
>> -void ref_transaction_update(struct ref_transaction *transaction,
>> -   const char *refname,
>> -   const unsigned char *new_sha1,
>> -   const unsigned char *old_sha1,
>> -   int flags, int have_old);
>> +int ref_transaction_update(struct ref_transaction *transaction,
>> +  const char *refname,
>> +  const unsigned char *new_sha1,
>> +  const unsigned char *old_sha1,
>> +  int flags, int have_old);
>>
>>  /*
>>   * Add a reference creation to transaction.  new_sha1 is the value
>> --
>> 1.9.1.528.g98b8868.dirty
--
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 12/27] replace.c: use the ref transaction functions for updates

2014-04-29 Thread Ronnie Sahlberg
On Mon, Apr 28, 2014 at 5:44 PM, Eric Sunshine  wrote:
> On Mon, Apr 28, 2014 at 6:54 PM, Ronnie Sahlberg  wrote:
>> Update replace.c to use ref transactions for updates.
>>
>> Signed-off-by: Ronnie Sahlberg 
>> ---
>>  builtin/replace.c | 14 --
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/replace.c b/builtin/replace.c
>> index b62420a..b037b29 100644
>> --- a/builtin/replace.c
>> +++ b/builtin/replace.c
>> @@ -129,7 +129,8 @@ static int replace_object(const char *object_ref, const 
>> char *replace_ref,
>> unsigned char object[20], prev[20], repl[20];
>> enum object_type obj_type, repl_type;
>> char ref[PATH_MAX];
>> -   struct ref_lock *lock;
>> +   struct ref_transaction *transaction;
>> +   struct strbuf err = STRBUF_INIT;
>>
>> if (get_sha1(object_ref, object))
>> die("Failed to resolve '%s' as a valid ref.", object_ref);
>> @@ -157,11 +158,12 @@ static int replace_object(const char *object_ref, 
>> const char *replace_ref,
>> else if (!force)
>> die("replace ref '%s' already exists", ref);
>>
>> -   lock = lock_any_ref_for_update(ref, prev, 0, NULL);
>> -   if (!lock)
>> -   die("%s: cannot lock the ref", ref);
>> -   if (write_ref_sha1(lock, repl, NULL) < 0)
>> -   die("%s: cannot update the ref", ref);
>> +   transaction = ref_transaction_begin();
>> +   if (!transaction ||
>> +   ref_transaction_update(transaction, ref, repl, prev,
>> +  0, !is_null_sha1(prev)) ||
>> +   ref_transaction_commit(transaction, NULL, &err))
>> +   die(_("%s: failed to replace ref: %s"), ref, err.buf);
>
> Even though 'err' will be empty after this conditional, would
> strbuf_release(&err) here be warranted to future-proof it? Today's
> implementation of strbuf will not have allocated any memory, but
> perhaps a future change might pre-allocate (unlikely though that is),
> which would leak here.


Thanks.


I have no strong feelings either way.
A previous patch got a comment to remove a strbuf_release() because we
knew in that
code path that the string would be empty and thus the call was superfluous.

So one for and one against so far.
I will leave as is until there is more consensus.



>
>> return 0;
>>  }
>> --
>> 1.9.1.528.g98b8868.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] Sleep 1 millisecond in poll() to avoid busy wait

2014-04-29 Thread Junio C Hamano
Stepan Kasal  writes:

> Hello Junio,
>
> thank you for pointing out the problems.
>
> Let me explain the background:
>
> After some discussion a one line fix to win32/poll.c was accepted to 
> msysgit/git
> at 2012-05-16 https://github.com/msysgit/git/pull/7
>
> The description of the commit looked like this:
>> I played around with this [...]
>> [...]
>> I also tested the very fast local case, and didn't see any measurable
>> difference. On a big repo with 4500 files, the upload-pack took about 2
>> seconds with and without the fix.
> ... but there was no sign-off by Theodore.
>
> Because poll.c comes from Gnulib, it was reported there as well.
> Paolo Bonzini accepted the fix for poll.c and added a fix for select.
> The combined commit got this changelog entry:
>
>> 2012-05-21  Paolo Bonzini  
>> 
>> poll/select: prevent busy-waiting.  SwitchToThread() only gives away
>> the rest of the current time slice to another thread in the current
>> process. So if the thread that feeds the file decscriptor we're
>> polling is not in the current process, we get busy-waiting.
>> * lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().
>> Patch from Theodore Leblond.
>> * lib/select.c: Split polling out of the loop that sets the output
>> fd_sets.  Check for zero result and loop if the wait timeout is
>> infinite.
>
> The patch that I (Stepan) submitted as "v2" combines things like this:
> - subject by Theodore
> - first paragraph by Paolo, concise problem description
> - rest from Theodore's original commit ("I" = Theodore, I suppose)
> - diff exctly as in gnulib commit
>
> On Mon, Apr 28, 2014 at 11:58:50AM -0700, Junio C Hamano wrote:
>> Subject: compat/poll: sleep 1 millisecond to avoid busy wait
>
> Thanks for improving it.
>
>> Signed-off-by: Theodore Leblond 
>> Signed-off-by: Stepan Kasal 
>> Acked-by: Johannes Sixt 
>> Acked-by: Erik Faye-Lund 
>
> Sorry that I forgot to add my sign-off (Stepan).
> But I'm afraid I cannot add Theodore's, it was not in the original
> commit.  But it is one line change; the real work was the analysis.

Well, the original git/pull/7 in msysgit repository says that is a
patch by Theodore, so the kosher thing to do is to ask him (and I
see he is on the Cc list), and also ask msysgit folks (and I see
they are on the Cc list as well) to be a bit more careful when
responding to pull requests, especially given that it is our mutual
benefit to make sure we keep the changes between our two trees to
the minimum by upstreaming.

I'll queue without the forged sign-off by Theodore, as I think DCO
(1.1) (c) read loosely would let me do so ;-)

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 v4 15/27] fast-import.c: change update_branch to use ref transactions

2014-04-29 Thread Ronnie Sahlberg
On Mon, Apr 28, 2014 at 7:18 PM, Eric Sunshine  wrote:
> On Mon, Apr 28, 2014 at 6:54 PM, Ronnie Sahlberg  wrote:
>> Change update_branch() to use ref transactions for updates.
>>
>> Signed-off-by: Ronnie Sahlberg 
>> ---
>>  fast-import.c | 20 
>>  1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/fast-import.c b/fast-import.c
>> index fb4738d..300c8dc 100644
>> --- a/fast-import.c
>> +++ b/fast-import.c
>> @@ -1678,36 +1678,40 @@ found_entry:
>>  static int update_branch(struct branch *b)
>>  {
>> static const char *msg = "fast-import";
>> -   struct ref_lock *lock;
>> +   struct ref_transaction *transaction;
>> unsigned char old_sha1[20];
>> +   struct strbuf err = STRBUF_INIT;
>>
>> if (is_null_sha1(b->sha1))
>> return 0;
>> if (read_ref(b->name, old_sha1))
>> hashclr(old_sha1);
>> -   lock = lock_any_ref_for_update(b->name, old_sha1, 0, NULL);
>> -   if (!lock)
>> -   return error("Unable to lock %s", b->name);
>> if (!force_update && !is_null_sha1(old_sha1)) {
>> struct commit *old_cmit, *new_cmit;
>>
>> old_cmit = lookup_commit_reference_gently(old_sha1, 0);
>> new_cmit = lookup_commit_reference_gently(b->sha1, 0);
>> if (!old_cmit || !new_cmit) {
>> -   unlock_ref(lock);
>> return error("Branch %s is missing commits.", 
>> b->name);
>> }
>>
>> if (!in_merge_bases(old_cmit, new_cmit)) {
>> -   unlock_ref(lock);
>> warning("Not updating %s"
>> " (new tip %s does not contain %s)",
>> b->name, sha1_to_hex(b->sha1), 
>> sha1_to_hex(old_sha1));
>> return -1;
>> }
>> }
>> -   if (write_ref_sha1(lock, b->sha1, msg) < 0)
>> -   return error("Unable to update %s", b->name);
>> +   transaction = ref_transaction_begin();
>> +   if ((!transaction ||
>> +   ref_transaction_update(transaction, b->name, b->sha1, old_sha1,
>> +  0, 1)) ||
>> +   (ref_transaction_commit(transaction, msg, &err) &&
>> +!(transaction = NULL))) {
>> +   ref_transaction_rollback(transaction);
>> +   return error("Unable to update branch %s: %s", b->name,
>> +strbuf_detach(&err, NULL));
>
> Iffy strbuf handling. The strbuf content is being leaked here whether
> detached or not.
>

Thanks!

I have updated this and all other patches to make sure we do a
strbuf_release() any time we have
added to the string.

I also did a quick audit of the strbuf_detach() use in builtin/*
(which I based my use on)
and there seems to be quite common that strbuf_detach() is used in a
way that will leak memory.


I will make a note and perhaps audit all the other strbuf_detach()
calls for a future patch series.




>> +   }
>> return 0;
>>  }
>>
>> --
>> 1.9.1.528.g98b8868.dirty
--
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] PAGER_ENV: remove 'S' from $LESS by default

2014-04-29 Thread Junio C Hamano
Matthieu Moy  writes:

>> I also agree that droppage of S does not have to wait for that
>> topic.
>
> So, shall I rewrite my patch on top of master? (not hard, but there will
> be a minor conflict to resolve when merging with Peff's cooking series).

Sure, the one near the tip of 'pu' can even be dropped, especially
when nobody is actively looking at it, if it turns out to be too
much of a nuisance.

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] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-04-29 Thread Junio C Hamano
Jeff King  writes:

> This patch just adds a test to demonstrate the breakage.
> Some possible fixes are:
>
>   1. Tell everyone that NFD in the git repo is wrong, and
>  they should make a new commit to normalize all their
>  in-repo files to be precomposed.
>
>  This is probably not the right thing to do, because it
>  still doesn't fix checkouts of old history. And it
>  spreads the problem to people on byte-preserving
>  filesystems (like ext4), because now they have to start
>  precomposing their filenames as they are adde to git.

Hmm, have we taught the "compare precomposed" for codepaths that
compare two trees and a tree and the index, too?  Otherwise, we
would have the same issue with commits in the old history.

Do we have a similar issue for older commit in a history under
"ignore-case" as well?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 16/19] branch.c: use ref transaction for all ref updates

2014-04-29 Thread Junio C Hamano
Michael Haggerty  writes:

> I have no compunctions about using update() to create or delete a
> reference.  My point of view is that update() is the general case, and
> create() and delete() are special-cases that exist only for the
> convenience of callers.  For example, our future pluggable backends
> might only have to implement update(), and the other two functions could
> delegate to it at the abstract layer.

Sounds like a sensible position.

Thanks for commenting, Michael, and thanks for working on this,
Ronnie.
--
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] Uses git-credential for git-imap-send

2014-04-29 Thread Junio C Hamano
Jeff King  writes:

> On Mon, Apr 28, 2014 at 08:00:04PM -0700, Dan Albert wrote:
>
>> > I noticed that we are just filling in the password here, since we'll
>> > always fill cred.username from srvc->user. The lines directly above are:
>> > 
>> > if (!srvc->user) {
>> > fprintf(stderr, "Skipping server %s, no user\n", 
>> > srvc->host);
>> > goto bail;
>> > }
>> > 
>> > That comes from the imap.user config variable. I wonder if we should
>> > just pass it off to credential_fill() in this case, too, which will fill
>> > in the username if necessary.
>> [...]
>> 
>> Yeah, doubtful anyone cares, but it's simple enough to do.
>
> Thanks, I think the result looks good.
>
> Acked-by: Jeff King 

OK, luckily I haven't merged the one from the yesterday yet, so I'll
replace ;-)

Thanks for working on this, Dan, and as always thanks for reviewing,
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 17/32] read-cache: split-index mode

2014-04-29 Thread Junio C Hamano
Duy Nguyen  writes:

>> I do think it is sensible to keep two arrays of "struct cache_entry"
>> around (one for base and one for incremental changes) inside
>> index_state, and the patch seems to do so via "struct split_index"
>> that does have a copy of saved_cache.  If the write-out codepath
>> walks these two sorted arrays in parallel, shouldn't it be able to
>> figure out which entry is added, deleted and modified without
>> fattening this structure?
>
> So far without that "index" field I would have to resort to hasing
> entries in both arrays to find the shared paths. But ideas are
> welcome.

Hmm, why do you need to hash, when both arrays are sorted?  Wouldn't
it be just the matter of walking these two arrays in parallel,
with one scanning index for each array initialized to the beginning,
comparing the elements pointed by these indices, noting the side
that comes earlier in the sort order and advancing the index on that
side (or if they compare equal then advance both), ...?

--
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: Tagging a branch as "not fitted for branching" ?

2014-04-29 Thread Junio C Hamano
Jean-Noël Avila  writes:

>  In your daily management of the pu
> branch for git, do you have to use the -f flag a lot?

During the day I prepare and validate all the branches I am going to
publish, and at the end of the day, I run "git push" (no options)
with something like this in my .git/config:

[remote "origin"]
url = k.org:/pub/scm/git/git.git
fetch = +refs/heads/*:refs/remotes/origin/*
push = heads/master
push = heads/next
push = +heads/pu
push = heads/maint

I may be on any branch (not one of these four branches) when I need
to run "git push" before I ran out of the office to catch my bus, so
these explicit "which branches are to be pushed" configuration that
does not change what is pushed based on the current branch is
perfect match for *my* workflow.  I know 'pu' is always forced, so a
single "+" in front of only that one would allow me to rely on the
fast-forward safety for other branches to stop me from rewinding
them.

I could be also using the --force-with-lease support to validate
that the current value of 'pu' matches what I expect with versions
of Git post 1.8.5, but I happen to be the only person who publishes
there, so there is no need for an extra safety.

If it were not for +heads/pu thing, I could even have relied on the
"matching" mode, because these four branches are the only ones I
have there, and most of the local branches I have do not have any
reason to be on that remote repository.

I should caution that the use of "matching" mode or the explicit
"remote.*.push" specifications are not suitable for non-maintainer
workflows, though.  As most of the people are non-maintainers, we
are switching the default to upcoming Git 2.0 release.
--
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] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-04-29 Thread Jeff King
On Tue, Apr 29, 2014 at 10:12:52AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > This patch just adds a test to demonstrate the breakage.
> > Some possible fixes are:
> >
> >   1. Tell everyone that NFD in the git repo is wrong, and
> >  they should make a new commit to normalize all their
> >  in-repo files to be precomposed.
> >
> >  This is probably not the right thing to do, because it
> >  still doesn't fix checkouts of old history. And it
> >  spreads the problem to people on byte-preserving
> >  filesystems (like ext4), because now they have to start
> >  precomposing their filenames as they are adde to git.
> 
> Hmm, have we taught the "compare precomposed" for codepaths that
> compare two trees and a tree and the index, too?  Otherwise, we
> would have the same issue with commits in the old history.

Ugh, yeah, I didn't think about that codepath. I think we would not want
to precompose in that case. IOW, git works byte-wise internally, but it
is only at the filesystem layer that we do such munging. The index
straddles the line between the filesystem and git's internal
representations.

I think my "keep the normalized names alongside index entries" approach
might still work there. But it means that we compare against the "real"
byte-wise names on the tree side, and against the normalized names on
the path side. But that means having two comparison/lookup functions for
the index, and always using the right one. And algorithms that rely on
traversing two sorted lists cannot work in both directions.

> Do we have a similar issue for older commit in a history under
> "ignore-case" as well?

I don't think so, because we handle ignorecase completely differently.
There we use the name-hash with a case-insensitive hash and a
case-insensitive comparison function. And we use strcasecmp liberally
throughout the code.

I don't think we have a "str_utf8_cmp" that ignores normalizations (or
maybe strcoll will do this?). But in theory we could use it everywhere
we use strcasecmp for ignore_case. And then we would not need to have
our readdir wrapper, maybe? I admit I haven't thought that much about
_either_ approach. But aside from some bugs in the hash system, I do not
recall seeing any design problems in the ignorecase code.

-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: git subtree issue in more recent versions

2014-04-29 Thread Junio C Hamano
"Kevin Cagle (kcagle) [CONT - Type 2]"  writes:

> $ git subtree add -P oldGit https://github.com/git/git.git tags/v1.9.2
>
> Will produce this error:
>
> 10ff115f5c572299de4e04ade0d7adb3c75fbf1f is not a valid 'commit' object
>
> The bug isn't found in 1.7.1 (installed subtree manually) but is found in 
> 1.9.0 and 2.0.0.rc1. 
>
> It's related to the git fetch putting the "wrong" SHA1 in .git/FETCH_HEAD.

The change 7a2b128d is very much deliberate; we wanted not to lose
information that the user was trying to pull a tag not a commit,
because not unwrapping a tag to a commit it points at too early is
essential to allow pulling and merging a signed tag, which was
released as part of the 1.7.9 that happened in late January 2012
(whew, is it already more than two years ago?  time flies).

commit 7a2b128d13d880635e7317a9208cfa42a660f143
Author: Linus Torvalds 
Date:   Wed Nov 2 19:19:34 2011 -0700

fetch: do not store peeled tag object names in FETCH_HEAD

We do not want to record tags as parents of a merge when the user does
"git pull $there tag v1.0" to merge tagged commit, but that is not a good
enough excuse to peel the tag down to commit when storing in FETCH_HEAD.
The caller of underlying "git fetch $there tag v1.0" may have other uses
of information contained in v1.0 tag in mind.

If the caller of "fetch" (like the codepath in subtree) must see a
commit object, it needs to unwrap the tag itself in the new (eh, not
so new, though ;-) world order.
--
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] Document RUN_SETUP_GENTLY

2014-04-29 Thread Junio C Hamano
David Turner  writes:

> Document RUN_SETUP_GENTLY
>
> Signed-off-by: David Turner 
> ---
>  Documentation/technical/api-builtin.txt | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/technical/api-builtin.txt
> b/Documentation/technical/api-builtin.txt
> index e3d6e7a..1bbeda2 100644
> --- a/Documentation/technical/api-builtin.txt
> +++ b/Documentation/technical/api-builtin.txt
> @@ -28,6 +28,11 @@ where options is the bitwise-or of:
>   in a subdirectory.  If there is no work tree, no chdir() is
>   done.
>
> +`RUN_SETUP_GENTLY`::
> +
> + If there is a Git directory, chdir as per RUN_SETUP, otherwise,
> + don't create one and don't chdir.

I can understand "don't chdir" part, but where does "don't create"
come from?  Makes it sound as if non GENTLY version would create one
if there isn't there, but I am guessing that that is not what you
meant to say.

Puzzled.

--
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 12/27] replace.c: use the ref transaction functions for updates

2014-04-29 Thread Eric Sunshine
On Tue, Apr 29, 2014 at 12:18 PM, Ronnie Sahlberg  wrote:
> On Mon, Apr 28, 2014 at 5:44 PM, Eric Sunshine  
> wrote:
>> On Mon, Apr 28, 2014 at 6:54 PM, Ronnie Sahlberg  wrote:
>>> Update replace.c to use ref transactions for updates.
>>>
>>> Signed-off-by: Ronnie Sahlberg 
>>> ---
>>>  builtin/replace.c | 14 --
>>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/builtin/replace.c b/builtin/replace.c
>>> index b62420a..b037b29 100644
>>> --- a/builtin/replace.c
>>> +++ b/builtin/replace.c
>>> @@ -129,7 +129,8 @@ static int replace_object(const char *object_ref, const 
>>> char *replace_ref,
>>> unsigned char object[20], prev[20], repl[20];
>>> enum object_type obj_type, repl_type;
>>> char ref[PATH_MAX];
>>> -   struct ref_lock *lock;
>>> +   struct ref_transaction *transaction;
>>> +   struct strbuf err = STRBUF_INIT;
>>>
>>> if (get_sha1(object_ref, object))
>>> die("Failed to resolve '%s' as a valid ref.", object_ref);
>>> @@ -157,11 +158,12 @@ static int replace_object(const char *object_ref, 
>>> const char *replace_ref,
>>> else if (!force)
>>> die("replace ref '%s' already exists", ref);
>>>
>>> -   lock = lock_any_ref_for_update(ref, prev, 0, NULL);
>>> -   if (!lock)
>>> -   die("%s: cannot lock the ref", ref);
>>> -   if (write_ref_sha1(lock, repl, NULL) < 0)
>>> -   die("%s: cannot update the ref", ref);
>>> +   transaction = ref_transaction_begin();
>>> +   if (!transaction ||
>>> +   ref_transaction_update(transaction, ref, repl, prev,
>>> +  0, !is_null_sha1(prev)) ||
>>> +   ref_transaction_commit(transaction, NULL, &err))
>>> +   die(_("%s: failed to replace ref: %s"), ref, err.buf);
>>
>> Even though 'err' will be empty after this conditional, would
>> strbuf_release(&err) here be warranted to future-proof it? Today's
>> implementation of strbuf will not have allocated any memory, but
>> perhaps a future change might pre-allocate (unlikely though that is),
>> which would leak here.
>
> I have no strong feelings either way.
> A previous patch got a comment to remove a strbuf_release() because we
> knew in that
> code path that the string would be empty and thus the call was superfluous.

Indeed, I realized later that the reason I gave for possibly adding
the strbuf_release() was effectively bogus. Code throughout the
project ignores strbuff_release() when it is obvious that the strbuf
hasn't been used. Such code looks like this:

func() {
struct strbuf foo = STRBUF_INIT;
...code not touching 'foo'...
if (...)
return;
...code touching 'foo'...
strbuf_release(&foo);
}

At the point of early return, it's obvious at a glance that no content
has been added to 'foo'.

It was only a little while ago that I came to understand why the code
in this patch bothered me:

func() {
struct strbuf foo = STRBUF_INIT;
if (bar(&foo))
die(...);
...return or fall off end without releasing 'foo'...
}

The problem is that it's not so easy to see that it's safe to "leak"
the strbuf without detouring through the documentation of bar() and
possibly its implementation as well, before finally resuming the
reading of func(). That extra cost of having to study bar() will
likely be paid by most readers of func() who are concerned about the
missing strbuf_release(). The cognitive burden is greater.

> So one for and one against so far.
> I will leave as is until there is more consensus.

Fair enough. The fact that the variable is named 'err' in this case
might be enough to allow one to infer, without detouring through
bar(), that the missing strbuf_release() is intentional.
--
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] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-04-29 Thread Junio C Hamano
Jeff King  writes:

> I don't think we have a "str_utf8_cmp" that ignores normalizations (or
> maybe strcoll will do this?). But in theory we could use it everywhere
> we use strcasecmp for ignore_case. And then we would not need to have
> our readdir wrapper, maybe? I admit I haven't thought that much about
> _either_ approach. But aside from some bugs in the hash system, I do not
> recall seeing any design problems in the ignorecase code.

Our diffs and merges depend on walking two (or more) sorted lists,
and that sort order is baked in the tree objects when they are
created.  Using "normalized comparison" only when comparing the
earliest elements picked from these sorted lists would not give you
the correct comparison or merge results, would it?
--
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 19/19] refs.c: pass **transaction to commit and have it clear the pointer

2014-04-29 Thread Ronnie Sahlberg
On Tue, Apr 29, 2014 at 2:25 AM, Michael Haggerty  wrote:
> On 04/28/2014 07:59 PM, Ronnie Sahlberg wrote:
>> On Fri, Apr 25, 2014 at 6:31 PM, Michael Haggerty  
>> wrote:
>>> On 04/25/2014 06:14 PM, Ronnie Sahlberg wrote:
 Change ref_transaction_commit to take a pointer to a pointer for the
 transaction. This allows us to clear the transaction pointer from within
 ref_transaction_commit so that it becomes NULL in the caller.

 This makes transaction handling in the callers much nicer since instead of
 having to write horrible code like :
   t = ref_transaction_begin();
   if ((!t ||
   ref_transaction_update(t, refname, sha1, oldval, flags,
  !!oldval)) ||
   (ref_transaction_commit(t, action, &err) && !(t = NULL))) {
   ref_transaction_rollback(t);

 we can now just do the much nicer
   t = ref_transaction_begin();
   if (!t ||
   ref_transaction_update(t, refname, sha1, oldval, flags,
  !!oldval) ||
   ref_transaction_commit(&t, action, &err)) {
   ref_transaction_rollback(t);
>>>
>>> I understand the motivation for this change, but passing
>>> pointer-to-pointer is unconventional in a case like this.  Unfortunately
>>> I ran out of steam for the night before I could think about alternatives.
>>
>> I see.
>> Yes passing a pointer to pointer is not ideal.
>> But I still want to be able to use the pattern
>>t = ref_transaction_begin();
>>if (!t ||
>>ref_transaction_update(t, ...) ||
>>ref_transaction_commit(t, ...)) {
>>ref_transaction_rollback(t);
>>
>> Maybe the problem is that ref_transaction_commit() implicitely also
>> frees the transaction.
>>
>>
>> What about changing ref_transaction_commit() would NOT free the
>> transaction and thus a caller would
>> always have to explicitely free the transaction afterwards?
>>
>> Something like this :
>>t = ref_transaction_begin();
>>if (!t ||
>>ref_transaction_update(t, ...) ||
>>ref_transaction_commit(&t, ...)) {
>
> You wouldn't need the "&" here then, right?
>
>>ref_transaction_rollback(t);
>>}
>>ref_transaction_free(t);
>
> That sounds like a better solution.  We would want to make sure that
> ref_transaction_commit() / ref_transaction_rollback() leaves the
> ref_transaction in a state that if it is accidentally passed to
> ref_transaction_update() or its friends, the function calls die("BUG: ...").

Thanks!

Good idea.
I will add a transaction->status field that can track OPEN/CLOSED/ERROR
and die(BUG:...) appropriately in _commit/_create/_delete/_update if
it has the wrong value.


>
> Unless we want to make ref_transaction objects reusable.  But then we
> would need an explicit "allocation" step in the boilerplate code:
>
> t = ref_transaction_alloc();
> while (something) {
> if (ref_transaction_begin(t) ||
>  ref_transaction_update(t, ...) ||
>  ref_transaction_commit(t, ...)) {
> ref_transaction_rollback(t);
> }
> }
> ref_transaction_free(t);
>
> Note that ref_transaction_begin() should in this case be converted to
> return 0-on-OK, negative-on-error for consistency.
>
> This would bring us back to the familiar pattern alloc...use...free.
>
> I was going to say that the extra boilerplate is not worth it, and
> anyway reusing ref_transaction objects won't save any significant work.
>  But then it occurred to me that ref_transaction_alloc() might be a
> place to do more expensive work, like creating a connection to a
> database, so reuse could potentially be a bigger win.

ACK, but I don't think we need reusable transaction yet. Once the API
is cleaned up
it should be reasonably easy to add in the future if we see a need for it.
Sounds reasonable to you ?


>
> All in all, either way is OK with me.
>
> Michael
>
> --
> Michael Haggerty
> mhag...@alum.mit.edu
> http://softwareswirl.blogspot.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


Re: [PATCH v4 2/2] test/send-email: to-cover, cc-cover tests

2014-04-29 Thread Junio C Hamano
"Michael S. Tsirkin"  writes:

> Add tests for the new feature.
>
> Signed-off-by: Michael S. Tsirkin 
> ---
>  t/t9001-send-email.sh | 45 +
>  1 file changed, 45 insertions(+)
>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 1ecdacb..97cc094 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1334,6 +1334,51 @@ test_expect_success $PREREQ '--force sends cover 
> letter template anyway' '
>   test -n "$(ls msgtxt*)"
>  '
>  
> +test_cover_addresses () {
> + header="$1"
> + shift
> + clean_fake_sendmail &&
> + rm -fr outdir &&
> + git format-patch --cover-letter -2 -o outdir &&
> + cover=`echo outdir/-*.patch` &&
> + mv $cover cover-to-edit.patch &&
> + sed "s/^From:/$header: ex...@address.com\nFrom:/" cover-to-edit.patch > 
> $cover &&

Please do the redirection like this:

sed "s/^From:/$header: ex...@address.com\nFrom:/" cover-to-edit.patch 
>"$cover" &&

in your later patches (I'll tweak this patch myself, so no need to
resend).  We know >$cover should be the same as >"$cover", but it
was reported that some version of bash does not know it and
complains instead (see Documentation/CodingGuidelines).
--
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


Bug: Case-insensitive filesystems can cause merge and checkout problems

2014-04-29 Thread David Turner
By default, git sets core.ignorecase=true when git init or git clone
is run on a machine with a case-insensitive filesystem.  Here's a
test-case for some problems that this causes:

git checkout master
touch TestCase
git add TestCase
git commit -m 'add TestCase'
git checkout -b with-camel
touch foo
git add foo
git commit -m 'intervening commit'
git checkout master
git rm TestCase
touch testcase
git add testcase
git commit -m 'rename to testcase'
git checkout with-camel
git merge master -m 'merge'

One would expect a clean working copy at this point, but in fact, the
file 'testcase' will be deleted.

With core.ignorecase=false, we get a different failure.  The
penultimate command fails with:
$ git checkout with-camel
error: The following untracked working tree files would be overwritten
by checkout:
TestCase
Please move or remove them before you can switch branches.
Aborting

Of course, there is no untracked working tree file by that name; there
is a tracked working tree file named testcase (all-lowercase).
--
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 2/2] test/send-email: to-cover, cc-cover tests

2014-04-29 Thread Michael S. Tsirkin
On Tue, Apr 29, 2014 at 12:01:10PM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin"  writes:
> 
> > Add tests for the new feature.
> >
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  t/t9001-send-email.sh | 45 +
> >  1 file changed, 45 insertions(+)
> >
> > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> > index 1ecdacb..97cc094 100755
> > --- a/t/t9001-send-email.sh
> > +++ b/t/t9001-send-email.sh
> > @@ -1334,6 +1334,51 @@ test_expect_success $PREREQ '--force sends cover 
> > letter template anyway' '
> > test -n "$(ls msgtxt*)"
> >  '
> >  
> > +test_cover_addresses () {
> > +   header="$1"
> > +   shift
> > +   clean_fake_sendmail &&
> > +   rm -fr outdir &&
> > +   git format-patch --cover-letter -2 -o outdir &&
> > +   cover=`echo outdir/-*.patch` &&
> > +   mv $cover cover-to-edit.patch &&
> > +   sed "s/^From:/$header: ex...@address.com\nFrom:/" cover-to-edit.patch > 
> > $cover &&
> 
> Please do the redirection like this:
> 
>   sed "s/^From:/$header: ex...@address.com\nFrom:/" cover-to-edit.patch 
> >"$cover" &&
> 
> in your later patches (I'll tweak this patch myself, so no need to
> resend).  We know >$cover should be the same as >"$cover", but it
> was reported that some version of bash does not know it and
> complains instead (see Documentation/CodingGuidelines).

I'll try to remember this, thanks.

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


[PATCH v2] Document RUN_SETUP_GENTLY

2014-04-29 Thread David Turner
Sorry about that -- the documentation of RUN_SETUP confused me.  So I
have a new patch that edits that as well.

--
RUN_SETUP_GENTLY and improve RUN_SETUP docs

Signed-off-by: David Turner 
---
 Documentation/technical/api-builtin.txt | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/technical/api-builtin.txt
b/Documentation/technical/api-builtin.txt
index e3d6e7a..b250c1a 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -23,10 +23,15 @@ where options is the bitwise-or of:

 `RUN_SETUP`::

- Make sure there is a Git directory to work on, and if there is a
- work tree, chdir to the top of it if the command was invoked
- in a subdirectory.  If there is no work tree, no chdir() is
- done.
+ If there is not a Git directory to work on, abort.  If there
+ is a work tree, chdir to the top of it if the command was
+ invoked in a subdirectory.  If there is no work tree, no
+ chdir() is done.
+
+`RUN_SETUP_GENTLY`::
+
+ If there is a Git directory, chdir as per RUN_SETUP, otherwise,
+ don't chdir anywhere.

 `USE_PAGER`::

-- 


On Tue, Apr 29, 2014 at 2:25 PM, Junio C Hamano  wrote:
> David Turner  writes:
>
>> Document RUN_SETUP_GENTLY
>>
>> Signed-off-by: David Turner 
>> ---
>>  Documentation/technical/api-builtin.txt | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/technical/api-builtin.txt
>> b/Documentation/technical/api-builtin.txt
>> index e3d6e7a..1bbeda2 100644
>> --- a/Documentation/technical/api-builtin.txt
>> +++ b/Documentation/technical/api-builtin.txt
>> @@ -28,6 +28,11 @@ where options is the bitwise-or of:
>>   in a subdirectory.  If there is no work tree, no chdir() is
>>   done.
>>
>> +`RUN_SETUP_GENTLY`::
>> +
>> + If there is a Git directory, chdir as per RUN_SETUP, otherwise,
>> + don't create one and don't chdir.
>
> I can understand "don't chdir" part, but where does "don't create"
> come from?  Makes it sound as if non GENTLY version would create one
> if there isn't there, but I am guessing that that is not what you
> meant to say.
>
> Puzzled.
>
--
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] t3910: show failure of core.precomposeunicode with decomposed filenames

2014-04-29 Thread Jeff King
On Tue, Apr 29, 2014 at 11:49:30AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I don't think we have a "str_utf8_cmp" that ignores normalizations (or
> > maybe strcoll will do this?). But in theory we could use it everywhere
> > we use strcasecmp for ignore_case. And then we would not need to have
> > our readdir wrapper, maybe? I admit I haven't thought that much about
> > _either_ approach. But aside from some bugs in the hash system, I do not
> > recall seeing any design problems in the ignorecase code.
> 
> Our diffs and merges depend on walking two (or more) sorted lists,
> and that sort order is baked in the tree objects when they are
> created.  Using "normalized comparison" only when comparing the
> earliest elements picked from these sorted lists would not give you
> the correct comparison or merge results, would it?

Right, but we do not do normalized comparisons on that side. Not for
precompose, and not for ignorecase. The entry in the index _is_
case-sensitive[1], and we compare it as such to the tree side.

It is only when comparing the filesystem side to the index that we need
to care about such normalizations. And there we have the name-hash code
to handle ignorecase, but nothing to handle precompose.

-Peff

[1] This works because you typically get the case-sensitive entry via
`git read-tree`, and then further update it from the filesystem. If
you were to add a new entry "makefile", and somebody else added
"Makefile", they would conflict.

When you do "git add makefile" and "Makefile" _does_ exist, I am not
sure, though, if it is git who handles making sure we find the
correct entry, or if it is simply the fact that case insensitive
filesystems are typically case-preserving (so you generally ask for
"Makefile" anyway). If it is the latter, then that is a problem for
precompose. HFS's NFD normalization is non-preserving.
--
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 24/32] split-index: strip pathname of on-disk replaced entries

2014-04-29 Thread Junio C Hamano
This triggers "saved_namelen may be used uninitialized" for me, even
though it looks clear that it is used under CE_STRIP_NAME and it is
assigned under that condition.  Sigh to a stupid compiler...
--
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: Recording the current branch on each commit?

2014-04-29 Thread James Denholm
On 29 April 2014 23:31:29 GMT+10:00, Felipe Contreras 
 wrote:
>James Denholm wrote:
>> So that we can all have egg on our faces when it takes off and is
>> proven superior... Right?
>
>I don't know what you mean by "we", but it certainly doesn't include
>you.
>
>  % git log --author=nod.h...@gmail.com master
>  empty

Sure it does. I didn't (and don't) have any impact on the
debate and resulting community views, but I recall recently
that I prescribed to the arguments that default aliases
are a bad idea. I'm not arrogant enough to suggest that
my views matter at this point, but if git-fc is proven superior
by community adoption, I would be as wrong as anyone else
who held that view.

So I'll ask again - you've described frustration at the
pace of git development, and that you feel that your patches
aren't being accepted. If you feel that this is ultimately to the
fatal detriment of git, why are you still trying to convince
vegetarians to join you in hunting when you could simply find
a more willing group?

Regards,
James Denholm.

--
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 00/27] Use ref transactions for all ref updates

2014-04-29 Thread Junio C Hamano
Ronnie Sahlberg  writes:

> This patch series is based on mhagger/ref-transactions and expands on the
> transaction API. It converts all external (outside of refs.c) callers to
> use the transaction API for any writes.
> This makes most of the ref updates to become atomic when there are failures
> locking or writing to a ref.

I think I saw some more comments to be addressed on this round, but
I'll try to push it through into 'pu' by attempting to resolve
conflicts so that I can ask you to eyeball the result.  This series
seem to conflict with your own rs/ref-update-check-errors-early
topic that is already in 'next'.

If I screwed up the conflict resolution (which is very possible), I
may have to ask you to rebase this series on top of acc62aa (which
is the merge of rs/ref-update-check-errors-early to 'next'), but I
am hoping that conflict resolution I'll push out today will be
correct.  We'll see.

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 v11 00/11] Add interpret-trailers builtin

2014-04-29 Thread Junio C Hamano
Thanks and sorry for taking a bit longer than usual; will push this
series out, replacing the previous round, when I am done for today's
integration cycle.

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


Re: [PATCH v1 0/4] replace: add option to edit a Git object

2014-04-29 Thread Junio C Hamano
Jeff King  writes:

> On Sat, Apr 26, 2014 at 10:00:53PM +0200, Christian Couder wrote:
>
>> This patch series comes from what Peff sent in the following thread:
>> 
>> http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528
>
> Thanks. As I recall, these were in pretty good shape, and I just read
> over them again and didn't see anything wrong.
>
>> I added the following fixes:
>> 
>> - add "strbuf_release(&result);" in import_object(); this was suggested
>>   by Eric Sunshine
>> - use MODE_LIST instead of MODE_DELETE if no arguments are passed; this
>>   makes the test suite pass
>> - add "--no-replace-objects" when calling "git cat-file" in export_object();
>>   so that we edit the original object if an object is already replaced
>
> All sensible, I think.
>
>> I am not happy with the fact that if the user doesn't modify the object when
>> editing it, then a replace ref can still be created that points to the
>> original object. I think something should be done to avoid that.
>
> Yeah, it should be easy to just hashcmp the sha1s after calling
> import_object. In fact, I think we can just erase any existing replace
> ref in that case (the user might have started with a replace ref and
> converted it _back_ to the original object, for example).
>
>> Once that is fixed, I plan to add some tests and documentation, but I wanted
>> first to let you know that I am looking at this.
>
> Great. Thanks for working on this.
>
> -Peff

Thanks.  In the meantime, I'll queue these as-is and push the result
out.
--
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 v6 1/5] patch-id: make it stable against hunk reordering

2014-04-29 Thread Junio C Hamano
Thanks.

I'll revert the merge of the previous round to 'next' and then queue
this series instead.


--
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: Recording the current branch on each commit?

2014-04-29 Thread Piotr Krukowiecki
On Tue, Apr 29, 2014 at 12:17 PM, Felipe Contreras
 wrote:
> That's all you could list for *four* years? None of that would even be noticed
> by most of our users, maybe push.default (when it actually happens), but 
> that's
> *one*.
>
> *One* important change in *four* years.

Hi,

one out of how many? (How many proposed important changes were rejected?)

Thanks,
-- 
Piotr Krukowiecki
--
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 v5 1/6] pull: rename pull.rename to pull.mode

2014-04-29 Thread Philip Oakley

From: "Felipe Contreras" 

Also 'branch..rebase' to 'branch..pullmode'.


Sorry I haven't commented earlier. Because the 0/6 explanation isn't a
commit, a few extra words would be useful to capture what the 0/6 cover
letter said to start the patch series cleanly/clearly e.g. start with

   Begin the "Reject non-ff pulls by default" process by creating new
   config variables which will allow extra options, to replace the old 
pull

   configuration options.

I didn't immediately grasp why the 'replacement' was happening, rather
than it being a creation and a deprecation.



This way 'pull.mode' can be set to 'merge', and the default can be
something else.

The old configurations still work, but get deprecated.

Signed-off-by: Felipe Contreras 
---
Documentation/config.txt   | 34 +-
Documentation/git-pull.txt |  2 +-
branch.c   |  4 ++--
builtin/remote.c   | 14 --
git-pull.sh| 39
+--
t/t3200-branch.sh  | 40

t/t5601-clone.sh   |  4 ++--
7 files changed, 91 insertions(+), 46 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c26a7c8..5978d35 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -708,7 +708,7 @@ branch.autosetupmerge::
branch.autosetuprebase::
 When a new branch is created with 'git branch' or 'git checkout'
 that tracks another branch, this variable tells Git to set
- up pull to rebase instead of merge (see "branch..rebase").
+ up pull to rebase instead of merge (see "branch..pullmode").
 When `never`, rebase is never automatically set to true.
 When `local`, rebase is set to true for tracked branches of
 other local branches.
@@ -764,15 +764,15 @@ branch..mergeoptions::
 option values containing whitespace characters are currently not
 supported.

-branch..rebase::
- When true, rebase the branch  on top of the fetched branch,
- instead of merging the default branch from the default remote when
- "git pull" is run. See "pull.rebase" for doing this in a non
- branch-specific manner.
+branch..pullmode::
+ When "git pull" is run, this determines if it would either merge or
+ rebase the fetched branch. The possible values are 'merge',
+ 'rebase', and 'rebase-preserve'. See "pull.mode" for doing this in a
+ non branch-specific manner.


I'd think it useful to add that:
   branch..rebase is deprecated.
given the large amount of internet cruft about this older config
variable name


+
- When preserve, also pass `--preserve-merges` along to 'git rebase'
- so that locally committed merge commits will not be flattened
- by running 'git pull'.
+ When 'rebase-preserve', also pass `--preserve-merges` along to
+ 'git rebase' so that locally committed merge commits will not be
+ flattened by running 'git pull'.
+
*NOTE*: this is a possibly dangerous operation; do *not* use
it unless you understand the implications (see linkgit:git-rebase[1]
@@ -1881,15 +1881,15 @@ pretty.::
 Note that an alias with the same name as a built-in format
 will be silently ignored.

-pull.rebase::
- When true, rebase branches on top of the fetched branch, instead
- of merging the default branch from the default remote when "git
- pull" is run. See "branch..rebase" for setting this on a
- per-branch basis.
+pull.mode::
+ When "git pull" is run, this determines if it would either merge or
+ rebase the fetched branch. The possible values are 'merge',
+ 'rebase', and 'rebase-preserve'. See "branch..pullmode" for
doing
+ this in a non branch-specific manner.


Add?:
   pull.rebase is deprecated.


+
- When preserve, also pass `--preserve-merges` along to 'git rebase'
- so that locally committed merge commits will not be flattened
- by running 'git pull'.
+ When 'rebase-preserve', also pass `--preserve-merges` along to
+ 'git rebase' so that locally committed merge commits will not be
+ flattened by running 'git pull'.
+
*NOTE*: this is a possibly dangerous operation; do *not* use
it unless you understand the implications (see linkgit:git-rebase[1]
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 200eb22..9a91b9f 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -117,7 +117,7 @@ locally created merge commits will not be
flattened.
+
When false, merge the current branch into the upstream branch.
+
-See `pull.rebase`, `branch..rebase` and
`branch.autosetuprebase` in
+See `pull.mode`, `branch..pullmode` and
`branch.autosetuprebase` in
linkgit:git-config[1] if you want to make `git pull` always use
`--rebase` instead of merging.
+
diff --git a/branch.c b/branch.c
index 723a36b..63ce671 100644
--- a/branch.c
+++ b/branch.c
@@ -71,8 +71,8 @@ void install_branch_config(int flag, const char
*local, const char *origin, cons

 if (rebasing) {
 strbuf_reset(&key);
- strbuf_addf(&key, "branch.%s.rebase", local);
- git_config_set(key.buf, "true");
+ strbuf_addf(&key, "bra

Re: Recording the current branch on each commit?

2014-04-29 Thread Felipe Contreras
James Denholm wrote:
> On 29 April 2014 23:31:29 GMT+10:00, Felipe Contreras 
>  wrote:
> >James Denholm wrote:
> >> So that we can all have egg on our faces when it takes off and is
> >> proven superior... Right?
> >
> >I don't know what you mean by "we", but it certainly doesn't include
> >you.
> >
> >  % git log --author=nod.h...@gmail.com master
> >  empty
> 
> Sure it does.

No it doesn't. Unless you have some credentials in the Git community,
which come after several contributions, your opinion carries no weight
at all. This might not be ideal, but that's the way it is.

You have no credentials, your opinion doesn't count. You are not part of
the "we" you referred before.

> So I'll ask again - you've described frustration at the
> pace of git development, and that you feel that your patches
> aren't being accepted. If you feel that this is ultimately to the
> fatal detriment of git, why are you still trying to convince
> vegetarians to join you in hunting when you could simply find
> a more willing group?

If by convince you mean apply my patches, my patches are still getting
applied [1].

Either way your analogy is completely wrong as I already explained
multiple times. I'm not trying to convince vegetarians to go hunting,
I'm saying they should eat something, bread, meat, vegetables, anything.
Instead they choose to starve to death.

And I'm done discussing with you. Your comments are content-free.

[1] https://www.ohloh.net/p/git/contributors?sort=latest_commit

-- 
Felipe Contreras
--
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: Recording the current branch on each commit?

2014-04-29 Thread David Lang

On Mon, 28 Apr 2014, David Kastrup wrote:


Jeremy Morton  writes:


On 28/04/2014 10:02, David Kastrup wrote:

Jeremy Morton  writes:


On 28/04/2014 09:32, Felipe Contreras wrote:

some people to is to always merge with --no-ff, that way you see the branch
name in the merge commit.


But surely, it's recommended with Git that you try to avoid doing
--no-ff merges to avoid commit noise?


Nope. Different people have different needs, there's no recommendation. If
anything, the recommendation is to do a ff merge, because that's the default.


That's what I'm saying.  With an ff merge, you don't get the merge
commit message telling you the branch name.


And I don't _want_ that branch name to be recorded.  The whole point of
a distributed version control system is that it's nobody else's business
how I organize my work before submitting it.


Well it would be optional, so obviously you wouldn't be forced to
share the branch name.  It's not like we're trying to "pry in" to your
private development.  It's a way of choosing to share what you may
consider to be useful contextual information about the commit.


It sounds like what you want is really a template for a commit message that lets 
you include arbitrary information in that template, including things like branch 
name that may not make sense for other people.


If there is no commit message, populate the template and show that to the user 
in their editor.


If there is a commit message, don't touch it.

Then people can use whatever they want (including environment variables) as part 
of their messages.


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


[PATCH 1/3] config: avoid yoda conditions

2014-04-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras 
---
 config.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/config.c b/config.c
index a30cb5c..bd69ad7 100644
--- a/config.c
+++ b/config.c
@@ -616,7 +616,7 @@ static int git_config_maybe_bool_text(const char *name, 
const char *value)
 int git_config_maybe_bool(const char *name, const char *value)
 {
int v = git_config_maybe_bool_text(name, value);
-   if (0 <= v)
+   if (v >= 0)
return v;
if (git_parse_int(value, &v))
return !!v;
@@ -626,7 +626,7 @@ int git_config_maybe_bool(const char *name, const char 
*value)
 int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 {
int v = git_config_maybe_bool_text(name, value);
-   if (0 <= v) {
+   if (v >= 0) {
*is_bool = 1;
return v;
}
-- 
1.9.2+fc1.3.gade8541

--
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 0/3] Trivial cleanups

2014-04-29 Thread Felipe Contreras
Felipe Contreras (3):
  config: avoid yoda conditions
  add: avoid yoda condition
  add: remove dead code

 builtin/add.c | 6 +-
 config.c  | 4 ++--
 2 files changed, 3 insertions(+), 7 deletions(-)

-- 
1.9.2+fc1.3.gade8541

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


[PATCH 3/3] add: remove dead code

2014-04-29 Thread Felipe Contreras
addremove is already 1 by default.

Signed-off-by: Felipe Contreras 
---
 builtin/add.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index ac10bab..980e247 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -329,10 +329,6 @@ int cmd_add(int argc, const char **argv, const char 
*prefix)
if (addremove && take_worktree_changes)
die(_("-A and -u are mutually incompatible"));
 
-   if (!take_worktree_changes && addremove_explicit < 0 && argc)
-   /* Turn "git add pathspec..." to "git add -A pathspec..." */
-   addremove = 1;
-
if (!show_only && ignore_missing)
die(_("Option --ignore-missing can only be used together with 
--dry-run"));
 
-- 
1.9.2+fc1.3.gade8541

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


[PATCH 2/3] add: avoid yoda condition

2014-04-29 Thread Felipe Contreras
18 is younger than person's age.

Signed-off-by: Felipe Contreras 
---
 builtin/add.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/add.c b/builtin/add.c
index 459208a..ac10bab 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -321,7 +321,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
argc--;
argv++;
 
-   if (0 <= addremove_explicit)
+   if (addremove_explicit >= 0)
addremove = addremove_explicit;
else if (take_worktree_changes && ADDREMOVE_DEFAULT)
addremove = 0; /* "-u" was given but not "-A" */
-- 
1.9.2+fc1.3.gade8541

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


Re: git-svn Rewrites Some Commits, but not All

2014-04-29 Thread Piotr Krukowiecki
On Mon, Apr 28, 2014 at 9:26 PM, Aaron Laws  wrote:
> The way I understand it, when `git svn dcommit` is run, new commits
> are created (A' is created from A adding SVN information), then the
> current branch is moved to point to A'. Why don't we move any other
> refs that were pointing to A over to A' ? What would be the point of
> continuing to point to A? I'm interested in looking into coding this
> change to git-svn, but I would like to hear some feedback first.

Hi,

I think A' might not always be simply (A + SVN info). I think you can
dcommit when you're not up to date.  So A' will have a different
parent than A (will be automatically rebased on top of current branch
tip). Other refs pointing to A might be used as bookmarks, and moving
them from A to A' would be a significant change.

-- 
Piotr Krukowiecki
--
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 8/8] remote-hg: trivial cleanups

2014-04-29 Thread Philip Oakley

From: "Felipe Contreras" 
Subject: [PATCH 8/8] remote-hg: trivial cleanups

It's useful, as a reader of the shortlog and email message subject 
lines, to know what sort of triviality is being tidied.
Usually they are 'spelling' or 'variable naming' or some such that can 
easily be squeezed on the end without breaking the ~50 char guide. In 
this case there were two types, so I ended up with this suggestion for 
the message heading


   remote-hg: cleanup 40*{0} string, and de-dup tests

which adds just enough (for me) to get a feel for the style of what's 
inside


regards

Philip


Cleanup 51be46e (remote-hg: do not fail on invalid bookmarks).

Having a 40-characters string is not ideal, and having three tests for
basically the same relatively rare situation is overkill.

Signed-off-by: Felipe Contreras 
---
git-remote-hg.py |  2 +-
t/t5810-remote-hg.sh | 56 


2 files changed, 5 insertions(+), 53 deletions(-)

diff --git a/git-remote-hg.py b/git-remote-hg.py
index 402b92f..74f2a2e 100755
--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -677,7 +677,7 @@ def do_list(parser):
print "? refs/heads/branches/%s" % gitref(branch)

for bmark in bmarks:
-if  bmarks[bmark].hex() == 
'':

+if  bmarks[bmark].hex() == '0' * 40:
warn("Ignoring invalid bookmark '%s'", bmark)
else:
print "? refs/heads/%s" % gitref(bmark)
diff --git a/t/t5810-remote-hg.sh b/t/t5810-remote-hg.sh
index ba8b2d8..9946f57 100755
--- a/t/t5810-remote-hg.sh
+++ b/t/t5810-remote-hg.sh
@@ -772,7 +772,7 @@ test_expect_success 'remote double failed push' '
 )
'

-test_expect_success 'clone remote with master null bookmark, then 
push to the bookmark' '

+test_expect_success 'clone remote with null bookmark, then push' '
 test_when_finished "rm -rf gitrepo* hgrepo*" &&

 hg init hgrepo &&
@@ -781,67 +781,19 @@ test_expect_success 'clone remote with master 
null bookmark, then push to the bo

 echo a >a &&
 hg add a &&
 hg commit -m a &&
- hg bookmark -r null master
+ hg bookmark -r null bookmark
 ) &&

 git clone "hg::hgrepo" gitrepo &&
 check gitrepo HEAD a &&
 (
 cd gitrepo &&
- git checkout --quiet -b master &&
- echo b >b &&
- git add b &&
- git commit -m b &&
- git push origin master
- )
-'
-
-test_expect_success 'clone remote with default null bookmark, then 
push to the bookmark' '

- test_when_finished "rm -rf gitrepo* hgrepo*" &&
-
- hg init hgrepo &&
- (
- cd hgrepo &&
- echo a >a &&
- hg add a &&
- hg commit -m a &&
- hg bookmark -r null -f default
- ) &&
-
- git clone "hg::hgrepo" gitrepo &&
- check gitrepo HEAD a &&
- (
- cd gitrepo &&
- git checkout --quiet -b default &&
- echo b >b &&
- git add b &&
- git commit -m b &&
- git push origin default
- )
-'
-
-test_expect_success 'clone remote with generic null bookmark, then 
push to the bookmark' '

- test_when_finished "rm -rf gitrepo* hgrepo*" &&
-
- hg init hgrepo &&
- (
- cd hgrepo &&
- echo a >a &&
- hg add a &&
- hg commit -m a &&
- hg bookmark -r null bmark
- ) &&
-
- git clone "hg::hgrepo" gitrepo &&
- check gitrepo HEAD a &&
- (
- cd gitrepo &&
- git checkout --quiet -b bmark &&
+ git checkout --quiet -b bookmark &&
 git remote -v &&
 echo b >b &&
 git add b &&
 git commit -m b &&
- git push origin bmark
+ git push origin bookmark
 )
'

--
1.9.2+fc1.3.gade8541

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



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


  1   2   >