[PATCH v4 2/2] test/send-email: to-cover, cc-cover tests
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" ?
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
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
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
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
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?
- 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?
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?
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
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
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
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
-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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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?
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?
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?
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
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?
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?
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
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?
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'
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
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
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
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
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
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
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
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'
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?
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?
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
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
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?
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
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
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'
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'
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?
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
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
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
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
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
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
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
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
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
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
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
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" ?
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
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
"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
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
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
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
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
"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
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
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
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
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
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?
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
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
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
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
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?
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
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?
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?
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
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
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
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
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
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
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