Re: [PATCH 2/4] revisions passed to cherry-pick should be in default order
On Mon, Aug 13, 2012 at 2:05 PM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: To connect to the other mail I sent on this thread (in parallel with yours), do you think git cherrry-pick HEAD HEAD~1 should apply the commits in the same order as git cherry-pick HEAD~2..HEAD (which would give the same result if passed to 'rev-list --no-walk' for a linear history) or in the order specified on the command line? Definitely the latter; I do not think of any semi-reasonable excuse to do otherwise. Indeed. My patches tried to fix the wrong problem. Sorry I'm slow, but I think I'm finally starting to understand what you've been saying all along about the bug being in sequencer. I'll try to recapitulate a bit for my own and maybe others' understanding. For simplicity, let's assume a linear history with unique timestamps, but not necessarily increasing with each commit. Currently: 1) 'git cherry-pick A..C' picks the commits order in reverse default order 2) 'git cherry-pick B C' picks the commits in chronological order 3) 'git rev-list --reverse A..C | git cherry-pick --stdin' behaves just like 'git cherry-pick B C' and therefore picks the commits in chronological order In cases 2) and 3), even though cherry-pick tells the revision walker not to walk, it still sorts the commits in reverse chronological order. But cherry-pick also tells the revision walker explicitly to reverse the list, so in the end, the order is chronological. In case 2), however, the first ordering make no difference in this limited case (IIUC). So the default ordering (which would be C, then B in this case, regardless of timestamps), gets reversed and B gets applied first, followed by C. So all of the above case give the right result in the end as long as the timestamps are chronological, and case 1) gives the right result regardless. The other two cases only works in most cases because the unexpcted sorting when no-walk is in effect counteracts the final reversal. When I noticed that the order of inputs to cases 2) and 3) above was ignored, and thinking that 'git rev-list A..C | git cherry-pick --stdin' should mimic 'git cherry-pick A..C', I incorrectly thought that the error was the use of --reverse to 'git rev-list' as well as the sorting done in the no-walk case. I think completely ignored case 2) at this point. I now think I understand that the sorting done in the no-walk case is indeed incorrect, but that the --reverse passed to rev-list is correct. Instead, the final reversal, which is currently unconditional, should not be done in the no-walk case. IIUC, this could be implemented by making cherry-pick iterate over rev_info.pending.objects just like 'git show' does when not walking. Junio, I think it makes sense to just drop this whole series for now. I'll probably include patch 1/4 in my stalled rebase-range series instead. If I understood you correctly, you didn't have any objections to that patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Your branch and 'origin/master' have diverged
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@student.ethz.ch writes:
In some sense this is a really bad case of wrong UI design, because we
(this happens on #git a lot) have to teach users not to use the command
so they won't trip over this problem. It would be better to fix the
real issue instead. IIRC it was even on the 1.8.0 wishlist...
Is it?
There already is a way to ask it to update the single tracking
branch while fetching; git fetch origin master that
unconditionally updates refs/remotes/origin/master without a way to
tell it not to do so will be a grave usability regression.
Grave? Do you have any data/use-cases to back that up with?
I have never had a need for a fetch that doesn't update the remote
namespace, nor heard anyone on IRC who has. OTOH, I do have anecdotal
evidence in support of the current state is confusing: this thread, or
the fact that Jan's IRC bot grew bot-quotes !fetch4/!pull4 that people
use to warn users of 'git pull origin master' (it's apparently very
common).
The 1.8.0 thread is here, and Peff even said he had a patch he uses in
his tree:
http://thread.gmane.org/gmane.comp.version-control.git/165720/focus=165758
There's even a newer thread suggesting the same:
http://thread.gmane.org/gmane.comp.version-control.git/192252
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line 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 - discard hunk in add --patch mode
Hi, I frequently stage files using git add --patch command and I almost always come across debug code I want to discard, but there is no option for that in the prompt. The result is that I have to run an extra command after the dialogue ends. I would like to add a feature to allow users to discard hunks using a command like r! or d! I was wondering if that would be a welcome addition to git. I am willing to work on the feature myself. Regards, Mina -- To unsubscribe from this list: send the line 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/RFC v3 02/16] Integrate remote-svn into svn-fe/Makefile.
On Tuesday 14 August 2012 13:14:12 Junio C Hamano wrote: Florian Achleitner florian.achleitner.2.6...@gmail.com writes: Requires some sha.h to be used and the libraries to be linked, this is currently hardcoded. Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com --- contrib/svn-fe/Makefile | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile index 360d8da..8f0eec2 100644 --- a/contrib/svn-fe/Makefile +++ b/contrib/svn-fe/Makefile @@ -1,14 +1,14 @@ -all:: svn-fe$X +all:: svn-fe$X remote-svn$X CC = gcc RM = rm -f MV = mv -CFLAGS = -g -O2 -Wall +CFLAGS = -g -O2 -Wall -DSHA1_HEADER='openssl/sha.h' -Wdeclaration-after-statement LDFLAGS = ALL_CFLAGS = $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) -EXTLIBS = +EXTLIBS = -lssl -lcrypto -lpthread ../../xdiff/lib.a I haven't looked carefully, but didn't we have to do a bit more elaborate when linking with ssl/crypto in our main Makefile to be portable across various vintages of OpenSSL libraries? Does contrib/svn-fe/ already depend on OpenSSL by the way? It needs to be documented somewhere in the same directory. If one builds the main Git binary with NO_OPENSSL, can this still be built and linked? What does this use xdiff/lib.a for? The above are just mental notes; I didn't read the later patches in the series that may already address these issues. For the makefile, I've to say that this is just a hack to make it work. I'm not sure how it would be correctly integrated into git's makefile hierarchy. The OPENSSL header and the xdiff/lib.a are here because it doesn't work otherwise. I need to dig into that to find out why. Any tips how to do it right? GIT_LIB = ../../libgit.a VCSSVN_LIB = ../../vcs-svn/lib.a @@ -37,8 +37,12 @@ svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(GIT_LIB) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ svn-fe.o \ $(ALL_LDFLAGS) $(LIBS) -svn-fe.o: svn-fe.c ../../vcs-svn/svndump.h - $(QUIET_CC)$(CC) -I../../vcs-svn -o $*.o -c $(ALL_CFLAGS) $ +remote-svn$X: remote-svn.o $(VCSSVN_LIB) $(GIT_LIB) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ remote-svn.o \ + $(ALL_LDFLAGS) $(LIBS) + +%.o: %.c ../../vcs-svn/svndump.h + $(QUIET_CC)$(CC) -I../../vcs-svn -I../../ -o $*.o -c $(ALL_CFLAGS) $ svn-fe.html: svn-fe.txt $(QUIET_SUBDIR0)../../Documentation $(QUIET_SUBDIR1) \ @@ -58,6 +62,6 @@ svn-fe.1: svn-fe.txt $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) libgit.a clean: - $(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1 + $(RM) svn-fe$X svn-fe.o svn-fe.html svn-fe.xml svn-fe.1 remote-svn.o .PHONY: all clean FORCE -- To unsubscribe from this list: send the line 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/RFC v3 07/16] Add a symlink 'git-remote-svn' in base dir.
On Tuesday 14 August 2012 13:46:43 Junio C Hamano wrote: Florian Achleitner florian.achleitner.2.6...@gmail.com writes: Allow execution of git-remote-svn even if the binary currently is located in contrib/svn-fe/. Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com --- git-remote-svn |1 + 1 file changed, 1 insertion(+) create mode 12 git-remote-svn diff --git a/git-remote-svn b/git-remote-svn new file mode 12 index 000..d3b1c07 --- /dev/null +++ b/git-remote-svn @@ -0,0 +1 @@ +contrib/svn-fe/remote-svn \ No newline at end of file Please scratch my previous comment. I thought you were adding an entry to .gitignore or something. I'd rather not to see such a symbolic link that points at a build product in the source tree. Making a symlink from the toplevel Makefile _after_ we built it in contrib/svn-fe/ (and removing it upon make clean) is OK, though. As with the makefile in contrib/svn-fe, this is just a hack. The toplevel Makefile doesn't seem to build contrib/* at all. I always need to call make explicitly in these subdirs. -- To unsubscribe from this list: send the line 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: Feature request - discard hunk in add --patch mode
Mina Almasry almasry.m...@hotmail.com writes:
I frequently stage files using git add --patch command and I almost
always come across debug code I want to discard, but there is no option
for that in the prompt. The result is that I have to run an extra
command after the dialogue ends.
I would like to add a feature to allow users to discard hunks using a
command like r! or d!
This has come up before, and actually led to the introduction of
'checkout -p' and 'reset -p':
http://thread.gmane.org/gmane.comp.version-control.git/123854
Judging by the '!' above, you are already aware that this is a dangerous
option that needs some safeguards. I imagine that would largely account
for Junio's safety concerns. So you could pick up from the general
direction of Pierre's post, and try to work out something.
However, life has become rather more complicated since 2009. Whatever
you do also needs to fit nicely with checkout/reset/stash -p. The first
two also take commit arguments, resulting in a total of seven modes of
operation, defined in %patch_modes.
I imagine one good angle of attack would be to proceed as follows:
* Optionally make add--interactive.perl more aware of what the patches
say. Ideally there would be a way to reverse the direction of a diff
(or otherwise manipulate it) in the program itself, instead of having
to decide this when fetching the patches.
* Change things so that the actions in %patch_modes are a hunk property
instead of a global state. Of course the actual mode still needs to
be global state. Not all actions should be possible in all modes;
figure out a clean way to implement this.
* Design commands in some modes that switch to another mode, and/or set
another action than the default in the current mode.
Note that r! commands need to also work in singlekey mode, where the 'r'
is read immediately and you would have to read for another
(confirmation) key to get the '!'.
Good luck!
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line 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/RFC v3 16/16] Add a test script for remote-svn.
Forget this patch! It contains some unwanted content. Something with rebasing
went wrong..
On Tuesday 14 August 2012 21:13:18 Florian Achleitner wrote:
Use svnrdump_sim.py to emulate svnrdump without an svn server.
Tests fetching, incremental fetching, fetching from file://,
and the regeneration of fast-import's marks file.
Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com
---
t/t9020-remote-svn.sh | 69
+ transport-helper.c|
15 ++-
2 files changed, 77 insertions(+), 7 deletions(-)
create mode 100755 t/t9020-remote-svn.sh
diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
new file mode 100755
index 000..a0c6a21
--- /dev/null
+++ b/t/t9020-remote-svn.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+test_description='tests remote-svn'
+
+. ./test-lib.sh
+
+# We override svnrdump by placing a symlink to the svnrdump-emulator in .
+export PATH=$HOME:$PATH
+ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py $HOME/svnrdump
+
+init_git () {
+ rm -fr .git
+ git init
+ #git remote add svnsim
svn::sim:///$TEST_DIRECTORY/t9020/example.svnrdump
+ # let's reuse an exisiting dump file!?
+ git remote add svnsim svn::sim:///$TEST_DIRECTORY/t9154/svn.dump
+ git remote add svnfile svn::file:///$TEST_DIRECTORY/t9154/svn.dump
+}
+
+test_debug '
+ git --version
+ which git
+ which svnrdump
+'
+
+test_expect_success 'simple fetch' '
+ init_git
+ git fetch svnsim
+ test_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master
+ cp .git/refs/remotes/svnsim/master master.good
+'
+
+test_debug '
+ cat .git/refs/svn/svnsim/master
+ cat .git/refs/remotes/svnsim/master
+'
+
+test_expect_success 'repeated fetch, nothing shall change' '
+ git fetch svnsim
+ test_cmp master.good .git/refs/remotes/svnsim/master
+'
+
+test_expect_success 'fetch from a file:// url gives the same result' '
+ git fetch svnfile
+'
+
+test_expect_failure 'the sha1 differ because the git-svn-id line in the
commit msg contains the url' ' + test_cmp
.git/refs/remotes/svnfile/master
.git/refs/remotes/svnsim/master +'
+
+test_expect_success 'mark-file regeneration' '
+ mv .git/info/fast-import/marks/svnsim
.git/info/fast-import/marks/svnsim.old + git fetch svnsim
+ test_cmp .git/info/fast-import/marks/svnsim.old
.git/info/fast-import/marks/svnsim +'
+
+test_expect_success 'incremental imports must lead to the same head' '
+ export SVNRMAX=3
+ init_git
+ git fetch svnsim
+ test_cmp .git/refs/svn/svnsim/master .git/refs/remotes/svnsim/master
+ unset SVNRMAX
+ git fetch svnsim
+ test_cmp master.good .git/refs/remotes/svnsim/master
+'
+
+test_debug 'git branch -a'
+
+test_done
diff --git a/transport-helper.c b/transport-helper.c
index 47db055..a363f2c 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -17,6 +17,7 @@ static int debug;
struct helper_data {
const char *name;
struct child_process *helper;
+ struct argv_array argv;
FILE *out;
unsigned fetch : 1,
import : 1,
@@ -103,7 +104,6 @@ static void do_take_over(struct transport *transport)
static struct child_process *get_helper(struct transport *transport)
{
struct helper_data *data = transport-data;
- struct argv_array argv = ARGV_ARRAY_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process *helper;
const char **refspecs = NULL;
@@ -125,10 +125,11 @@ static struct child_process *get_helper(struct
transport *transport) helper-in = -1;
helper-out = -1;
helper-err = 0;
- argv_array_pushf(argv, git-remote-%s, data-name);
- argv_array_push(argv, transport-remote-name);
- argv_array_push(argv, remove_ext_force(transport-url));
- helper-argv = argv.argv;
+ argv_array_init(data-argv);
+ argv_array_pushf(data-argv, git-remote-%s, data-name);
+ argv_array_push(data-argv, transport-remote-name);
+ argv_array_push(data-argv, remove_ext_force(transport-url));
+ helper-argv = data-argv.argv;
helper-git_cmd = 0;
helper-silent_exec_failure = 1;
@@ -143,8 +144,6 @@ static struct child_process *get_helper(struct transport
*transport)
data-helper = helper;
data-no_disconnect_req = 0;
- free((void*) helper_env[1]);
- argv_array_clear(argv);
/*
* Open the output as FILE* so strbuf_getline() can be used.
@@ -247,6 +246,8 @@ static int disconnect_helper(struct transport
*transport) close(data-helper-out);
fclose(data-out);
res = finish_command(data-helper);
+ free((void*) data-helper-env[1]);
+ argv_array_clear(data-argv);
free(data-helper);
data-helper = NULL;
}
--
To unsubscribe from this list: send the
Re: [PATCH/RFC v3 04/16] Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability.
On Tuesday 14 August 2012 13:40:20 Junio C Hamano wrote:
Florian Achleitner florian.achleitner.2.6...@gmail.com writes:
The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers
to retrieve information about blobs and trees that already exist in
fast-import's memory. This requires a channel from fast-import to the
remote-helper.
remote-helpers that use this features shall advertise the new
'bidi-import'
s/this fea/these fea/
capability so signal that they require the communication channel.
s/so sig/to sig/, I think.
When forking fast-import in transport-helper.c connect it to a dup of
the remote-helper's stdin-pipe. The additional file descriptor is passed
to fast-import via it's command line (--cat-blob-fd).
s/via it's/via its/;
It follows that git and fast-import are connected to the remote-helpers's
stdin.
Because git can send multiple commands to the remote-helper on it's stdin,
it is required that helpers that advertise 'bidi-import' buffer all input
commands until the batch of 'import' commands is ended by a newline
before sending data to fast-import.
This is to prevent mixing commands and fast-import responses on the
helper's stdin.
Please have a blank line each between paragraphs; a solid block of
text is very hard to follow.
Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com
---
transport-helper.c | 45 -
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index cfe0988..257274b 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -10,6 +10,7 @@
#include string-list.h
#include thread-utils.h
#include sigchain.h
+#include argv-array.h
static int debug;
@@ -19,6 +20,7 @@ struct helper_data {
FILE *out;
unsigned fetch : 1,
import : 1,
+ bidi_import : 1,
export : 1,
option : 1,
push : 1,
@@ -101,6 +103,7 @@ static void do_take_over(struct transport *transport)
static struct child_process *get_helper(struct transport *transport)
{
struct helper_data *data = transport-data;
+ struct argv_array argv = ARGV_ARRAY_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process *helper;
const char **refspecs = NULL;
@@ -122,11 +125,10 @@ static struct child_process *get_helper(struct
transport *transport)
helper-in = -1;
helper-out = -1;
helper-err = 0;
- helper-argv = xcalloc(4, sizeof(*helper-argv));
- strbuf_addf(buf, git-remote-%s, data-name);
- helper-argv[0] = strbuf_detach(buf, NULL);
- helper-argv[1] = transport-remote-name;
- helper-argv[2] = remove_ext_force(transport-url);
+ argv_array_pushf(argv, git-remote-%s, data-name);
+ argv_array_push(argv, transport-remote-name);
+ argv_array_push(argv, remove_ext_force(transport-url));
+ helper-argv = argv.argv;
Much nicer than before thanks to argv_array ;-)
helper-git_cmd = 0;
helper-silent_exec_failure = 1;
@@ -141,6 +143,8 @@ static struct child_process *get_helper(struct
transport *transport)
data-helper = helper;
data-no_disconnect_req = 0;
+ free((void*) helper_env[1]);
What is this free() for???
Sorry, legacy from previous versions, will be deleted.
+ argv_array_clear(argv);
See below.
/*
* Open the output as FILE* so strbuf_getline() can be used.
@@ -178,6 +182,8 @@ static struct child_process *get_helper(struct
transport *transport)
data-push = 1;
else if (!strcmp(capname, import))
data-import = 1;
+ else if (!strcmp(capname, bidi-import))
+ data-bidi_import = 1;
else if (!strcmp(capname, export))
data-export = 1;
else if (!data-refspecs !prefixcmp(capname, refspec )) {
@@ -241,8 +247,6 @@ static int disconnect_helper(struct transport
*transport)
close(data-helper-out);
fclose(data-out);
res = finish_command(data-helper);
- free((char *)data-helper-argv[0]);
- free(data-helper-argv);
free(data-helper);
data-helper = NULL;
}
@@ -376,14 +380,24 @@ static int fetch_with_fetch(struct transport
*transport,
static int get_importer(struct transport *transport, struct child_process
*fastimport) {
struct child_process *helper = get_helper(transport);
+ struct helper_data *data = transport-data;
+ struct argv_array argv = ARGV_ARRAY_INIT;
+ int cat_blob_fd, code;
memset(fastimport, 0, sizeof(*fastimport));
fastimport-in = helper-out;
-
Re: [PATCH] daemon: --access-hook option
On Tue, Aug 14, 2012 at 10:12 PM, Junio C Hamano gits...@pobox.com wrote: The --access-hook option to git daemon specifies an external command to be run every time a client connects, with - service name (e.g. upload-pack, etc.), - path to the repository, - hostname (%H), - canonical hostname (%CH), - ip address (%IP), - tcp port (%P) as its command line arguments. The external command can decide to decline the service by exiting with a non-zero status (or to allow it by exiting with a zero status). It can also look at the $REMOTE_ADDR and $REMOTE_PORT environment variables to learn about the requestor when making this decision. The external command can optionally write a single line to its standard output to be sent to the requestor as an error message when it declines the service. Signed-off-by: Junio C Hamano gits...@pobox.com Thanks Junio, this looks like the best approach. Acked-by: Shawn O. Pearce spea...@spearce.org -- To unsubscribe from this list: send the line 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] help: correct behavior for is_executable on Windows
Hi Junio,
On Mon, Aug 13, 2012 at 10:48:14AM -0700, Junio C Hamano wrote:
Heiko Voigt hvo...@hvoigt.net writes:
What do you think?
Does having the stat() help on Windows in any way? Does it ever
return an executable bit by itself?
No, AFAIK it does not return anything about executability. But I think
the stat is still necessary to verify that the file exists and is a
regular file.
Here is a draft of a patch I would follow up with:
Subject: [PATCH RFC] help: extract platform dependent part of is_executable in
extra module
To remove various ifdefs required for the special handling of
executables on windows we create a new module 'executable' in compat
which allows a user to correctly fill the S_IXUSR flag of struct stat on
Windows.
Since this is valid for all variants of windows (mingw, msvc, cygwin) we
create a new module to avoid code duplication. compat/msvc.h includes
mingw.h so we do not add an extra include there. By default we define
correct_executeable_stat() to a no op on all other platforms. This is
guarded by a NO_EXECUTABLE_STAT which when defined by a compat header
requires and implementation of this function.
NOTE: I did not test this. I first would like to discuss whether the
overall structure this approach is taking is ok.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
Makefile | 8 +---
compat/cygwin.h | 2 ++
compat/mingw.h| 2 ++
compat/win32/executable.c | 33 +
compat/win32/executable.h | 9 +
git-compat-util.h | 4
help.c| 30 ++
7 files changed, 57 insertions(+), 31 deletions(-)
create mode 100644 compat/win32/executable.c
create mode 100644 compat/win32/executable.h
diff --git a/Makefile b/Makefile
index 6b0c961..cea3559 100644
--- a/Makefile
+++ b/Makefile
@@ -1073,7 +1073,7 @@ ifeq ($(uname_O),Cygwin)
# Try commenting this out if you suspect MMAP is more efficient
NO_MMAP = YesPlease
X = .exe
- COMPAT_OBJS += compat/cygwin.o
+ COMPAT_OBJS += compat/cygwin.o compat/win32/executable.o
UNRELIABLE_FSTAT = UnfortunatelyYes
SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield
endif
@@ -1257,7 +1257,8 @@ ifeq ($(uname_S),Windows)
BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild
-Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H
-D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
COMPAT_OBJS = compat/msvc.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
- compat/win32/poll.o compat/win32/dirent.o
+ compat/win32/poll.o compat/win32/dirent.o \
+ compat/win32/executable.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
EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
@@ -1347,7 +1348,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
- compat/win32/poll.o compat/win32/dirent.o
+ compat/win32/poll.o compat/win32/dirent.o \
+ compat/win32/executable.o
EXTLIBS += -lws2_32
PTHREAD_LIBS =
X = .exe
diff --git a/compat/cygwin.h b/compat/cygwin.h
index a3229f5..e3bbd4d 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -1,6 +1,8 @@
#include sys/types.h
#include sys/stat.h
+#include win32/executable.h
+
typedef int (*stat_fn_t)(const char*, struct stat*);
extern stat_fn_t cygwin_stat_fn;
extern stat_fn_t cygwin_lstat_fn;
diff --git a/compat/mingw.h b/compat/mingw.h
index 61a6521..73c4f3d 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -1,6 +1,8 @@
#include winsock2.h
#include ws2tcpip.h
+#include win32/executable.h
+
/*
* things that are not available in header files
*/
diff --git a/compat/win32/executable.c b/compat/win32/executable.c
new file mode 100644
index 000..326ddb1
--- /dev/null
+++ b/compat/win32/executable.c
@@ -0,0 +1,33 @@
+#include executeable.h
+
+void correct_executable_stat(const char *filename, struct stat *st)
+{
+ char buf[3] = { 0 };
+ int n, fd;
+
+ /* On Windows we cannot use the executable bit. The executable
+* state is determined by extension only. We do this first
+* because with virus scanners opening an executeable for
+* reading is potentially expensive.
+*/
+ if (has_extension(name, .exe))
+ st-st_mode |= S_IXUSR;
+
+ if (st.st_mode S_IXUSR)
+ return;
+
+ /* now that we know it does not have an executable extension,
+ peek into the
[PATCH v2] add some bash style we prefer
During discussion of other patches these preferences have been revealed.
Lets add them to the guidelines.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
Here an updated version of the patch.
On Tue, Aug 14, 2012 at 02:09:35PM -0700, Junio C Hamano wrote:
Heiko Voigt hvo...@hvoigt.net writes:
@@ -97,6 +102,7 @@ For shell scripts specifically (not exhaustive):
interface translatable. See Marking strings for translation in
po/README.
+
For C programs:
Probably not needed, as there is no such double space between C
and Documentation sections.
Sorry about that whitespace noise.
Cheers Heiko
Documentation/CodingGuidelines | 8
1 file changed, 8 insertions(+)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 4557711..e70d110 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -76,11 +76,19 @@ For shell scripts specifically (not exhaustive):
- We do not use Process Substitution (list) or (list).
+ - We prefer writing all control structures without semicolon on the
+ same line. E.g. then should be on the next line for if statements.
+ The same applies to while, for, ...
+
- We prefer test over [ ... ].
- We do not write the noiseword function in front of shell
functions.
+ - We prefer a space between the function name and the parentheses. The
+ opening { should also be on the same line.
+ E.g.: my_function () {
+
- As to use of grep, stick to a subset of BRE (namely, no \{m,n\},
[::], [==], nor [..]) for portability.
--
1.7.12.rc2.11.g5d52328
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] revisions passed to cherry-pick should be in default order
Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: So all of the above case give the right result in the end as long as the timestamps are chronological, and case 1) gives the right result regardless. The other two cases only works in most cases because the unexpcted sorting when no-walk is in effect counteracts the final reversal. In short, if you have three commits in a row, A--B--C, with timestamps that are not skewed, and want to replay changes of B and then C in that order, all three you listed ends up doing the right thing. But if you want to apply the change C and then B: - git cherry-pick A..C is obviously not a way to do so, so we won't discuss it further. - git cherry-pick C B is the most natural way the user would want to express this request, but because of the sorting (i.e. commit_list_sort_by_date() in prepare_revision_walk(), combined with -reverse in sequencer.c::prepare_revs()), it applies B and then C. That is the real bug. Feeding the revs to git cherry-pick --stdin in the order the user wishes them to be applied has the same issue. IIUC, this could be implemented by making cherry-pick iterate over rev_info.pending.objects just like 'git show' does when not walking. Yes, that was exactly why I said sequencer.c::prepare_revs() is wrong to call prepare_revision_walk() unconditionally, even when there is no revision walking involved. I actually think your approach to place the do not sort when we are not walking logic in prepare_revision_walk() makes more sense. show has to look at pending.objects[] because it needs to show objects other than commits (e.g. git show :foo), so there won't be any change in its implementation with your change. It will have to look at pending.objects[] itself. But cherry-pick and sequencer-derived commands only deal with commits. It would be far less error prone to let them call get_revision() repeatedly like all other revision enumerating commands do, than to have them go over the pending.objects[] list, dereferencing tags and using only commits. The resulting callers would be more readable, too, I would think. -- To unsubscribe from this list: send the line 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: Your branch and 'origin/master' have diverged
Thomas Rast tr...@student.ethz.ch writes:
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@student.ethz.ch writes:
In some sense this is a really bad case of wrong UI design, because we
(this happens on #git a lot) have to teach users not to use the command
so they won't trip over this problem. It would be better to fix the
real issue instead. IIRC it was even on the 1.8.0 wishlist...
Is it?
There already is a way to ask it to update the single tracking
branch while fetching; git fetch origin master that
unconditionally updates refs/remotes/origin/master without a way to
tell it not to do so will be a grave usability regression.
Grave? Do you have any data/use-cases to back that up with?
When I get a pull request from say Eric, I would:
git fetch git-svn master
git show-branch remotes/git-svn/master FETCH_HEAD
to see what happened since the last pull request on the other side.
He may have rebased (which is not necessarily a crime), or I may see
more commits in the output than what he lists in the request message
(which may indicate I may have missed an earlier pull request from
him).
Such a sanity check will stop working if the first fetch updated
my remotes/git-svn/master. I would have to enable reflog for
tracking branch and do something like this:
git show-branch remotes/git-svn/master@{1} remotes/git-svn/master
So I was correct in saying that without an easy escape hatch, such a
change would be a regression.
But I think I (and others) could just train fingers to do
git fetch git-svn master:
as a workaround.
Updating Documentation/pull-fetch-param.txt would be a bear, though.
The documentation is stale in that it was written in the days back
when .git/remotes/ was the primary way to configure remotes, and was
not adjusted to use the termilology used in the [remote where]
section of the .git/config file (notice a mention of 'Pull: '
lines there), so it needs cosmetic adjustment anyway, but the
semantics it spells is still up to date. The current rule is very
simple and understandable. You either say from the command line
exactly what should happen (refspec without colon is the same as the
refspec with colon at the end, meaning do not track; if you want
to track, you write what to update with the fetch), or we use the
configured refspec (which again spells what should happen).
The updated rule would be more complex. If a remote nickname is
used, and a refspec given from the command line is without colon, a
new special rule overrides the current behaviour and tries to match
with a configured refspec. You would need to desribe what happens
in that case.
--
To unsubscribe from this list: send the line 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/RFC v3 04/16] Connect fast-import to the remote-helper via pipe, adding 'bidi-import' capability.
Florian Achleitner florian.achleitner.2.6...@gmail.com writes: The updated code frees argv[] immediately after start_command() returns, and it may happen to be safe to do so with the current implementation of start_command() and friends, but I think it is a bad taste to free argv[] (or env[] for that matter) before calling finish_command(). These pieces of memory are still pointed by the child_process structure, and users of the structure may want to use contents of them (especially, argv[0]) for reporting errors and various other purposes, e.g. child = get_helper(); trace(started %s\n, child-argv[0]); if (finish_command(child)) return error(failed to cleanly finish %s, child-argv[0]); Yes, sounds reasonable. The present of immedate clearing has the advantage that I don't have to store the struct argv_array, as struct child_process only has a member for const char **argv. And updated code shouldn't have to store struct argv_array either. If you just give the ownership of argv_array.argv to child_process and clean it as part of destroying the child_process, you do not have to worry about argv_array at all. In order to cleanly support that use case at the API level, we may want to introduce argv_array_detach() that is similar in spirit to strbuf_detach(), which transfers ownership of the underlying memory to the caller. -- To unsubscribe from this list: send the line 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] help: correct behavior for is_executable on Windows
Heiko Voigt hvo...@hvoigt.net writes:
On Mon, Aug 13, 2012 at 10:48:14AM -0700, Junio C Hamano wrote:
Heiko Voigt hvo...@hvoigt.net writes:
What do you think?
Does having the stat() help on Windows in any way? Does it ever
return an executable bit by itself?
No, AFAIK it does not return anything about executability. But I think
the stat is still necessary to verify that the file exists and is a
regular file.
But if you are going to read it anyway, you can tell it from open()
and read() of the first 2 bytes failing, no? That will still be an
implementation detail of platform specific is_path_executable().
So you are forcing Windows an extra and unnecessary stat() that only
is needed on Cygwin, no?
@@ -1347,7 +1348,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
- compat/win32/poll.o compat/win32/dirent.o
+ compat/win32/poll.o compat/win32/dirent.o \
+ compat/win32/executable.o
Looks sensible, even though the filename does not tell what it does
about executable. is_executable.o might be a better name for them.
diff --git a/help.c b/help.c
index ebc2c42..d9fae3c 100644
--- a/help.c
+++ b/help.c
@@ -106,34 +106,8 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#if defined(WIN32) || defined(__CYGWIN__)
- /* On Windows we cannot use the executable bit. The executable
- * state is determined by extension only. We do this first
- * because with virus scanners opening an executeable for
- * reading is potentially expensive.
- */
- if (has_extension(name, .exe))
- return S_IXUSR;
-
-#if defined(__CYGWIN__)
-if ((st.st_mode S_IXUSR) == 0)
-#endif
-{/* now that we know it does not have an executable extension,
-peek into the file instead */
- char buf[3] = { 0 };
- int n;
- int fd = open(name, O_RDONLY);
- st.st_mode = ~S_IXUSR;
- if (fd = 0) {
- n = read(fd, buf, 2);
- if (n == 2)
- /* look for a she-bang */
- if (!strcmp(buf, #!))
- st.st_mode |= S_IXUSR;
- close(fd);
- }
-}
-#endif
+ correct_executable_stat(name, st);
+
Yuck.
Why should we need even a single line of the implementation of a
function that tells if a given pathname contains an executable
command, which we know is platform specific?
On Posix systems, the implementation will be stat() and check
S_IXUSR. On pure Windows, it will be check .exe, or open it and
read the first two bytes. On Cygwin, it will also be check .exe,
stat() and check S_IXUSR, or open it and read the first two bytes.
It is not like the caller of is_executable() needed to run stat for
other purposes on its own and we can optimize by either borrowing
the stat data the caller already collected for us, or returning the
stat data we collected for later use by the caller. The use of stat
is entirely contained in the POSIX implementation of this function.
Why are you so dead-set to shoehorn the different semantics into
struct stat that we know is not an appropriate abstraction across
platforms?
--
To unsubscribe from this list: send the line 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] add some bash style we prefer
Heiko Voigt hvo...@hvoigt.net writes:
During discussion of other patches these preferences have been revealed.
Lets add them to the guidelines.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
Here an updated version of the patch.
Thanks.
On Tue, Aug 14, 2012 at 02:09:35PM -0700, Junio C Hamano wrote:
Heiko Voigt hvo...@hvoigt.net writes:
@@ -97,6 +102,7 @@ For shell scripts specifically (not exhaustive):
interface translatable. See Marking strings for translation in
po/README.
+
For C programs:
Probably not needed, as there is no such double space between C
and Documentation sections.
Sorry about that whitespace noise.
Cheers Heiko
Documentation/CodingGuidelines | 8
1 file changed, 8 insertions(+)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 4557711..e70d110 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -76,11 +76,19 @@ For shell scripts specifically (not exhaustive):
- We do not use Process Substitution (list) or (list).
+ - We prefer writing all control structures without semicolon on the
+ same line. E.g. then should be on the next line for if statements.
+ The same applies to while, for, ...
+
- We prefer test over [ ... ].
- We do not write the noiseword function in front of shell
functions.
+ - We prefer a space between the function name and the parentheses. The
+ opening { should also be on the same line.
+ E.g.: my_function () {
+
- As to use of grep, stick to a subset of BRE (namely, no \{m,n\},
[::], [==], nor [..]) for portability.
--
To unsubscribe from this list: send the line 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: Feature request - discard hunk in add --patch mode
Thomas Rast tr...@student.ethz.ch writes: Mina Almasry almasry.m...@hotmail.com writes: I frequently stage files using git add --patch command and I almost always come across debug code I want to discard, but there is no option for that in the prompt. The result is that I have to run an extra command after the dialogue ends. I would like to add a feature to allow users to discard hunks using a command like r! or d! This has come up before, and actually led to the introduction of 'checkout -p' and 'reset -p': http://thread.gmane.org/gmane.comp.version-control.git/123854 That is a blast from the past. Why is saying git checkout . too much work, after add -p that you excluded the debugging cruft? I actually do this for extra safety, though: git add -p ;# add everything but exclude debug cruft git diff ;# make really sure that this shows only garbage git diff | git apply -R ;# and get rid of that garbage primarily because I can take the git diff output in the second step to a file, remove the hunk that I accidentally forgot to add in the add -p stage, and apply -R to remove only the cruft. After doing so, git diff will show the important-but-forgotten bit, and I can choose to add it to the next commit, or I can choose to leave it in the working tree for a future commit after the current index is committed. But the above is a tangent side-note to show possibly a better way to work, not about adding new operations to add -p. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] revisions passed to cherry-pick should be in default order
On Wed, Aug 15, 2012 at 10:16 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: So all of the above case give the right result in the end as long as the timestamps are chronological, and case 1) gives the right result regardless. The other two cases only works in most cases because the unexpcted sorting when no-walk is in effect counteracts the final reversal. In short, if you have three commits in a row, A--B--C, with timestamps that are not skewed, and want to replay changes of B and then C in that order, all three you listed ends up doing the right thing. But if you want to apply the change C and then B: - git cherry-pick A..C is obviously not a way to do so, so we won't discuss it further. - git cherry-pick C B is the most natural way the user would want to express this request, but because of the sorting (i.e. commit_list_sort_by_date() in prepare_revision_walk(), combined with -reverse in sequencer.c::prepare_revs()), it applies B and then C. That is the real bug. Feeding the revs to git cherry-pick --stdin in the order the user wishes them to be applied has the same issue. Exactly. I actually think your approach to place the do not sort when we are not walking logic in prepare_revision_walk() makes more sense. show has to look at pending.objects[] because it needs to show objects other than commits (e.g. git show :foo), so there won't be any change in its implementation with your change. It will have to look at pending.objects[] itself. Yes, I noticed that's why show has to do it that way. But cherry-pick and sequencer-derived commands only deal with commits. It would be far less error prone to let them call get_revision() repeatedly like all other revision enumerating commands do, than to have them go over the pending.objects[] list, dereferencing tags and using only commits. The resulting callers would be more readable, too, I would think. Makes sense, I'll try to implement it that way. I was afraid that we would need to call prepare_revision_walk() once first and then if we afterwards find out that we should not walk, we would need to call it again without the reverse option. But after looking at how rev_info.reverse is used, it seem like it's only used in get_revision(), so we can leave it either on or off during the prepare_revision_walk() and the and set appropriately before calling get_revision(), like so: init_revisions(revs); revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED; setup_revisions(...); prepare_revision_walk(revs); revs.reverse = !revs.no_walk; // iterate over revisions -- To unsubscribe from this list: send the line 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] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO
On Thu, 9 Aug 2012, Junio C Hamano wrote: Jay Soffian jaysoff...@gmail.com writes: When gitweb is used as a DirectoryIndex, it attempts to strip PATH_INFO on its own, as $cgi-url() fails to do so. However, it fails to account for the fact that PATH_INFO has already been URL-decoded by the web server, but the value returned by $cgi-url() has not been. This causes the stripping to fail whenever the URL contains encoded characters. To see this in action, setup gitweb as a DirectoryIndex and then use it on a repository with a directory containing a space in the name. Navigate to tree view, examine the gitweb generated html and you'll see a link such as: a href=/test.git/tree/HEAD:/directory with spacesdirectory with spaces/a When clicked on, the browser will URL-encode this link, giving a $cgi-url() of the form: /test.git/tree/HEAD:/directory%20with%20spaces While PATH_INFO is: /test.git/tree/HEAD:/directory with spaces Fix this by calling unescape() on both $my_url and $my_uri before stripping PATH_INFO from them. Signed-off-by: Jay Soffian jaysoff...@gmail.com --- Thanks. From a cursory look, with the help from the explanation in the proposed commit log message, the change looks sensible. I wonder if a breakage like this is something we can catch in one of the t95xx series of tests, though. No, it is unfortunately not possible with current test infrastructure for gitweb. The gitweb_run from t/gitweb-lib.sh allows to set PATH_INFO and QUERY_STRING, but does not allow to set up URL. That might change in the future... Jakub, Ack? Acked-by: Jakub Narebski jna...@gmail.com Uf ut us bot too late... -- Jakub Narebski Poland -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] revisions passed to cherry-pick should be in default order
Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: Makes sense, I'll try to implement it that way. I was afraid that we would need to call prepare_revision_walk() once first and then if we afterwards find out that we should not walk, we would need to call it again without the reverse option. But after looking at how rev_info.reverse is used, it seem like it's only used in get_revision(), so we can leave it either on or off during the prepare_revision_walk() and the and set appropriately before calling get_revision(), like so: init_revisions(revs); revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED; setup_revisions(...); prepare_revision_walk(revs); revs.reverse = !revs.no_walk; Sorry, but I do not understand why you frutz with reverse after prepare, and not before. I think you can just set no_walk and let setup_revisions() turn it off upon seeing a range (this happens in add_pending_object()). After setup_revisions() returns, if no_walk is still set, you only got individual refs without ranges, so no reversing required. You also need to be careful about revert that shares the code; when reverting range A..C in your example, you want to undo C and then B, and you do not want to reverse them. -- To unsubscribe from this list: send the line 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: Feature request - discard hunk in add --patch mode
Junio C Hamano gits...@pobox.com writes: Thomas Rast tr...@student.ethz.ch writes: This has come up before, and actually led to the introduction of 'checkout -p' and 'reset -p': http://thread.gmane.org/gmane.comp.version-control.git/123854 That is a blast from the past. Why is saying git checkout . too much work, after add -p that you excluded the debugging cruft? Please forget this question. A better way in the form of stash -p was suggested in the old thread to get rid of debug cruft in the tree before an add -p session (or during a series of add -p sessions). So is this still an issue? -- To unsubscribe from this list: send the line 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] gitweb: URL-decode $my_url/$my_uri when stripping PATH_INFO
Jakub Narebski jna...@gmail.com writes: On Thu, 9 Aug 2012, Junio C Hamano wrote: Jay Soffian jaysoff...@gmail.com writes: When gitweb is used as a DirectoryIndex, it attempts to strip PATH_INFO on its own, as $cgi-url() fails to do so. However, it fails to account for the fact that PATH_INFO has already been URL-decoded by the web server, but the value returned by $cgi-url() has not been. This causes the stripping to fail whenever the URL contains encoded characters. To see this in action, setup gitweb as a DirectoryIndex and then use it on a repository with a directory containing a space in the name. Navigate to tree view, examine the gitweb generated html and you'll see a link such as: a href=/test.git/tree/HEAD:/directory with spacesdirectory with spaces/a When clicked on, the browser will URL-encode this link, giving a $cgi-url() of the form: /test.git/tree/HEAD:/directory%20with%20spaces While PATH_INFO is: /test.git/tree/HEAD:/directory with spaces Fix this by calling unescape() on both $my_url and $my_uri before stripping PATH_INFO from them. Signed-off-by: Jay Soffian jaysoff...@gmail.com --- Thanks. From a cursory look, with the help from the explanation in the proposed commit log message, the change looks sensible. I wonder if a breakage like this is something we can catch in one of the t95xx series of tests, though. No, it is unfortunately not possible with current test infrastructure for gitweb. The gitweb_run from t/gitweb-lib.sh allows to set PATH_INFO and QUERY_STRING, but does not allow to set up URL. That might change in the future... Jakub, Ack? Acked-by: Jakub Narebski jna...@gmail.com Uf ut us bot too late... 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: Your branch and 'origin/master' have diverged
Holger Hellmuth (IKS) hellm...@ira.uka.de writes: Am 15.08.2012 19:30, schrieb Junio C Hamano: The current rule is very simple and understandable. You either say from the command line exactly what should happen (refspec without colon is the same as the refspec with colon at the end, meaning do not track; if you want to track, you write what to update with the fetch), or we use the configured refspec (which again spells what should happen). Couldn't a similar new rule just say that refspec name is a short for name:name ? Of course not. -- To unsubscribe from this list: send the line 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] help: correct behavior for is_executable on Windows
Junio C Hamano gits...@pobox.com writes: diff --git a/help.c b/help.c ... + Yuck. Why should we need even a single line of the implementation of a function that tells if a given pathname contains an executable command, which we know is platform specific? Sorry; sent without sufficient proofreading. Obviously we need such an implementation (per platform) somewhere in the system. What I meant to ask was Why should we need a function whose implementation has to be platform-specific in this help.c file. The last part of the sentence was missing. -- To unsubscribe from this list: send the line 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/RFC v3 14/16] transport-helper: add import|export-marks to fast-import command line.
Florian Achleitner florian.achleitner.2.6...@gmail.com writes: fast-import internally uses marks that refer to an object via its sha1. Those marks are created during import to find previously created objects. At exit the accumulated marks can be exported to a file and reloaded at startup, so that the previous marks are available. Add command line options to the fast-import command line to enable this. The mark files are stored in info/fast-import/marks/remote-name. Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com --- transport-helper.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/transport-helper.c b/transport-helper.c index 7fb52d4..47db055 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -387,6 +387,9 @@ static int get_importer(struct transport *transport, struct child_process *fasti fastimport-in = helper-out; argv_array_push(argv, fast-import); argv_array_push(argv, debug ? --stats : --quiet); + argv_array_push(argv, --relative-marks); + argv_array_pushf(argv, --import-marks-if-exists=marks/%s, transport-remote-name); + argv_array_pushf(argv, --export-marks=marks/%s, transport-remote-name); Is this something we want to do unconditionally? -- To unsubscribe from this list: send the line 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] rev-list docs: clarify --topo-order description
It was unclear what --topo-order was really about in the documentation. It is not just about children before parent, but also about don't mix lineages. Reword the description for both --date-order and --topo-order, and add an illustration to it. Signed-off-by: Junio C Hamano gits...@pobox.com --- Thomas Rast tr...@student.ethz.ch writes: But then the new description is wrong. It claims that children are always before parents, which is not true in the face of clock skew. Thanks for sanity-check. Here is an updated one. Documentation/rev-list-options.txt | 31 --- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 6a4b635..9404d08 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -578,16 +578,33 @@ Commit Ordering By default, the commits are shown in reverse chronological order. ---topo-order:: +--date-order:: + Show no parents before all of its children are shown, but + otherwise show commits in the commit timestamp order. - This option makes them appear in topological order (i.e. - descendant commits are shown before their parents). +--topo-order:: + Show no parents before all of its children are shown, and + avoid showing commits on multiple lines of history + intermixed. ++ +For example, in a commit history like this: ++ + ---date-order:: +---1247 + \ \ +3568--- - This option is similar to '--topo-order' in the sense that no - parent comes before all of its children, but otherwise things - are still ordered in the commit timestamp order. + ++ +where the numbers denote the order of commit timestamps, `git +rev-list` and friends with `--date-order` show the commits in the +timestamp order: 8 7 6 5 4 3 2 1. ++ +With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5 +3 1); some older commits are shown before newer ones in order to +avoid showing the commits from two parallel development track mixed +together. --reverse:: -- 1.7.12.rc2.85.g1de7134 -- To unsubscribe from this list: send the line 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/RFC v3 10/16] Create a note for every imported commit containing svn metadata.
On Wednesday 15 August 2012 12:49:04 Junio C Hamano wrote:
Florian Achleitner florian.achleitner.2.6...@gmail.com writes:
To provide metadata from svn dumps for further processing, e.g.
branch detection, attach a note to each imported commit that
stores additional information.
The notes are currently hard-coded in refs/notes/svn/revs.
Currently the following lines from the svn dump are directly
accumulated in the note. This can be refined on purpose, of course.
- Revision-number
- Node-path
- Node-kind
- Node-action
- Node-copyfrom-path
- Node-copyfrom-rev
Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com
---
vcs-svn/fast_export.c | 13 +
vcs-svn/fast_export.h |2 ++
vcs-svn/svndump.c | 21 +++--
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 1ecae4b..796dd1a 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -12,6 +12,7 @@
#include svndiff.h
#include sliding_window.h
#include line_buffer.h
+#include cache.h
Shouldn't it be near the beginning? Also if you include cache.h,
it probably makes git-compat-util and strbuf redundant.
Ack.
#define MAX_GITSVN_LINE_LEN 4096
@@ -68,6 +69,18 @@ void fast_export_modify(const char *path, uint32_t
mode, const char *dataref)
putchar('\n');
}
+void fast_export_begin_note(uint32_t revision, const char *author,
+ const char *log, unsigned long timestamp)
+{
+ timestamp = 1341914616;
The magic number needs some comment.
+ size_t loglen = strlen(log);
decl-after-statement. I am starting to suspect that the assignment
is a leftover from an earlier debugging effort, though.
Oh yes sorry. Leftover from a previous experiment.
Thx for your reviews Junio, I got too blind to see this.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 14/16] transport-helper: add import|export-marks to fast-import command line.
On Wednesday 15 August 2012 12:52:43 Junio C Hamano wrote: Florian Achleitner florian.achleitner.2.6...@gmail.com writes: fast-import internally uses marks that refer to an object via its sha1. Those marks are created during import to find previously created objects. At exit the accumulated marks can be exported to a file and reloaded at startup, so that the previous marks are available. Add command line options to the fast-import command line to enable this. The mark files are stored in info/fast-import/marks/remote-name. Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com --- transport-helper.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/transport-helper.c b/transport-helper.c index 7fb52d4..47db055 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -387,6 +387,9 @@ static int get_importer(struct transport *transport, struct child_process *fasti fastimport-in = helper-out; argv_array_push(argv, fast-import); argv_array_push(argv, debug ? --stats : --quiet); + argv_array_push(argv, --relative-marks); + argv_array_pushf(argv, --import-marks-if-exists=marks/%s, transport-remote-name); + argv_array_pushf(argv, --export-marks=marks/%s, transport-remote-name); Is this something we want to do unconditionally? Good question. It doesn't hurt, but it maybe . We could add another capability for remote-helpers, that tells us if it needs masks. What do you think? -- To unsubscribe from this list: send the line 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/RFC v3 11/16] When debug==1, start fast-import with --stats instead of --quiet.
Florian Achleitner florian.achleitner.2.6...@gmail.com writes:
fast-import prints statistics that could be interesting to the
developer of remote helpers.
Signed-off-by: Florian Achleitner florian.achleitner.2.6...@gmail.com
---
Sounds sensible and could be useful outside the context of this
series. Perhaps place it earlier in the series?
transport-helper.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/transport-helper.c b/transport-helper.c
index 257274b..7fb52d4 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -386,7 +386,7 @@ static int get_importer(struct transport *transport,
struct child_process *fasti
memset(fastimport, 0, sizeof(*fastimport));
fastimport-in = helper-out;
argv_array_push(argv, fast-import);
- argv_array_push(argv, --quiet);
+ argv_array_push(argv, debug ? --stats : --quiet);
if (data-bidi_import) {
cat_blob_fd = xdup(helper-in);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] revisions passed to cherry-pick should be in default order
On Wed, Aug 15, 2012 at 11:39 AM, Junio C Hamano gits...@pobox.com wrote: Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: Makes sense, I'll try to implement it that way. I was afraid that we would need to call prepare_revision_walk() once first and then if we afterwards find out that we should not walk, we would need to call it again without the reverse option. But after looking at how rev_info.reverse is used, it seem like it's only used in get_revision(), so we can leave it either on or off during the prepare_revision_walk() and the and set appropriately before calling get_revision(), like so: init_revisions(revs); revs.no_walk = REVISION_WALK_NO_WALK_UNSORTED; setup_revisions(...); prepare_revision_walk(revs); revs.reverse = !revs.no_walk; Sorry, but I do not understand why you frutz with reverse after prepare, and not before. I think you can just set no_walk and let setup_revisions() turn it off upon seeing a range (this happens in add_pending_object()). Ah, of course. For some reason I thought that was called from prepare_revision_walk() After setup_revisions() returns, if no_walk is still set, you only got individual refs without ranges, so no reversing required. Yes, it's in the other case (e.g. 'git cherry-pick A..C', when no_walk is not set), that we need to set reverse before walking. You also need to be careful about revert that shares the code; when reverting range A..C in your example, you want to undo C and then B, and you do not want to reverse them. Yep. It looks like this, so should be safe. But thanks for the reminder. if (opts-action != REPLAY_REVERT) opts-revs-reverse ^= 1; -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature request - discard hunk in add --patch mode
On 12-08-15 02:46 PM, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Thomas Rast tr...@student.ethz.ch writes: This has come up before, and actually led to the introduction of 'checkout -p' and 'reset -p': http://thread.gmane.org/gmane.comp.version-control.git/123854 That is a blast from the past. Why is saying git checkout . too much work, after add -p that you excluded the debugging cruft? Please forget this question. A better way in the form of stash -p was suggested in the old thread to get rid of debug cruft in the tree before an add -p session (or during a series of add -p sessions). So is this still an issue? I read most of the thread, and I do think it still is. Here are my 2 cents: 1. The alternative commands aren't nearly as time efficient: - git checkout . is fast and awesome, but you can't use it if, for example, you have to maintain a dirty working tree - git (stash|reset|checkout) -p make you go through (all|most) of the hunks you have to hunt down those 2 lines that say echo 'This line is runningantoeuhaoeuaoae' 2. All of the safety concerns can be alleviated with the right interface. I am glad the u option mentioned in the thread did not go through since I agree it is not ideal. However, if the command is: (a) something with a '!', then no one will hit it by mistake, and (b) the prompt makes it clear that work is lost then I think it would be fine The advantages of a command like this are pretty huge IMO. I can see this being a big time saver. How about adding this to the git add -p prompt: r! - remove this hunk. The hunk is discarded from the working tree, and is irrevocably lost. -- To unsubscribe from this list: send the line 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: Feature request - discard hunk in add --patch mode
Mina Almasry almasry.m...@hotmail.com writes:
On 12-08-15 02:46 PM, Junio C Hamano wrote:
...
Please forget this question. A better way in the form of stash -p
was suggested in the old thread to get rid of debug cruft in the
tree before an add -p session (or during a series of add -p
sessions).
So is this still an issue?
I read most of the thread, and I do think it still is. Here are my 2 cents:
1. The alternative commands aren't nearly as time efficient:
- git checkout . is fast and awesome, but you can't use it if, for
example, you have to maintain a dirty working tree
- git (stash|reset|checkout) -p make you go through (all|most) of
the hunks you have to hunt down those 2 lines that say
echo 'This line is runningantoeuhaoeuaoae'
You have to do that _at least once_ anyway, as there is no other way
for Git to tell which one is debugging cruft and which one is the
real change you value without you telling it. Will return to the
topic later.
2. All of the safety concerns can be alleviated with the right
interface.
There are three safety concerns I raised in the original thread.
Among them, I do not think
(1) new user may mistake undo to be something safe; and
(2) experts will continue making typo between y and u
are the primary risks of the original patch Thomas pointed out.
A much bigger problem with the approach is (3) letting add touch
the working tree breaks the world model. Both experts and newbies
alike, people have learned that git add will never clobber what
they have in the working tree and rely on that promise.
And your key assignment, command renaming or extra prompting do not
change this fundamental issue at all.
Let's step back a bit, and define the problem we are solving.
Suppose you have changes in your working tree that are worth
multiple commits, debugging aid, and uncommittable WIP. You want to
create multiple commits, possibly giving each of them the final
testing before committing, and want to end up with the WIP (plus
possibly the debugging aid, as that may still help your WIP) in your
working tree.
Do we agree that the goal of the discussion of this thread is to
make that process simple, safe, efficient and easy?
Now, back when the original patch by Pierre was proposed, it indeed
was cumbersome. You could sift things through by add -p to build
the first commit in the index, commit, and iterate. In each round,
add -p step had to skip the same debugging aid and WIP over and
over again. If you wanted to give the result of git add -p a
final test before committing, stash save -k would give you the
state you would be committing, but it isn't easy to reintroduce only
the debugging aid to the working tree.
Since then stash -p was added to our toolchest. So theoretically,
we should be able to do something like this:
# start from N-commit worth of change, debug and WIP
git stash save -p debug ;# stash away only the debugging aid
# now we have only N-commit worth of change and WIP
git stash save -p wip ;# stash away WIP
Then after that, you need N round of git add -p git commit.
Now, with what we have already, can we also give final testing
before committing? Each round may now start with:
git add -p ;# prepare the index for the next commit
git stash save -k ;# save away the changes for later commits
and at this point, your working tree and the index has what you are
about to commit. If we can apply the debugging aid stash we
created earlier to the working tree, that would allow you to test
the state you are about to commit with your debugging aid. The
command to do so may be
git stash apply stash@{2}
Once you are satisfied, you can reset this change out of your
working tree with checkout ., and then git commit to record what
is in the index.
And then you can git stash pop to bring back the remainder of
N-commit series worth of changes. You are ready to continue to the
next round.
Once you created N-commit series, you will still have two stash
entries, one debug and one wip. You should be able to resurrect
wip with
git stash pop
just fine, but there is a little problem after this step. Because
git stash [apply|pop] does not want to work on a dirty working
tree, starting from this state just after popping wip stash, you
cannot git stash pop to have both WIP changes and debugging aid to
the working tree.
A topic to improve stash [apply/pop] to allow it may be a valid
and useful thing to do.
As an approximation, without changing any of the current tools,
however, you should be able to do this after creating N-commit
series following the above procedure.
git stash pop ;# resurrect wip to the working tree.
git add -u ;# and add that to the index temporarily
git stash pop ;# resurrect debug to the working tree as well.
git reset ;# then match the index to HEAD
That will mix the WIP and debug again in your working tree.
Why you would want to mix
Re: [PATCH] help: correct behavior for is_executable on Windows
Hi Junio,
On Wed, Aug 15, 2012 at 10:53:55AM -0700, Junio C Hamano wrote:
Heiko Voigt hvo...@hvoigt.net writes:
On Mon, Aug 13, 2012 at 10:48:14AM -0700, Junio C Hamano wrote:
Heiko Voigt hvo...@hvoigt.net writes:
What do you think?
Does having the stat() help on Windows in any way? Does it ever
return an executable bit by itself?
No, AFAIK it does not return anything about executability. But I think
the stat is still necessary to verify that the file exists and is a
regular file.
But if you are going to read it anyway, you can tell it from open()
and read() of the first 2 bytes failing, no? That will still be an
implementation detail of platform specific is_path_executable().
No I am not opening the file anyway. Only when it does not have a
.exe postfix. That at least was the intention of my previous patch in
this thread.
So you are forcing Windows an extra and unnecessary stat() that only
is needed on Cygwin, no?
My first patch in this thread (which lead to this extraction) is about
avoiding the open because it is possibly very costly on our git binaries
in case a virus scanner is running.
The optimized (and correct) check does only look at the given filename
if it contains a .exe postfix. A directory (although unlikely) could
also have such a suffix. Should we then consider that directory to be a
git command? AFAICS there is no such check in the earlier codepath.
A stat is much cheaper since it does not open the file. For scripts the
open is much cheaper than for our binaries because they are much
smaller. For clarification: All the git builtin binaries basically
contain the same code. Even though they are hardlinked, the system (or
the scanner) does still consider them like individual files and executes
a scan for each of them. That at least seems to be the case on a number
of systems.
@@ -1347,7 +1348,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
- compat/win32/poll.o compat/win32/dirent.o
+ compat/win32/poll.o compat/win32/dirent.o \
+ compat/win32/executable.o
Looks sensible, even though the filename does not tell what it does
about executable. is_executable.o might be a better name for them.
Agreed, I was not sure about the name anyway. is_executable.o sounds
better.
diff --git a/help.c b/help.c
index ebc2c42..d9fae3c 100644
--- a/help.c
+++ b/help.c
@@ -106,34 +106,8 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#if defined(WIN32) || defined(__CYGWIN__)
- /* On Windows we cannot use the executable bit. The executable
-* state is determined by extension only. We do this first
-* because with virus scanners opening an executeable for
-* reading is potentially expensive.
-*/
- if (has_extension(name, .exe))
- return S_IXUSR;
-
-#if defined(__CYGWIN__)
-if ((st.st_mode S_IXUSR) == 0)
-#endif
-{ /* now that we know it does not have an executable extension,
- peek into the file instead */
- char buf[3] = { 0 };
- int n;
- int fd = open(name, O_RDONLY);
- st.st_mode = ~S_IXUSR;
- if (fd = 0) {
- n = read(fd, buf, 2);
- if (n == 2)
- /* look for a she-bang */
- if (!strcmp(buf, #!))
- st.st_mode |= S_IXUSR;
- close(fd);
- }
-}
-#endif
+ correct_executable_stat(name, st);
+
Yuck.
Why should we need even a single line of the implementation of a
function that tells if a given pathname contains an executable
command, which we know is platform specific?
On Posix systems, the implementation will be stat() and check
S_IXUSR. On pure Windows, it will be check .exe, or open it and
read the first two bytes. On Cygwin, it will also be check .exe,
stat() and check S_IXUSR, or open it and read the first two bytes.
It is not like the caller of is_executable() needed to run stat for
other purposes on its own and we can optimize by either borrowing
the stat data the caller already collected for us, or returning the
stat data we collected for later use by the caller. The use of stat
is entirely contained in the POSIX implementation of this function.
Why are you so dead-set to shoehorn the different semantics into
struct stat that we know is not an appropriate abstraction across
platforms?
I simply think filling in the correct information into the data
structure we use on all platforms is the most transparent approach. We
could also call this function
correct_executable_stat_if_needed_by_platform()
but I considered that name to be too long. As explained above: To be
fully correct the stat call is still needed to make sure we are not
looking at a directory.
Re: [PATCH] help: correct behavior for is_executable on Windows
Heiko Voigt hvo...@hvoigt.net writes: I do not know why you are against filling that information into struct stat. Because it is *WRONG*. Isn't it a good enough reason? If the issue you are trying to solve were stat emulation on Windows and Cygwin does not give the correct x-bit (and the user sometimes has to work it around with update-index --chmod), and by using other clues the emulation can be improved to give a better result, I agree that it would be a good solution to have the Does it exist as a regular file and ends with .exe? Otherwise does it start with MZ or #!? heuristics in a helper function correct_executable_stat(), and to have the implementation of stat emulation on these two platforms call that shared helper function. But look at the caller again. The problem the caller wants this function to solve is not I want you to stat this file. It has a pathname, and only wants to know if it is an executable file. It does not care about who owns it, what time it was last touched, etc., but calling the incomplete stat emulation on Windows will try to come up with an answer, and the is_executable() function discards most of it. In other words, you are solving a wrong problem with that approach. Run stat() and look at S_IXUSR happens to be an implementation detail that is valid only on POSIX systems for a function to answer the question: Is this an executable file?, and in that specific implementation, the answer to that question appears in the S_IXUSR bit of st.st_mode. That does not mean the struct stat is the best container for the answer to that question on other platforms. So why structure your abstraction around the inappropriate data structure? Between the function (is_executable) and its callers, there is only one bit that needs to be passed. My preference is to remove static int is_executable() function from help.c, have an extern int is_executable(const char *) that has platform-specific implementation in compat/ layer, and call it from help.c::list_commands_in_dir() without any #ifdef. In git-compat-util.h, have something like: #ifdef __CYGWIN__ #define is_executable(path) cygwin_is_executable(path) #else # ifdef WIN32 # define is_executable(path) win32_is_executable(path) # endif #endif #ifndef is_exectutable #define is_executable(path) posix_is_executable(path) #endif extern int is_executable(const char *); I wouldn't mind seeing the implementation of posix_is_executable() in help.c, which will be dead-code on Windows and Cygwin, if that makes linking and Makefile easier. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.7.11.5
The latest maintenance release Git v1.7.11.5 is now available at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 44013d9418ef23dd8bb67e80b27c9327356bfae8 git-1.7.11.5.tar.gz 8e19f56b2f484dc3327f1e8316c114dbe0ee2743 git-htmldocs-1.7.11.5.tar.gz d328241c130bbe38b12adf5702568c1edfff8623 git-manpages-1.7.11.5.tar.gz Also the following public repositories all have a copy of the v1.7.11.5 tag and the maint branch that the tag points at: url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.7.11.5 Release Notes === Fixes since v1.7.11.4 - * The Makefile rule to create assembly output (primarily for debugging purposes) did not create it next to the source. * The code to avoid mistaken attempt to add the object directory itself as its own alternate could read beyond end of a string while comparison. * On some architectures, block-sha1 did not compile correctly when compilers inferred alignment guarantees from our source we did not intend to make. * When talking to a remote running ssh on IPv6 enabled host, whose address is spelled as [HOST]:PORT, we did not parse the address correctly and failed to connect. * git-blame.el (in compat/) have been updated to use Elisp more correctly. * git checkout branchname to come back from a detached HEAD state incorrectly computed reachability of the detached HEAD, resulting in unnecessary warnings. * git mergetool did not support --tool-help option to give the list of supported backends, like git difftool does. * git grep stopped spawning an external grep long time ago, but a duplicated test to check internal and external grep was left behind. Also contains minor typofixes and documentation updates. Changes since v1.7.11.4 are as follows: Heiko Voigt (1): link_alt_odb_entry: fix read over array bounds reported by valgrind Jeff King (1): checkout: don't confuse ref and object flags Jonathan Nieder (4): block-sha1: avoid pointer conversion that violates alignment constraints block-sha1: put expanded macro parameters in parentheses Makefile: fix location of listing produced by make subdir/foo.s Makefile: BLK_SHA1 does not require fast htonl() and unaligned loads Junio C Hamano (4): mergetool: support --tool-help option like difftool does Enumerate revision range specifiers in the documentation Prepare for 1.7.11.5 Git 1.7.11.5 Lawrence Mitchell (2): git-blame.el: Use with-current-buffer where appropriate git-blame.el: Do not use bare 0 to mean (point-min) Max Horn (1): Make refname documentation more consistent. Michael Schubert (1): Documentation/git-daemon: add missing word Ramkumar Ramachandra (1): commit: document a couple of options Ramsay Allan Jones (1): t7810-*.sh: Remove redundant test René Scharfe (1): git: Wrong parsing of ssh urls with IPv6 literals ignores port Rüdiger Sonderfeld (2): git-blame.el: use mapc instead of mapcar git-blame.el: Do not use goto-line in lisp code Štěpán Němec (1): doc: A few minor copy edits. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.7.12-rc3
A release candidate Git v1.7.12-rc3 is now available for testing at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 8719af22c3479b3e21845a6fba0b9c56087a0280 git-1.7.12.rc3.tar.gz 7dbb5ba4f9ed0202e7153e8728561922b3d9a788 git-htmldocs-1.7.12.rc3.tar.gz 6374e277f868d66ce6d5ab7909247bc107830519 git-manpages-1.7.12.rc3.tar.gz Also the following public repositories all have a copy of the v1.7.12-rc3 tag and the master branch that the tag points at: url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.7.12 Release Notes (draft) = Updates since v1.7.11 - UI, Workflows Features * Git can be told to normalize pathnames it read from readdir(3) and all arguments it got from the command line into precomposed UTF-8 (assuming that they come as decomposed UTF-8), in order to work around issues on Mac OS. I think there still are other places that need conversion (e.g. paths that are read from stdin for some commands), but this should be a good first step in the right direction. * Per-user $HOME/.gitconfig file can optionally be stored in $HOME/.config/git/config instead, which is in line with XDG. * The value of core.attributesfile and core.excludesfile default to $HOME/.config/git/attributes and $HOME/.config/git/ignore respectively when these files exist. * Logic to disambiguate abbreviated object names have been taught to take advantage of object types that are expected in the context, e.g. XX in the git describe output v1.2.3-gXX must be a commit object, not a blob nor a tree. This will help us prolong the lifetime of abbreviated object names. * git apply learned to wiggle the base version and perform three-way merge when a patch does not exactly apply to the version you have. * Scripted Porcelain writers now have access to the credential API via the git credential plumbing command. * git help used to always default to man format even on platforms where man viewer is not widely available. * git clone --local $path started its life as an experiment to optionally use link/copy when cloning a repository on the disk, but we didn't deprecate it after we made the option a no-op to always use the optimization. The command learned --no-local option to turn this off, as a more explicit alternative over use of file:// URL. * git fetch and friends used to say remote side hung up unexpectedly when they failed to get response they expect from the other side, but one common reason why they don't get expected response is that the remote repository does not exist or cannot be read. The error message in this case was updated to give better hints to the user. * git help -w $cmd can show HTML version of documentation for git-$cmd by setting help.htmlpath to somewhere other than the default location where the build procedure installs them locally; the variable can even point at a http:// URL. * git rebase [-i] --root $tip can now be used to rewrite all the history leading to $tip down to the root commit. * git rebase -i learned -x cmd to insert exec cmd after each commit in the resulting history. * git status gives finer classification to various states of paths in conflicted state and offer advice messages in its output. * git submodule learned to deal with nested submodule structure where a module is contained within a module whose origin is specified as a relative URL to its superproject's origin. * A rather heavy-ish git completion script has been split to create a separate git prompting script, to help lazy-autoloading of the completion part while making prompting part always available. * gitweb pays attention to various forms of credits that are similar to Signed-off-by: lines in the commit objects and highlights them accordingly. Foreign Interface * mediawiki remote helper (in contrib/) learned to handle file attachments. * git p4 now uses Jobs: and p4 move when appropriate. * vcs-svn has been updated to clean-up compilation, lift 32-bit limitations, etc. Performance, Internal Implementation, etc. (please report possible regressions) * Some tests showed false failures caused by a bug in ecryptofs. * We no longer use AsciiDoc7 syntax in our documentation and favor a more modern style. * git am --rebasing codepath was taught to grab authorship, log message and the patch text directly out of existing commits. This will help rebasing commits that have confusing diff output in their log messages. * git index-pack and git pack-objects use streaming API to read from the object store to avoid having to
Re: [PATCH] help: correct behavior for is_executable on Windows
Junio C Hamano gits...@pobox.com writes:
My preference is to remove static int is_executable() function
from help.c, have an...
... I wouldn't mind seeing the implementation of posix_is_executable()
in help.c, which will be dead-code on Windows and Cygwin, if that
makes linking and Makefile easier.
An outline of such a change might look like this.
As usual, I am not great at names, so you may want to rename the
shared one. Also I do not know what headers Cygwin and Windows
specific bits would need to include, so you would have to include
more than just git-compat-util.h in the compat/*.c files this
patch adds.
It would make sense to move the does it end with .exe check from
the caller to the shared function, but because that does not match
the concept of the name I came up with (peek-contents), I left it
out of the function. If there is a good name to express what such a
combined ends with .exe, or inspected contents look like an
executable, it would make sense to rename it as such and move
.exe check to it.
Thanks.
Makefile | 6 --
compat/cygwin.c | 16
compat/win32/is_executable.c | 14 ++
compat/win32/peek_contents.c | 16
git-compat-util.h| 9 +
help.c | 32 +---
is_executable.c | 12
7 files changed, 72 insertions(+), 33 deletions(-)
diff --git a/Makefile b/Makefile
index 6b0c961..2bcb9f4 100644
--- a/Makefile
+++ b/Makefile
@@ -734,6 +734,7 @@ LIB_OBJS += hash.o
LIB_OBJS += help.o
LIB_OBJS += hex.o
LIB_OBJS += ident.o
+LIB_OBJS += is_executable.o
LIB_OBJS += kwset.o
LIB_OBJS += levenshtein.o
LIB_OBJS += list-objects.o
@@ -1073,7 +1074,7 @@ ifeq ($(uname_O),Cygwin)
# Try commenting this out if you suspect MMAP is more efficient
NO_MMAP = YesPlease
X = .exe
- COMPAT_OBJS += compat/cygwin.o
+ COMPAT_OBJS += compat/cygwin.o compat/win32/peek_contents.o
UNRELIABLE_FSTAT = UnfortunatelyYes
SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield
endif
@@ -1257,7 +1258,8 @@ ifeq ($(uname_S),Windows)
BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild
-Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H
-D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
COMPAT_OBJS = compat/msvc.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
- compat/win32/poll.o compat/win32/dirent.o
+ compat/win32/poll.o compat/win32/dirent.o \
+ compat/win32/is_executable.o compat/win32/peek_contents.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
EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
diff --git a/compat/cygwin.c b/compat/cygwin.c
index dfe9b30..5eb948a 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -140,3 +140,19 @@ static int cygwin_lstat_stub(const char *file_name, struct
stat *buf)
stat_fn_t cygwin_stat_fn = cygwin_stat_stub;
stat_fn_t cygwin_lstat_fn = cygwin_lstat_stub;
+extern int win32_peek_contents_executable(const char *);
+
+int cygwin_is_path_executable(const char *name)
+{
+ struct stat st;
+
+ /* stat, not lstat */
+ if (stat(name, st) || !S_ISREG(st.st_mode))
+ return 0;
+ if (st.st_mode S_IXUSR)
+ return 1;
+
+ if (has_extension(name, .exe))
+ return 1;
+ return win32_peek_contents_executable(name);
+}
diff --git a/compat/win32/is_executable.c b/compat/win32/is_executable.c
new file mode 100644
index 000..f7907b3
--- /dev/null
+++ b/compat/win32/is_executable.c
@@ -0,0 +1,14 @@
+#include git-compat-util.h
+
+int win32_path_is_executable(const char *name)
+{
+ /*
+* Do whatever you would do on win32 to make sure
+* name exists here
+*/
+ if (!file_exists(name))
+ return 0;
+ if (has_extension(name, .exe))
+ return 1;
+ return win32_peek_contents_executable(name);
+}
diff --git a/compat/win32/peek_contents.c b/compat/win32/peek_contents.c
new file mode 100644
index 000..5e425aa
--- /dev/null
+++ b/compat/win32/peek_contents.c
@@ -0,0 +1,16 @@
+#include git-compat-util.h
+
+int win32_peek_contents_executable(const char *name)
+{
+ char buf[2];
+ int n, fd;
+
+ fd = open(name, O_RDONLY);
+ if (fd 0)
+ return 0;
+ n = read(fd, buf, 2);
+ close(fd);
+ return (n == 2
+ /* DOS executables start with MZ */
+ (!memcmp(buf, #!, 2) || !memcmp(buf, MZ, 2)));
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 35b095e..e8f8cb1 100644
--- a/git-compat-util.h
Re: Can't track untracked file
Marco S Hyman m...@snafu.org writes: And there is NOTHING I can do to get that directory into git. $ git add 2010 $ git commit -m 'will it work?' # On branch master # Untracked files: # (use git add file... to include in what will be committed) # # 2010/ nothing added to commit but untracked files present (use git add to track) The directory is not empty. Perhaps 2010 or everything in it is ignored in .gitignore in higher directories? -- To unsubscribe from this list: send the line 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/RFC] index-pack: produce pack index version 3
Shawn Pearce spea...@spearce.org writes:
... But I think its worth giving
him a few weeks to finish getting the code ready, vs. rushing
something in that someone else thinks might help. We have waited more
than 6 years or whatever to improve packing. Colby's experiments are
showing massive improvements (40s enumeration cut to 100ms) with low
disk usage increase (10%) and no pack file format changes.
No matter what you do, you would be saving the bitmap somewhere
outside the *.pack file, yes? Will it be in some extended section
of the new *.idx file?
With the bitmap, your object enumeration phase may go very fast, and
you would be able to figure out, in response to a fetch request I
have these tips of refs; please update me with these refs of yours,
the set of objects you would need to pack (i.e. the ones that are
reachable from your refs that were asked for, but that are not
reachable from the refs the requestor has).
Among these objects, there will be ones that are expressed as a
delta against what you are going to send, or as a delta against what
you know the recipient must already have (if you are using thin pack
transfer) in the packfiles you have, and you can send these deltas
as-is without recomputation.
But there will be ones that are either expressed as a base in your
packfile, or as a delta against something you are not going to send
and you know that the recipient does not have. In order to turn
these objects into deltas, it may be necessary to have a way to
record which delta chain each object belongs to, and if you are
introducing the mechanism to have extended sections in the new *.idx
file, that may be a good place to do so. When you need to express
an object that your bitmap told you to send (as opposed to rev-list
walking told you with the paths to the objects), you can find other
objects that belong to the same delta chain and that you know are
available to the recipient when it starts to fixing the thin pack
using that extra piece of information, in order to find the optimal
delta base to encode such an object against.
Just for fun, I applied the attached patch and repacked the history
leading to v1.7.12-rc3 with the default depth/window:
git rev-list --objects --all |
git pack-objects \
--no-reuse-delta --no-reuse-object --delta-base-offset \
[--no-namehash] pack
with and without the experimental --no-namehash option. The result
is staggering. With name-hash to group objects from the same path
close together in the delta window, the resulting pack is 33M.
Without the name-hash hint, the same pack is 163M! Needless to say,
keeping the objects that should be hashed together inside a delta
window is really important.
An obvious way to record the delta chain is to simply keep the
name_hash of each object in the pack, which would need 2 bytes per
object in the pack, that would bloat pack_idx_entry size from 32
bytes to 34 bytes per entry. That way, after your bitmap discovers
an object that cannot reuse existing deltas, you can throw it, other
such objects with the same name-hash, and then objects that you know
will be available to the recipient (you mark the last category of
objects as preferred base), into the delta_list so that they are
close together in the delta window.
And here is a patch to experiment the name-hash based grouping
heuristics.
builtin/pack-objects.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git c/builtin/pack-objects.c w/builtin/pack-objects.c
index 782e7d0..a2dd67b 100644
--- c/builtin/pack-objects.c
+++ w/builtin/pack-objects.c
@@ -67,6 +67,7 @@ static int keep_unreachable, unpack_unreachable, include_tag;
static unsigned long unpack_unreachable_expiration;
static int local;
static int incremental;
+static int use_name_hash = 1;
static int ignore_packed_keep;
static int allow_ofs_delta;
static struct pack_idx_option pack_idx_opts;
@@ -863,7 +864,7 @@ static unsigned name_hash(const char *name)
{
unsigned c, hash = 0;
- if (!name)
+ if (!name || !use_name_hash)
return 0;
/*
@@ -2468,6 +2469,8 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
limit pack window by memory in addition to object
limit),
OPT_INTEGER(0, depth, depth,
maximum length of delta chain allowed in the
resulting pack),
+ OPT_BOOL(0, namehash, use_name_hash,
+assign name hint to each path),
OPT_BOOL(0, reuse-delta, reuse_delta,
reuse existing deltas),
OPT_BOOL(0, reuse-object, reuse_object,
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
