Git development
 help / color / mirror / Atom feed
* [PATCH 18/21] Convert refresh_index to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-06  6:21 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1357453268-12543-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c    | 14 ++++++--------
 builtin/commit.c |  2 +-
 builtin/rm.c     |  2 +-
 cache.h          |  2 +-
 read-cache.c     |  5 +++--
 5 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index af36bc4..623f167 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -180,19 +180,17 @@ static void treat_gitlinks(const char **pathspec)
 	}
 }
 
-static void refresh(int verbose, const char **pathspec)
+static void refresh(int verbose, const struct pathspec *pathspec)
 {
 	char *seen;
-	int i, specs;
+	int i;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
+	seen = xcalloc(pathspec->nr, 1);
 	refresh_index(&the_index, verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET,
 		      pathspec, seen, _("Unstaged changes after refreshing the index:"));
-	for (i = 0; i < specs; i++) {
+	for (i = 0; i < pathspec->nr; i++) {
 		if (!seen[i])
-			die(_("pathspec '%s' did not match any files"), pathspec[i]);
+			die(_("pathspec '%s' did not match any files"), pathspec->raw[i]);
 	}
         free(seen);
 }
@@ -437,7 +435,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	}
 
 	if (refresh_only) {
-		refresh(verbose, pathspec.raw);
+		refresh(verbose, &pathspec);
 		goto finish;
 	}
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 71c5afb..193a37e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1208,7 +1208,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		parse_pathspec(&s.pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
 
 	read_cache_preload(&s.pathspec);
-	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec.raw, NULL, NULL);
+	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &s.pathspec, NULL, NULL);
 
 	fd = hold_locked_index(&index_lock, 0);
 	if (0 <= fd)
diff --git a/builtin/rm.c b/builtin/rm.c
index d719d95..b5edde8 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -250,7 +250,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	}
 
 	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
-	refresh_index(&the_index, REFRESH_QUIET, pathspec.raw, NULL, NULL);
+	refresh_index(&the_index, REFRESH_QUIET, &pathspec, NULL, NULL);
 
 	seen = NULL;
 	seen = xcalloc(pathspec.nr, 1);
diff --git a/cache.h b/cache.h
index 3c34ef5..41e1421 100644
--- a/cache.h
+++ b/cache.h
@@ -511,7 +511,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
 #define REFRESH_IGNORE_MISSING	0x0008	/* ignore non-existent */
 #define REFRESH_IGNORE_SUBMODULES	0x0010	/* ignore submodules */
 #define REFRESH_IN_PORCELAIN	0x0020	/* user friendly output, not "needs update" */
-extern int refresh_index(struct index_state *, unsigned int flags, const char **pathspec, char *seen, const char *header_msg);
+extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg);
 
 struct lock_file {
 	struct lock_file *next;
diff --git a/read-cache.c b/read-cache.c
index fda78bc..dec2ba6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1093,7 +1093,8 @@ static void show_file(const char * fmt, const char * name, int in_porcelain,
 	printf(fmt, name);
 }
 
-int refresh_index(struct index_state *istate, unsigned int flags, const char **pathspec,
+int refresh_index(struct index_state *istate, unsigned int flags,
+		  const struct pathspec *pathspec,
 		  char *seen, const char *header_msg)
 {
 	int i;
@@ -1128,7 +1129,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 			continue;
 
 		if (pathspec &&
-		    !match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
+		    !match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen))
 			filtered = 1;
 
 		if (ce_stage(ce)) {
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 20/21] Convert add_files_to_cache to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-06  6:21 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1357453268-12543-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c    | 8 +++++---
 builtin/commit.c | 2 +-
 cache.h          | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index f5ceb5c..641037f 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -80,13 +80,15 @@ static void update_callback(struct diff_queue_struct *q,
 	}
 }
 
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+int add_files_to_cache(const char *prefix,
+		       const struct pathspec *pathspec, int flags)
 {
 	struct update_callback_data data;
 	struct rev_info rev;
 	init_revisions(&rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
-	init_pathspec(&rev.prune_data, pathspec);
+	if (pathspec)
+		rev.prune_data = *pathspec;
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	data.flags = flags;
@@ -464,7 +466,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	plug_bulk_checkin();
 
-	exit_status |= add_files_to_cache(prefix, pathspec.raw, flags);
+	exit_status |= add_files_to_cache(prefix, &pathspec, flags);
 
 	if (add_new_files)
 		exit_status |= add_files(&dir, flags);
diff --git a/builtin/commit.c b/builtin/commit.c
index 193a37e..a3a22b8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -330,7 +330,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
 	 */
 	if (all || (also && pathspec.nr)) {
 		fd = hold_locked_index(&index_lock, 1);
-		add_files_to_cache(also ? prefix : NULL, pathspec.raw, 0);
+		add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
 		refresh_cache_or_die(refresh_flags);
 		update_main_cache_tree(WRITE_TREE_SILENT);
 		if (write_cache(fd, active_cache, active_nr) ||
diff --git a/cache.h b/cache.h
index 41e1421..10cba21 100644
--- a/cache.h
+++ b/cache.h
@@ -1224,7 +1224,7 @@ void packet_trace_identity(const char *prog);
  * return 0 if success, 1 - if addition of a file failed and
  * ADD_FILES_IGNORE_ERRORS was specified in flags
  */
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags);
+int add_files_to_cache(const char *prefix, const struct pathspec *pathspec, int flags);
 
 /* diff.c */
 extern int diff_auto_refresh_index;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 19/21] Convert {read,fill}_directory to take struct pathspec
From: Nguyễn Thái Ngọc Duy @ 2013-01-06  6:21 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1357453268-12543-1-git-send-email-pclouds@gmail.com>


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c      |  2 +-
 builtin/clean.c    |  2 +-
 builtin/grep.c     |  2 +-
 builtin/ls-files.c |  2 +-
 dir.c              | 10 +++++-----
 dir.h              |  4 ++--
 wt-status.c        |  4 ++--
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 623f167..f5ceb5c 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -429,7 +429,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		}
 
 		/* This picks up the paths that are not tracked */
-		baselen = fill_directory(&dir, pathspec.raw);
+		baselen = fill_directory(&dir, &pathspec);
 		if (pathspec.nr)
 			seen = prune_directory(&dir, pathspec.raw, baselen);
 	}
diff --git a/builtin/clean.c b/builtin/clean.c
index 788ad8c..41c8cad 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -103,7 +103,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	parse_pathspec(&pathspec, PATHSPEC_FROMTOP, 0, prefix, argv);
 
-	fill_directory(&dir, pathspec.raw);
+	fill_directory(&dir, &pathspec);
 
 	if (pathspec.nr)
 		seen = xmalloc(pathspec.nr);
diff --git a/builtin/grep.c b/builtin/grep.c
index 705f9ff..f370bad 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -522,7 +522,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
 	if (exc_std)
 		setup_standard_excludes(&dir);
 
-	fill_directory(&dir, pathspec->raw);
+	fill_directory(&dir, pathspec);
 	for (i = 0; i < dir.nr; i++) {
 		const char *name = dir.entries[i]->name;
 		int namelen = strlen(name);
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index be6e05d..7bb637b 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -216,7 +216,7 @@ static void show_files(struct dir_struct *dir)
 
 	/* For cached/deleted files we don't need to even do the readdir */
 	if (show_others || show_killed) {
-		fill_directory(dir, pathspec.raw);
+		fill_directory(dir, &pathspec);
 		if (show_others)
 			show_other_files(dir);
 		if (show_killed)
diff --git a/dir.c b/dir.c
index 11e8c1d..cfd9dac 100644
--- a/dir.c
+++ b/dir.c
@@ -72,7 +72,7 @@ char *common_prefix(const char **pathspec)
 	return len ? xmemdupz(*pathspec, len) : NULL;
 }
 
-int fill_directory(struct dir_struct *dir, const char **pathspec)
+int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
 {
 	size_t len;
 
@@ -80,10 +80,10 @@ int fill_directory(struct dir_struct *dir, const char **pathspec)
 	 * Calculate common prefix for the pathspec, and
 	 * use that to optimize the directory walk
 	 */
-	len = common_prefix_len(pathspec);
+	len = common_prefix_len(pathspec->raw);
 
 	/* Read the directory and prune it */
-	read_directory(dir, pathspec ? *pathspec : "", len, pathspec);
+	read_directory(dir, pathspec->nr ? pathspec->raw[0] : "", len, pathspec);
 	return len;
 }
 
@@ -1211,14 +1211,14 @@ static int treat_leading_path(struct dir_struct *dir,
 	return rc;
 }
 
-int read_directory(struct dir_struct *dir, const char *path, int len, const char **pathspec)
+int read_directory(struct dir_struct *dir, const char *path, int len, const struct pathspec *pathspec)
 {
 	struct path_simplify *simplify;
 
 	if (has_symlink_leading_path(path, len))
 		return dir->nr;
 
-	simplify = create_simplify(pathspec);
+	simplify = create_simplify(pathspec ? pathspec->raw : NULL);
 	if (!len || treat_leading_path(dir, path, len, simplify))
 		read_directory_recursive(dir, path, len, 0, simplify);
 	free_simplify(simplify);
diff --git a/dir.h b/dir.h
index 1d4888b..b51d2e9 100644
--- a/dir.h
+++ b/dir.h
@@ -74,8 +74,8 @@ extern int match_pathspec_depth(const struct pathspec *pathspec,
 				int prefix, char *seen);
 extern int within_depth(const char *name, int namelen, int depth, int max_depth);
 
-extern int fill_directory(struct dir_struct *dir, const char **pathspec);
-extern int read_directory(struct dir_struct *, const char *path, int len, const char **pathspec);
+extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec);
+extern int read_directory(struct dir_struct *, const char *path, int len, const struct pathspec *pathspec);
 
 extern int excluded_from_list(const char *pathname, int pathlen, const char *basename,
 			      int *dtype, struct exclude_list *el);
diff --git a/wt-status.c b/wt-status.c
index 13e6aba..2e1a62b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -502,7 +502,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 			DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
 	setup_standard_excludes(&dir);
 
-	fill_directory(&dir, s->pathspec.raw);
+	fill_directory(&dir, &s->pathspec);
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
 		if (cache_name_is_other(ent->name, ent->len) &&
@@ -514,7 +514,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 	if (s->show_ignored_files) {
 		dir.nr = 0;
 		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
-		fill_directory(&dir, s->pathspec.raw);
+		fill_directory(&dir, &s->pathspec);
 		for (i = 0; i < dir.nr; i++) {
 			struct dir_entry *ent = dir.entries[i];
 			if (cache_name_is_other(ent->name, ent->len) &&
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH 21/21] Convert more init_pathspec() to parse_pathspec()
From: Nguyễn Thái Ngọc Duy @ 2013-01-06  6:21 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1357453268-12543-1-git-send-email-pclouds@gmail.com>

init_pathspec() was introduced to work with the result from
get_pathspec(). init_pathspec() will be removed eventually after
parse_pathspec() takes over, so that there is only place that
initializes struct pathspec.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 archive.c          |  2 +-
 builtin/log.c      |  2 +-
 builtin/ls-files.c | 10 ++++------
 diff-lib.c         |  2 +-
 merge-recursive.c  |  2 +-
 5 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/archive.c b/archive.c
index 530badb..3caa31f 100644
--- a/archive.c
+++ b/archive.c
@@ -218,7 +218,7 @@ static int path_exists(struct tree *tree, const char *path)
 	struct pathspec pathspec;
 	int ret;
 
-	init_pathspec(&pathspec, paths);
+	parse_pathspec(&pathspec, 0, 0, "", paths);
 	ret = read_tree_recursive(tree, "", 0, 0, &pathspec, reject_entry, NULL);
 	free_pathspec(&pathspec);
 	return ret != 0;
diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..495ae77 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -455,7 +455,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 	init_grep_defaults();
 	git_config(git_log_config, NULL);
 
-	init_pathspec(&match_all, NULL);
+	memset(&match_all, 0, sizeof(match_all));
 	init_revisions(&rev, prefix);
 	rev.diff = 1;
 	rev.always_show_header = 1;
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7bb637b..79949de 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -318,13 +318,11 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)
 	}
 
 	if (prefix) {
-		static const char *(matchbuf[2]);
-		matchbuf[0] = prefix;
-		matchbuf[1] = NULL;
-		init_pathspec(&pathspec, matchbuf);
-		pathspec.items[0].nowildcard_len = pathspec.items[0].len;
+		static const char *(matchbuf[1]);
+		matchbuf[0] = NULL;
+		parse_pathspec(&pathspec, 0, 0, prefix, matchbuf);
 	} else
-		init_pathspec(&pathspec, NULL);
+		memset(&pathspec, 0, sizeof(pathspec));
 	if (read_tree(tree, 1, &pathspec))
 		die("unable to read tree entries %s", tree_name);
 
diff --git a/diff-lib.c b/diff-lib.c
index f35de0f..9c07f6a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -500,7 +500,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
 	struct rev_info revs;
 
 	init_revisions(&revs, NULL);
-	init_pathspec(&revs.prune_data, opt->pathspec.raw);
+	revs.prune_data = opt->pathspec;
 	revs.diffopt = *opt;
 
 	if (diff_cache(&revs, tree_sha1, NULL, 1))
diff --git a/merge-recursive.c b/merge-recursive.c
index d882060..cd95bdb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -297,7 +297,7 @@ static int get_files_dirs(struct merge_options *o, struct tree *tree)
 {
 	int n;
 	struct pathspec match_all;
-	init_pathspec(&match_all, NULL);
+	memset(&match_all, 0, sizeof(match_all));
 	if (read_tree_recursive(tree, "", 0, 0, &match_all, save_files_dirs, o))
 		return 0;
 	n = o->current_file_set.nr + o->current_directory_set.nr;
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* RE: Trying to understand the web dav details
From: Jason Pyeron @ 2013-01-06  6:20 UTC (permalink / raw)
  To: 'git'
In-Reply-To: <20130106053807.GA8551@sigill.intra.peff.net>

Ignore everything below, it was a case sensitive typo. It always worked it.

> -----Original Message-----
> From: Jeff King
> Sent: Sunday, January 06, 2013 0:38
> 
> On Sun, Jan 06, 2013 at 04:49:57AM +0000, Pyeron, Jason J CTR 
> (US) wrote:
> 
> > > > How does the ?service=xxxx get translated in to the action 
> > > > performed on the web server?
> > > 
> > > If you are using the git-http-backend CGI, it will interpret the 
> > > service
> > 
> > No, using plain jane http and webdav. This server is not 
> "allowed" to 
> > use cgi processes.
> 
> Then the service parameter should be ignored by your 
> webserver, and it should just serve the info/refs file from 
> the repository on the filesystem. And you are stuck using 
> WebDAV for push.
> 
> > > GET /git/project-x/info/refs HTTP/1.1
> > [...]
> > * The requested URL returned error: 404 Not Found
> 
> Does the info/refs file exist in the project-x repository?

Yes.

> 
> > fatal: https://server/git/project-x/info/refs not found: 
> did you run git update-server-info on the server?
> 
> Did you?
> 

Many times.

> If you can't run any git programs on the server at all (and 
> it sounds like that may be the case), you'll need to run it 
> locally before putting the repository data on the server.
> 
> Once you have WebDAV set up for pushing, it will update the 
> info/refs file for each push. But if you are initially 
> seeding the server with rsync or a tarfile, you'll want to 

Seeding it seems to work, it is the bare init that seems to be failing. Might be
on to something there.

> make sure it has an up-to-date info/refs file.

Here is the create script:

#!/bin/bash

if [ $# != 1 ]; then
exit 1;
fi

if [ -e "$1" ]; then
exit 2;
fi

mkdir "$1"
cd "$1"
git init --bare
cp hooks/post-update.sample hooks/post-update
chmod +x hooks/post-update
git update-server-info




--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-                                                               -
- Jason Pyeron                      PD Inc. http://www.pdinc.us -
- Principal Consultant              10 West 24th Street #100    -
- +1 (443) 269-1555 x333            Baltimore, Maryland 21218   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.

 

^ permalink raw reply

* RE: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Jason Pyeron @ 2013-01-06  6:29 UTC (permalink / raw)
  To: git
In-Reply-To: <1890551.8jTmplCF6O@thunderbird>

> -----Original Message-----
> From: Stephen & Linda Smith 
> Sent: Sunday, January 06, 2013 1:21
> 
> > Was it the commit before
> > 9fca6cffc05321445b59c91e8f8d308f41588b53 that compiles or was it 
> > 9fca6cffc05321445b59c91e8f8d308f41588b53 that compiled? I 
> am doing a 
> > cygwin update presently to look at it.
> 
> Since the email earlier today, I had blown away the 
> directory.   I just now 
> did the following
> 
> git clone https://github.com/git/git.git git-src && cd 
> git-src && make all
> ...   The make errored out as before
> 

No error for me.

> git co 9fca6c && make all
> ...   The make errored out as before

No error for me.

> 
> git co 9fca6c^  && make all
> ... and this compiles to completion
> 
> CYGWIN_NT-5.1 WINXPMACHINE 1.7.14(0.260/5/3) 2012-04-24 17:22 
> i686 Cygwin

This is old, do you have the luxury of updating it?

> 
> What else can I do to test this out (I will get a current 
> cygwin tomorrow to use in a test).

I would also check to see if your devel packages are up to date too.


--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-                                                               -
- Jason Pyeron                      PD Inc. http://www.pdinc.us -
- Principal Consultant              10 West 24th Street #100    -
- +1 (443) 269-1555 x333            Baltimore, Maryland 21218   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.

 

^ permalink raw reply

* Re: [PATCH] clone: support atomic operation with --separate-git-dir
From: Junio C Hamano @ 2013-01-06  6:43 UTC (permalink / raw)
  To: Jens Lehmann
  Cc: Heiko Voigt, git, Manlio Perillo, W. Trevor King,
	Nguyen Thai Ngoc Duy
In-Reply-To: <50E88A40.9010904@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Since b57fb80a7d (init, clone: support --separate-git-dir for .git file)
> git clone supports the --separate-git-dir option to create the git dir
> outside the work tree. But when that option is used, the git dir won't be
> deleted in case the clone fails like it would be without this option. This
> makes clone lose its atomicity as in case of a failure a partly set up git
> dir is left behind. A real world example where this leads to problems is
> when "git submodule update" fails to clone a submodule and later calls to
> "git submodule update" stumble over the partially set up git dir and try
> to revive the submodule from there, which then fails with a not very user
> friendly error message.
>
> Fix that by updating the junk_git_dir variable (used to remember if and
> what git dir should be removed in case of failure) to the new value given
> with the --seperate-git-dir option. Also add a test for this to t5600 (and
> while at it fix the former last test to not cd into a directory to test
> for its existence but use "test -d" instead).
>
> Reported-by: Manlio Perillo <manlio.perillo@gmail.com>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---

I hate to see that git_link is not an argument to init_db() but is a
file-scope static in init-db.c to be used to communicate between
set_git_dir_init() and init_db(), but that would be a separate thing
to be cleaned up, I guess.

How is the file that points at the real git dir removed with this
fix, by the way?

Thanks.

^ permalink raw reply

* Re: [PATCH] Documentation: fix man page dependency on asciidoc.conf
From: Junio C Hamano @ 2013-01-06  6:51 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: John Keeping, git, Sergey Vlasov
In-Reply-To: <20130105232800.GF3247@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

> John Keeping wrote:
>
>> When building manual pages, the source text is transformed to XML with
>> AsciiDoc before the man pages are generated from the XML with xmlto.
>>
>> Fix the dependency in the Makefile so that the XML files are rebuilt
>> when asciidoc.conf changes and not just the manual pages from unchanged
>> XML.
>
> Good catch, thanks.
>
> Would something like the following make sense, to make it more obvious
> how the dependency needs to be adjusted if we change the $(ASCIIDOC)
> command line for some reason?

I think such a more explicit approach is easier to understand, than
a separate "By the way, I do not define any rule to build these
targets using asciidoc.conf, but I know they depend on it" rule.

Care to do a real patch?

Thanks.


> diff --git i/Documentation/Makefile w/Documentation/Makefile
> index e53d333e..971977b8 100644
> --- i/Documentation/Makefile
> +++ w/Documentation/Makefile
> @@ -178,8 +178,6 @@ all: html man
>  
>  html: $(DOC_HTML)
>  
> -$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7): asciidoc.conf
> -
>  man: man1 man5 man7
>  man1: $(DOC_MAN1)
>  man5: $(DOC_MAN5)
> @@ -257,7 +255,7 @@ clean:
>  	$(RM) $(cmds_txt) *.made
>  	$(RM) manpage-base-url.xsl
>  
> -$(MAN_HTML): %.html : %.txt
> +$(MAN_HTML): %.html : %.txt asciidoc.conf
>  	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
>  	$(ASCIIDOC) -b xhtml11 -d manpage -f asciidoc.conf \
>  		$(ASCIIDOC_EXTRA) -agit_version=$(GIT_VERSION) -o $@+ $< && \
> @@ -270,7 +268,7 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
>  	$(QUIET_XMLTO)$(RM) $@ && \
>  	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
>  
> -%.xml : %.txt
> +%.xml : %.txt asciidoc.conf
>  	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
>  	$(ASCIIDOC) -b docbook -d manpage -f asciidoc.conf \
>  		$(ASCIIDOC_EXTRA) -agit_version=$(GIT_VERSION) -o $@+ $< && \
> @@ -286,7 +284,7 @@ technical/api-index.txt: technical/api-index-skel.txt \
>  	$(QUIET_GEN)cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh
>  
>  technical/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
> -$(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt
> +$(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt asciidoc.conf
>  	$(QUIET_ASCIIDOC)$(ASCIIDOC) -b xhtml11 -f asciidoc.conf \
>  		$(ASCIIDOC_EXTRA) -agit_version=$(GIT_VERSION) $*.txt
>  
> diff --git i/t/test-terminal.perl w/t/test-terminal.perl
> index 10172aee..1fb373f2 100755
> --- i/t/test-terminal.perl
> +++ w/t/test-terminal.perl
> @@ -31,7 +31,7 @@ sub finish_child {
>  	} elsif ($? & 127) {
>  		my $code = $? & 127;
>  		warn "died of signal $code";
> -		return $code - 128;
> +		return $code + 128;
>  	} else {
>  		return $? >> 8;
>  	}

^ permalink raw reply

* Re: [PATCH] archive-tar: split long paths more carefully
From: Junio C Hamano @ 2013-01-06  6:54 UTC (permalink / raw)
  To: René Scharfe; +Cc: git
In-Reply-To: <50E8AE12.8040102@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> The name field of a tar header has a size of 100 characters.  This limit
> was extended long ago in a backward compatible way by providing the
> additional prefix field, which can hold 155 additional characters.  The
> actual path is constructed at extraction time by concatenating the prefix
> field, a slash and the name field.
>
> get_path_prefix() is used to determine which slash in the path is used as
> the cutting point and thus which part of it is placed into the field
> prefix and which into the field name.  It tries to cram as much into the
> prefix field as possible.  (And only if we can't fit a path into the
> provided 255 characters we use a pax extended header to store it.)
>
> If a path is longer than 100 but shorter than 156 characters and ends
> with a slash (i.e. is for a directory) then get_path_prefix() puts the
> whole path in the prefix field and leaves the name field empty.  GNU tar
> reconstructs the path without complaint, but the tar included with
> NetBSD 6 does not: It reports the header to be invalid.
>
> For compatibility with this version of tar, make sure to never leave the
> name field empty.  In order to do that, trim the trailing slash from the
> part considered as possible prefix, if it exists -- that way the last
> path component (or more, but not less) will end up in the name field.

Nicely explained; thanks.

Makes me wonder what we should do for a file inside a directory
whose name is 10 bytes long, and whose filename is 120 bytes long,
though.

Sounds like people on NetBSD are SOL due to the 155+'/'+100 in such
a case and there is nothing we can do, I guess.

> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
>  archive-tar.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/archive-tar.c b/archive-tar.c
> index 0ba3f25..923daf5 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -153,6 +153,8 @@ static unsigned int ustar_header_chksum(const struct ustar_header *header)
>  static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)
>  {
>  	size_t i = pathlen;
> +	if (i > 1 && path[i - 1] == '/')
> +		i--;
>  	if (i > maxlen)
>  		i = maxlen;
>  	do {

^ permalink raw reply

* Re: [PATCH] run-command: encode signal death as a positive integer
From: Junio C Hamano @ 2013-01-06  7:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, git, Bart Trojanowski
In-Reply-To: <20130105144949.GA24479@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sat, Jan 05, 2013 at 09:03:16AM -0500, Jeff King wrote:
> ...
> The downside is that callers of run_command can no longer
> differentiate between a signal received directly by the
> sub-process, and one propagated. However, no caller
> currently cares, and since we already optimize out some
> calls to the shell under the hood, that distinction is not
> something that should be relied upon by callers.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Very nicely explained.  Thanks.

>  Documentation/technical/api-run-command.txt | 6 ++----
>  editor.c                                    | 2 +-
>  run-command.c                               | 2 +-
>  3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt
> index f18b4f4..5d7d7f2 100644
> --- a/Documentation/technical/api-run-command.txt
> +++ b/Documentation/technical/api-run-command.txt
> @@ -55,10 +55,8 @@ The functions above do the following:
>    non-zero.
>  
>  . If the program terminated due to a signal, then the return value is the
> -  signal number - 128, ie. it is negative and so indicates an unusual
> -  condition; a diagnostic is printed. This return value can be passed to
> -  exit(2), which will report the same code to the parent process that a
> -  POSIX shell's $? would report for a program that died from the signal.
> +  signal number + 128, ie. the same value that a POSIX shell's $? would
> +  report.  A diagnostic is printed.
>  
>  
>  `start_async`::
> diff --git a/editor.c b/editor.c
> index 065a7ab..27bdecd 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -51,7 +51,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
>  		sigchain_push(SIGINT, SIG_IGN);
>  		sigchain_push(SIGQUIT, SIG_IGN);
>  		ret = finish_command(&p);
> -		sig = ret + 128;
> +		sig = ret - 128;
>  		sigchain_pop(SIGINT);
>  		sigchain_pop(SIGQUIT);
>  		if (sig == SIGINT || sig == SIGQUIT)
> diff --git a/run-command.c b/run-command.c
> index 757f263..cfb7274 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -249,7 +249,7 @@ static int wait_or_whine(pid_t pid, const char *argv0)
>  		 * mimics the exit code that a POSIX shell would report for
>  		 * a program that died from this signal.
>  		 */
> -		code -= 128;
> +		code += 128;
>  	} else if (WIFEXITED(status)) {
>  		code = WEXITSTATUS(status);
>  		/*

^ permalink raw reply

* Re: [PATCH] Alphabetize the fast-import options, following a suggestion on the list.
From: Junio C Hamano @ 2013-01-06  7:12 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Eric S. Raymond, git, David Michael Barr, Pete Wyckoff
In-Reply-To: <20130105231151.GD3247@elie.Belkin>

Jonathan Nieder <jrnieder@gmail.com> writes:

> My knee-jerk response was "If the options are currently organized logically,
> wouldn't it be more appropriate to add a sub-heading for each group of options
> and alphabetize only within the subgroups?"
>
> But in fact the current options list doesn't seem to be well organized at all.
> What do you think would be a logical way to group these?
>
>  Features of input syntax
>
> 	--date-format
> 	--done
>
>  Verbosity
>
> 	--quiet
> 	--stats
>
>  Marks handling (checkpoint/restore)
>
> 	--import-marks
> 	--import-marks-if-exists
> 	--export-marks
> 	--relative-marks
>
>  Semantics of execution
>
> 	--dry-run
> 	--force
> 	--cat-blob-fd
> 	--export-pack-edges
>
>  Tuning
>
> 	--active-branches
> 	--max-pack-size
> 	--big-file-threshold
> 	--depth

Sounds sensible.

^ permalink raw reply

* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Torsten Bögershausen @ 2013-01-06  7:23 UTC (permalink / raw)
  To: Jason Pyeron; +Cc: git
In-Reply-To: <BB541ECCD3F04E479F06CA491DDB598D@black>

On 06.01.13 07:29, Jason Pyeron wrote:
>> -----Original Message-----
>> From: Stephen & Linda Smith 
>> Sent: Sunday, January 06, 2013 1:21
>>
>>> Was it the commit before
>>> 9fca6cffc05321445b59c91e8f8d308f41588b53 that compiles or was it 
>>> 9fca6cffc05321445b59c91e8f8d308f41588b53 that compiled? I 
>> am doing a 
>>> cygwin update presently to look at it.
>>
>> Since the email earlier today, I had blown away the 
>> directory.   I just now 
>> did the following
>>
>> git clone https://github.com/git/git.git git-src && cd 
>> git-src && make all
>> ...   The make errored out as before
>>
> 
> No error for me.
> 
>> git co 9fca6c && make all
>> ...   The make errored out as before
> 
> No error for me.
> 
>>
>> git co 9fca6c^  && make all
>> ... and this compiles to completion
>>
>> CYGWIN_NT-5.1 WINXPMACHINE 1.7.14(0.260/5/3) 2012-04-24 17:22 
>> i686 Cygwin
> 
> This is old, do you have the luxury of updating it?
> 
>>
>> What else can I do to test this out (I will get a current 
>> cygwin tomorrow to use in a test).
> 
> I would also check to see if your devel packages are up to date too.

You can either upgrade to cygwin 1.17 or higher.
Or, if that is really not possible (because you are sitting on a production machine,
where no changes are allowed),

You can enable this in Makefile: 
CYGWIN_V15_WIN32API = YesPlease

HTH
/Torsten

^ permalink raw reply

* Re: [PATCH] SubmittingPatches: Document how to request a patch review tag
From: Michael Haggerty @ 2013-01-06  7:32 UTC (permalink / raw)
  To: Jason Holden; +Cc: gitster, git
In-Reply-To: <1357333116-6971-1-git-send-email-jason.k.holden.swdev@gmail.com>

On 01/04/2013 09:58 PM, Jason Holden wrote:
> Document the preferred way a developer should request to have their
> Acked-by/Tested-by/Reviewed-by tag to a patch series under discussion
> 
> Signed-off-by: Jason Holden <jason.k.holden.swdev@gmail.com>
> ---
> Junio,
>   I was ready to add my Reviewed-by to this patch series, but I wasn't sure if
> I should email just you the patch author (to cut down on overall list traffic)
> or both you and the list.  If all reviewed-by/acked-by/tested-by traffic 
> should go via the email list I think this patch would be helpful, as I 
> wasn't quite sure how wide of a distribution list to use for my 
> "Reviewed-by" email.
> 
> A very similiar question was asked previously in:
> http://thread.gmane.org/gmane.comp.version-control.git/185564/focus=185570

On 01/04/2013 10:47 PM, Junio C Hamano wrote:
> "Reviewed-by" is for those who are familiar with the part of the
> system being touched to say "I reviewed this patch, it looks good",
> and Michael indeed was involved in recent updates to the refs.c
> infrastructure, so as he said in his message "it looks like I should",
> it was the right thing to do.
>
> I do not think Michael was asking if that was the standard _thing_
> to do; I think the question was if there was a standard _way_
> (perhaps a tool) to send such a "Reviewed-by:" line.

Junio is correct; I was just asking whether there was a particular email
convention for adding a "Reviewed-by:" line.  It would be nice for this
to be mentioned in the documentation.

>  Documentation/SubmittingPatches | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index f6276ff..80001c9 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -268,6 +268,11 @@ If you like, you can put extra tags at the end:
>  4. "Tested-by:" is used to indicate that the person applied the patch
>     and found it to have the desired effect.
>  
> +If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag
> +to a patch series under discussion (after having reviewed it or tested it
> +of course!), reply to the author of the patch series, cc'ing the git mailing
> +list.
> +
>  You can also create your own tag or use one that's in common usage
>  such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".

I don't think this is quite correct.  Such emails should be
"reply-to-all" people who have participated in the thread, which might
include more than just the patch author and the git mailing list.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCH] clone: support atomic operation with --separate-git-dir
From: Duy Nguyen @ 2013-01-06  8:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jens Lehmann, Heiko Voigt, git, Manlio Perillo, W. Trevor King
In-Reply-To: <7vfw2eq0a0.fsf@alter.siamese.dyndns.org>

On Sun, Jan 6, 2013 at 1:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> How is the file that points at the real git dir removed with this
> fix, by the way?

It's part of the worktree cleanup, pointed by junk_work_tree. And
because junk_work_tree is not set up for --bare, I guess we still need
to fix "--bare --separate-git-dir" case, or forbid it because i'm not
sure if that case makes sense at all.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] SubmittingPatches: Document how to request a patch review tag
From: Junio C Hamano @ 2013-01-06  9:01 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Jason Holden, git
In-Reply-To: <50E92875.6080305@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 01/04/2013 10:47 PM, Junio C Hamano wrote:
>> "Reviewed-by" is for those who are familiar with the part of the
>> system being touched to say "I reviewed this patch, it looks good",
>> and Michael indeed was involved in recent updates to the refs.c
>> infrastructure, so as he said in his message "it looks like I should",
>> it was the right thing to do.
>>
>> I do not think Michael was asking if that was the standard _thing_
>> to do; I think the question was if there was a standard _way_
>> (perhaps a tool) to send such a "Reviewed-by:" line.
>
> Junio is correct; I was just asking whether there was a particular email
> convention for adding a "Reviewed-by:" line.  It would be nice for this
> to be mentioned in the documentation.

Yeah, I wasn't exactly correct in that I was talking more about
Acked-by than Reviewed-by, but they are morally very similar and the
same argument applies to both.

In the particular case of your "refs.c" review, because you are not
just familiar with the code, but essentially are the current owner
of the code, Acked-by might have been even more appropriate than
Reviewed-by, by the way.

>> +If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag
>> +to a patch series under discussion (after having reviewed it or tested it
>> +of course!), reply to the author of the patch series, cc'ing the git mailing
>> +list.
>> +
>>  You can also create your own tag or use one that's in common usage
>>  such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
>
> I don't think this is quite correct.  Such emails should be
> "reply-to-all" people who have participated in the thread, which might
> include more than just the patch author and the git mailing list.

That would be more helpful.  In practice, I can pick these up either
way, but Cc'ing everybody would be better as a common courtesy.

When the author resubmits an already reviewed patch with these Acks
and Reviews for final application, these reviewers should be Cc'ed
so that they can say "Huh?  that is not the exact patch I reviewed.
What is going on?".

Thanks for a review.

^ permalink raw reply

* Re: [PATCH] clone: support atomic operation with --separate-git-dir
From: Jonathan Nieder @ 2013-01-06  9:16 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Junio C Hamano, Jens Lehmann, Heiko Voigt, git, Manlio Perillo,
	W. Trevor King
In-Reply-To: <CACsJy8Dbe1+e4-XjB+S=kvXyi_Hdt4CQ6K1Z1V-4sqaYekKPWw@mail.gmail.com>

Duy Nguyen wrote:

>                                                               And
> because junk_work_tree is not set up for --bare, I guess we still need
> to fix "--bare --separate-git-dir" case, or forbid it because i'm not
> sure if that case makes sense at all.

Forbidding it makes sense to me.  If some day we want a git command to
create a .git file, I don't think it should be spelled as "git clone
--bare --separate-git-dir <repo>".  In the meantime,

	printf '%s\n' 'gitdir: /path/to/repo' >repo.git

works fine.

^ permalink raw reply

* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Jonathan Nieder @ 2013-01-06  9:32 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Stephen & Linda Smith, Jason Pyeron, git, Mark Levedahl
In-Reply-To: <50E92675.4010907@web.de>

Hi,

Torsten Bögershausen wrote:
>> Stephen & Linda Smith wrote:

>>> git co 9fca6c && make all
>>> ...   The make errored out as before
[...]
>>> git co 9fca6c^  && make all
>>> ... and this compiles to completion
>>>
>>> CYGWIN_NT-5.1 WINXPMACHINE 1.7.14(0.260/5/3) 2012-04-24 17:22 
>>> i686 Cygwin
[...]
> You can either upgrade to cygwin 1.17 or higher.
> Or, if that is really not possible (because you are sitting on a production machine,
> where no changes are allowed),
>
> You can enable this in Makefile: 
> CYGWIN_V15_WIN32API = YesPlease

What I don't understand is why commit 9fca6c would have had any
effect at all.  Since 1.7.14 doesn't match /^1\.[1-6]\./, wouldn't
the setting to avoid including <sys/stat.h> and <sys/errno.h> be
unset both before and after that commit?

Stephen, what is the content of your config.mak?

Curious,
Jonathan

^ permalink raw reply

* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Torsten Bögershausen @ 2013-01-06  9:42 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Torsten Bögershausen, Stephen & Linda Smith,
	Jason Pyeron, git, Mark Levedahl
In-Reply-To: <20130106093211.GB10956@elie.Belkin>

On 06.01.13 10:32, Jonathan Nieder wrote:
> Hi,
> 
> Torsten Bögershausen wrote:
>>> Stephen & Linda Smith wrote:
> 
>>>> git co 9fca6c && make all
>>>> ...   The make errored out as before
> [...]
>>>> git co 9fca6c^  && make all
>>>> ... and this compiles to completion
>>>>
>>>> CYGWIN_NT-5.1 WINXPMACHINE 1.7.14(0.260/5/3) 2012-04-24 17:22 
>>>> i686 Cygwin
> [...]
>> You can either upgrade to cygwin 1.17 or higher.
>> Or, if that is really not possible (because you are sitting on a production machine,
>> where no changes are allowed),
>>
>> You can enable this in Makefile: 
>> CYGWIN_V15_WIN32API = YesPlease
> 
> What I don't understand is why commit 9fca6c would have had any
> effect at all.  Since 1.7.14 doesn't match /^1\.[1-6]\./, wouldn't
> the setting to avoid including <sys/stat.h> and <sys/errno.h> be
> unset both before and after that commit?
> 
> Stephen, what is the content of your config.mak?
> 
> Curious,
> Jonathan
The short version:
Cygwin versions  1.7.1 up to 1.7.16 use the same header files as cygwin 1.5

The change in cygwin came in in 1.7.17, 
(and from that version of cygwin we need commit 9fca6c to compile git)

Currently we can not compile git under cygwin 1.7.1 .. 1.7.16 :-(
As "everybody" running cygwin 1.7 seems to update to the latest,

I don't know if we want to improve the Makefile to enable 
CYGWIN_V15_WIN32API = YesPlease 
for cygwin versions 1.7.1 .. 1.7.16 (which are outdated)

/Torsten




 


http://git.661346.n2.nabble.com/PATCH-Rename-V15-MINGW-HEADERS-into-CYGWIN-OLD-WINSOCK-HEADERS-td7571449.html

^ permalink raw reply

* [PATCH] clone: forbid --bare --separate-git-dir <dir>
From: Nguyễn Thái Ngọc Duy @ 2013-01-06  9:47 UTC (permalink / raw)
  To: Jonathan Niedier
  Cc: git, Junio C Hamano, Jens Lehmann, Heiko Voigt, Manlio Perillo,
	W. Trevor King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20130106091642.GA10956@elie.Belkin>

--separate-git-dir was added to clone with the repository away from
standard position <worktree>/.git. It does not make sense to use it
without creating working directory.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On Sun, Jan 6, 2013 at 4:16 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
 > Duy Nguyen wrote:
 >
 >>                                                               And
 >> because junk_work_tree is not set up for --bare, I guess we still need
 >> to fix "--bare --separate-git-dir" case, or forbid it because i'm not
 >> sure if that case makes sense at all.
 >
 > Forbidding it makes sense to me.  If some day we want a git command to
 > create a .git file, I don't think it should be spelled as "git clone
 > --bare --separate-git-dir <repo>".

 That command does not work because --separate-git-dir requires an
 argument in addition to <repo>, which is taken as the target repo.
 Still it's hard to find a use case for

 git clone --bare --separate-git-dir <real-repo> <symlink-repo>
 
 > In the meantime,
 >
 >         printf '%s\n' 'gitdir: /path/to/repo' >repo.git
 >
 > works fine.

 Yeah. A separate clone operation following by that printf should be
 clearer.


 builtin/clone.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/builtin/clone.c b/builtin/clone.c
index ec2f75b..360e430 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -704,6 +704,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		if (option_origin)
 			die(_("--bare and --origin %s options are incompatible."),
 			    option_origin);
+		if (real_git_dir)
+			/*
+			 * If you lift this restriction, remember to
+			 * clean .git in this case in remove_junk_on_signal
+			 */
+			die(_("--bare and --separate-git-dir are incompatible."));
 		option_no_checkout = 1;
 	}
 
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Jonathan Nieder @ 2013-01-06  9:57 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Stephen & Linda Smith, Jason Pyeron, git, Mark Levedahl,
	Eric Blake
In-Reply-To: <50E946EB.1000709@web.de>

Torsten Bögershausen wrote:

> The short version:
> Cygwin versions  1.7.1 up to 1.7.16 use the same header files as cygwin 1.5
[...]
> I don't know if we want to improve the Makefile to enable 
> CYGWIN_V15_WIN32API = YesPlease 
> for cygwin versions 1.7.1 .. 1.7.16 (which are outdated)

Confusing.  Sounds like the condition in 380a4d92 (Update cygwin.c for
new mingw-64 win32 api headers, 2012-11-11) was too strict and the
Makefile should say something like

	# Cygwin versions up to 1.7.16 used the same headers
	# as Cygwin 1.5.
	ifeq ($(shell expr "$(uname_R)" : '1\.7\.[0-9]$$'),5)
		CYGWIN_V15_WIN32API = YesPlease
	endif
	ifeq ($(shell expr "$(uname_R)" : '1\.7\.1[0-6]$$'),6)
		CYGWIN_V15_WIN32API = YesPlease
	endif

	ifeq ($(shell expr "$(uname_R)" : '1\.[1-6]\.'),4)
		CYGWIN_V15_WIN32API = YesPlease
		...
	endif

Is that right?

^ permalink raw reply

* Re: [PATCH] clone: forbid --bare --separate-git-dir <dir>
From: Jonathan Nieder @ 2013-01-06 10:19 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy
  Cc: git, Junio C Hamano, Jens Lehmann, Heiko Voigt, Manlio Perillo,
	W. Trevor King
In-Reply-To: <1357465670-32766-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

> --separate-git-dir was added to clone with the repository away from
> standard position <worktree>/.git. It does not make sense to use it
> without creating working directory.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

The patch correctly implements the above.  The description leaves out
detail.  I'd say something like

	The --separate-git-dir option was introduced to make it simple
	to put the git directory somewhere outside the worktree, for
	example when cloning a repository for use as a submodule.

	It was not intended for use when creating a bare repository.
	In that case there is no worktree and it is more natural to
	directly clone the repository and create a .git file as
	separate steps:

		git clone --bare /path/to/repo.git bar.git
		printf 'gitdir: bar.git\n' >foo.git

	Unfortunately we forgot to forbid the --bare
	--separate-git-dir combination.  In practice, we know no one
	could be using --bare with --separate-git-dir because it is
	broken in the following way: <explanation here>.  So it is
	safe to make good on our mistake and forbid the combination,
	making the command easier to explain.

I don't know what would go in the <explanation here> blank above,
though.  Is it possible that some people are relying on this option
combination?

^ permalink raw reply

* Re: cvsps, parsecvs, svn2git and the CVS exporter mess
From: Michael Haggerty @ 2013-01-06 11:15 UTC (permalink / raw)
  To: esr
  Cc: Max Horn, Yann Dirson, Heiko Voigt, Antoine Pelisse, Bart Massey,
	Keith Packard, David Mansfield, git
In-Reply-To: <20130105151106.GA1938@thyrsus.com>

On 01/05/2013 04:11 PM, Eric S. Raymond wrote:
> Perhaps I was unclear.  I consider the interface design error to
> be not in the fact that all the blobs are written first or detached,
> but rather that the implementation detail of the two separate journal
> files is ever exposed.
> 
> I understand why the storage of intermediate results was done this
> way, in order to decrease the tool's working set during the run, but
> finishing by automatically concatenating the results and streaming
> them to stdout would surely have been the right thing here.

cvs2svn/cvs2git is built to be able to handle very large CVS
repositories, not only those that can fit in RAM.  This goal influences
a lot of its design, including the pass-by-pass structure with
intermediate databases and the resumability of passes.

The blobfile necessarily contains every version of every file, with no
delta-encoding and no compression.  Its size can be a large multiple of
the on-disk size of the original CVS repository.  If the "save to
tempfiles then cat tempfiles at end of run" behavior were hard-coded
into cvs2git, then there would be no way to avoid requiring enough
temporary space to hold the whole blobfile.

Writing the blobfile into a separate file, on the other hand, means that
for example the blobfile could be written into a named pipe connected to
the standard input of "git fast-import" [1].  "git fast-import" could
even be run on a remote server.

I consider these bigger advantages than the ability to pipe the output
of cvs2git directly into another command.

> The downstream cost of letting the journalling implementation be
> exposed, instead, can be seen in this snippet from the new git-cvsimport
> I've been working on:
> 
>     def command(self):
>         "Emit the command implied by all previous options."
>         return "(cvs2git --username=git-cvsimport --quiet --quiet --blobfile={0} --dumpfile={1} {2} {3} && cat {0} {1} && rm {0} {1})".format(tempfile.mkstemp()[1], tempfile.mkstemp()[1], self.opts, self.modulepath)
> 
> According to the documentation, every caller of csv2git must go
> through analogous contortions!  This is not the Unix way; if Unix
> design principles had been minimally applied, that second line would
> just read like this:
> 
>      return "cvs2git --username=git-cvsimport --quiet --quiet"

Never in my worst nightmares did I imagine that my terrible design taste
would force you to type an extra two lines of code.  Oh the humanity!

By the way, patches are welcome.  And you don't need to trumpet their
imminent arrival [2] or malign the existing code beforehand.  Moreover,
it would be adequate if you just demonstrate working code and *then* ask
for "sign-in", rather than the other way around.

> If Unix design principles had been thoroughly applied, the "--quiet
> --quiet" part would be unnecessary too - well-behaved Unix commands
> *default* to being completely quiet unless either (a) they have an
> exceptional condition to report, or (b) their expected running time is
> so long that tasteful silence would leave users in doubt that they're
> working.

cvs2git is not a command that one uses 100 times a day.  It is a tool
for one-shot conversions of CVS repositories to git.  These conversions
can take hours or even days of processing time (not to mention the time
for configuring the conversion and changing the rest of a project's
infrastructure from CVS to git).  So yes, I think we would like to
appeal to (b) and humbly ask for your permission to give the user some
feedback during the conversion.

> (And yes, I do think violating these principles is a lapse of taste when
> git tools do it, too.)
> 
> Michael Haggerty wants me to trust that cvs2git's analysis stage has
> been fixed, but I must say that is a more difficult leap of faith when
> two of the most visible things about it are still (a) a conspicuous
> instance of interface misdesign, and (b) documentation that is careless and
> incomplete.

The cvs2git documentation is lacking; I admit it (as opposed to the
cvs2svn documentation, which I think is quite complete).  And the
program itself also has a lot of rough edges, for example its inability
to convert .cvsignore files into .gitignore files.  Patches are welcome.
 I haven't used cvs2svn for my own purposes in many years and I've
*never* once had a need to use cvs2git; I maintain these programs purely
as a service to the community.  Most of the community seems satisfied
with the programs as they are, and if not they usually submit courteous
and concrete bug reports or submit patches.

I request that you follow their example.  I especially ask that you
restrain from spreading public FUD about imagined problems based on
speculation.  Please do your tests and *then* report any problems that
you find.

Yours,
Michael

[1] In fact, the current implementation of generate_blobs.py sometimes
seeks back to earlier parts of the blob file when it needs the fulltext
of a revision that has already been output, but this would be easy to
change as soon as somebody needs it.

[2] http://comments.gmane.org/gmane.comp.version-control.git/212340

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: Version 1.8.1 does not compile on Cygwin 1.7.14
From: Mark Levedahl @ 2013-01-06 11:48 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Torsten Bögershausen, Stephen & Linda Smith,
	Jason Pyeron, git, Eric Blake
In-Reply-To: <20130106095757.GC10956@elie.Belkin>

On 01/06/2013 04:57 AM, Jonathan Nieder wrote:
> Torsten Bögershausen wrote:
>
>> The short version:
>> Cygwin versions  1.7.1 up to 1.7.16 use the same header files as cygwin 1.5
> [...]
>> I don't know if we want to improve the Makefile to enable
>> CYGWIN_V15_WIN32API = YesPlease
>> for cygwin versions 1.7.1 .. 1.7.16 (which are outdated)
>>

You are conflating the cygwin dll version with the win32 api version. 
These are independent packages (just as the kernel and glibc packages 
are independent on linux) and do not share a version number. However, 
the newer win32api is provided only for the current cygwin release 
series, which can be reliably identified by having dll version 1.7.x, 
while the older frozen releases (dll versions 1.6.x from redhat, 1.5.x 
open source) still have the older api as no updates are being made for 
the legacy version(s).

Cygwin does not version the win32api in any useful way: the package 
names changed completely, for instance, and there is no macro defined 
from the header files to indicate a version number. Also, there is no 
supported way to now install the older version: the only supported 
configuration is with the *current* win32api: multiple packages depend 
by name on the current win32api package, so the installer will insist 
upon its installation.

So the solution is to update the cygwin installation. Really. If you 
don't believe me, try asking on the cygwin mailing list. They only 
support the current releases, not obsolete packages, and the older 
win32api is explicitly obsolete.

Mark

^ permalink raw reply

* [PATCH] docs: manpage XML depends on asciidoc.conf
From: Jonathan Nieder @ 2013-01-06 12:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John Keeping, git, Sergey Vlasov, Thomas Ackermann
In-Reply-To: <7vbod2pzxd.fsf@alter.siamese.dyndns.org>

When building manual pages, the source text is transformed to XML with
AsciiDoc before the man pages are generated from the XML with xmlto.

Fix the dependencies in the Makefile so that the XML files are rebuilt
when asciidoc.conf changes and not just the manual pages from
unchanged XML, and move the dependencies from a recipeless rule to the
rules with commands that use asciidoc.conf to make the dependencies
easier to understand and maintain.

Reported-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> Care to do a real patch?

Here you go.

 Documentation/Makefile | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e53d333e..971977b8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -178,8 +178,6 @@ all: html man
 
 html: $(DOC_HTML)
 
-$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7): asciidoc.conf
-
 man: man1 man5 man7
 man1: $(DOC_MAN1)
 man5: $(DOC_MAN5)
@@ -257,7 +255,7 @@ clean:
 	$(RM) $(cmds_txt) *.made
 	$(RM) manpage-base-url.xsl
 
-$(MAN_HTML): %.html : %.txt
+$(MAN_HTML): %.html : %.txt asciidoc.conf
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(ASCIIDOC) -b xhtml11 -d manpage -f asciidoc.conf \
 		$(ASCIIDOC_EXTRA) -agit_version=$(GIT_VERSION) -o $@+ $< && \
@@ -270,7 +268,7 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_XMLTO)$(RM) $@ && \
 	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
 
-%.xml : %.txt
+%.xml : %.txt asciidoc.conf
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(ASCIIDOC) -b docbook -d manpage -f asciidoc.conf \
 		$(ASCIIDOC_EXTRA) -agit_version=$(GIT_VERSION) -o $@+ $< && \
@@ -286,7 +284,7 @@ technical/api-index.txt: technical/api-index-skel.txt \
 	$(QUIET_GEN)cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh
 
 technical/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
-$(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt
+$(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt asciidoc.conf
 	$(QUIET_ASCIIDOC)$(ASCIIDOC) -b xhtml11 -f asciidoc.conf \
 		$(ASCIIDOC_EXTRA) -agit_version=$(GIT_VERSION) $*.txt
 
-- 
1.8.1

^ permalink raw reply related

* Re: [PATCH v3 02/19] Improve documentation and comments regarding directory traversal API
From: Adam Spiers @ 2013-01-06 12:02 UTC (permalink / raw)
  To: git list
In-Reply-To: <CAOkDyE_DX8iAAd5ubJaQ_guPQ-PSz4-sFETZoRf7JRTrH6Qcpw@mail.gmail.com>

On Wed, Jan 02, 2013 at 12:54:19PM +0000, Adam Spiers wrote:
> On Tue, Jan 1, 2013 at 8:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Adam Spiers <git@adamspiers.org> writes:
> >> diff --git a/dir.c b/dir.c
> >> index ee8e711..89e27a6 100644
> >> --- a/dir.c
> >> +++ b/dir.c
> >> @@ -2,6 +2,8 @@
> >>   * This handles recursive filename detection with exclude
> >>   * files, index knowledge etc..
> >>   *
> >> + * See Documentation/technical/api-directory-listing.txt
> >> + *
> >>   * Copyright (C) Linus Torvalds, 2005-2006
> >>   *            Junio Hamano, 2005-2006
> >>   */
> >> @@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)
> >>               die("cannot use %s as an exclude file", fname);
> >>  }
> >>
> >> +/*
> >> + * Loads the per-directory exclude list for the substring of base
> >> + * which has a char length of baselen.
> >> + */
> >>  static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> >>  {
> >>       struct exclude_list *el;
> >> @@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> >>           (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
> >>               return; /* too long a path -- ignore */
> >>
> >> -     /* Pop the ones that are not the prefix of the path being checked. */
> >> +     /* Pop the directories that are not the prefix of the path being checked. */
> >
> > The "one" does not refer to a "directory", but to an "exclude-list".
> 
> No, if that was the case, it would mean that multiple exclude lists
> would be popped, but that is not the case here (prior to v4).

Sorry, I meant prior to v3 11/19.

> >         Pop the ones that are not for parent directories of the path
> >         being checked
> 
> Better would be:
> 
>     Pop the entries within the EXCL_DIRS exclude list which originate
>     from directories not in the prefix of the path being checked.
> 
> although as previously stated, the v4 series I have been holding off
> from submitting (in order not to distract you from a maint release)
> actually changes this behaviour so EXCL_DIRS becomes an exclude_group of
> multiple exclude_lists, one per directory.  So in v4, multiple
> exclude_lists *will* be popped.  I'll tweak the comment in v4 to make
> this clear.

Again, I got confused and forgot that I already included the switch to
exclude_list_groups as v3 11/19.  But since the patch being discussed
here is v3 02/19 which precedes it, everything I said still applies.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox