[PATCH v2] all: new command used for multi-repo operations

2013-01-23 Thread Lars Hjemli
When working with multiple, unrelated (or loosly related) git repos,
there is often a need to locate all repos with uncommitted work and
perform some action on them (say, commit and push). Before this patch,
such tasks would require manually visiting all repositories, running
`git status` within each one and then decide what to do next.

This mundane task can now be automated by e.g. `git all --dirty status`,
which will find all git repositories below the current directory (even
nested ones), check if they are dirty (as defined by `git diff --quiet &&
git diff --cached --quiet`), and for each dirty repo print the path to the
repo and then execute `git status` within the repo.

The command also honours the option '--clean' which restricts the set of
repos to those which '--dirty' would skip.

Finally, the command to execute within each repo is optional. If none is
given, git-all will just print the path to each repo found.

Signed-off-by: Lars Hjemli 
---

Changes since v1:
* uses setenv() instead of chdir(), which fixes .gitfile handling
* uses DTYPE() and stat(), which fixes NO_D_TYPE_IN_DIRENT platforms
* uses OPT_SET_INT() instead of OPT_BOOLEAN
* support for --all (complements --clean/--dirty)
* removed from command-list.txt
* added to .gitignore

I've not yet renamed the command. If it should be changed to 'git
for-each-repo', I'm tempted to make a patch which transforms
`git -ad status` into `git for-each-repo -d status`.

 .gitignore|   1 +
 Documentation/git-all.txt |  42 ++
 Makefile  |   1 +
 builtin.h |   1 +
 builtin/all.c | 110 ++
 git.c |   1 +
 t/t0064-all.sh|  46 +++
 7 files changed, 202 insertions(+)
 create mode 100644 Documentation/git-all.txt
 create mode 100644 builtin/all.c
 create mode 100755 t/t0064-all.sh

diff --git a/.gitignore b/.gitignore
index aa258a6..27118d7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@
 /git
 /git-add
 /git-add--interactive
+/git-all
 /git-am
 /git-annotate
 /git-apply
diff --git a/Documentation/git-all.txt b/Documentation/git-all.txt
new file mode 100644
index 000..baaa57e
--- /dev/null
+++ b/Documentation/git-all.txt
@@ -0,0 +1,42 @@
+git-all(1)
+==
+
+NAME
+
+git-all - Execute a git command in multiple repositories
+
+SYNOPSIS
+
+[verse]
+'git all' [--all|--clean|--dirty] [command]
+
+DESCRIPTION
+---
+The git-all command is used to locate all git repositoris within the
+current directory tree, and optionally execute a git command in each
+of the found repos.
+
+OPTIONS
+---
+-a::
+--all::
+   Include both clean and dirty repositories (this is the default
+   behaviour of `git-all`).
+
+-c::
+--clean::
+   Only include repositories with a clean worktree.
+
+-d::
+--dirty::
+   Only include repositories with a dirty worktree.
+
+NOTES
+-
+
+For the purpose of `git-all`, a dirty worktree is defined as a worktree
+with uncommitted changes.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 1b30d7b..8bf0583 100644
--- a/Makefile
+++ b/Makefile
@@ -840,6 +840,7 @@ LIB_OBJS += xdiff-interface.o
 LIB_OBJS += zlib.o
 
 BUILTIN_OBJS += builtin/add.o
+BUILTIN_OBJS += builtin/all.o
 BUILTIN_OBJS += builtin/annotate.o
 BUILTIN_OBJS += builtin/apply.o
 BUILTIN_OBJS += builtin/archive.o
diff --git a/builtin.h b/builtin.h
index 7e7bbd6..438c265 100644
--- a/builtin.h
+++ b/builtin.h
@@ -41,6 +41,7 @@ void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg 
*c);
 extern int textconv_object(const char *path, unsigned mode, const unsigned 
char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
+extern int cmd_all(int argc, const char **argv, const char *prefix);
 extern int cmd_annotate(int argc, const char **argv, const char *prefix);
 extern int cmd_apply(int argc, const char **argv, const char *prefix);
 extern int cmd_archive(int argc, const char **argv, const char *prefix);
diff --git a/builtin/all.c b/builtin/all.c
new file mode 100644
index 000..b170b26
--- /dev/null
+++ b/builtin/all.c
@@ -0,0 +1,110 @@
+/*
+ * "git all" builtin command.
+ *
+ * Copyright (c) 2013 Lars Hjemli 
+ */
+#include "cache.h"
+#include "color.h"
+#include "builtin.h"
+#include "run-command.h"
+#include "parse-options.h"
+
+#define ALL 0
+#define DIRTY 1
+#define CLEAN 2
+
+static int match;
+
+static const char * const builtin_all_usage[] = {
+   N_("git all [--all|--clean|--dirty] [cmd]"),
+   NULL
+};
+
+static struct option builtin_all_options[] = {
+   OPT_SET_INT('a', "all", &match, N_("match both clean and dirty 
repositories"), ALL),
+   OPT_SET_INT('c', "clean", &match, N_("only show clean repositories"), 
CLEAN),
+   OPT_SET_INT('d', "dirty", &match, N_("only show dirty repositories"), 
DIRTY),
+   O

Re: [PATCH] all: new command used for multi-repo operations

2013-01-23 Thread Duy Nguyen
On Wed, Jan 23, 2013 at 4:10 AM, Lars Hjemli  wrote:
> When working with multiple, unrelated (or loosly related) git repos,
> there is often a need to locate all repos with uncommitted work and
> perform some action on them (say, commit and push). Before this patch,
> such tasks would require manually visiting all repositories, running
> `git status` within each one and then decide what to do next.
>
> This mundane task can now be automated by e.g. `git all --dirty status`,
> which will find all git repositories below the current directory (even
> nested ones), check if they are dirty (as defined by `git diff --quiet &&
> git diff --cached --quiet`), and for each dirty repo print the path to the
> repo and then execute `git status` within the repo.

I think it should leave out the execute part. The command, say
ls-repo, lists repositories in specified state. The execute part could
be easily done by

xargs -I{} git --git-dir={} status blah

I haven't thought it through. I know xargs does not support chdir'ing
into a repo, so maybe a new git-xargs could be introduced for that.
But there's still a problem with git-xargs (or git-all), printed paths
are relative to the subrepos, not where the git-all/xargs command is
executed. This could be confusing. Personally I'd like to do it
without chdir. That is

xargs -I{} git --git-dir={}/.git --work-tree={} status blah

should print paths relative to current working directory even if cwd
is outside the repo.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] all: new command used for multi-repo operations

2013-01-23 Thread Lars Hjemli
On Wed, Jan 23, 2013 at 9:39 AM, Duy Nguyen  wrote:
> On Wed, Jan 23, 2013 at 4:10 AM, Lars Hjemli  wrote:
>> When working with multiple, unrelated (or loosly related) git repos,
>> there is often a need to locate all repos with uncommitted work and
>> perform some action on them (say, commit and push). Before this patch,
>> such tasks would require manually visiting all repositories, running
>> `git status` within each one and then decide what to do next.
>>
>> This mundane task can now be automated by e.g. `git all --dirty status`,
>> which will find all git repositories below the current directory (even
>> nested ones), check if they are dirty (as defined by `git diff --quiet &&
>> git diff --cached --quiet`), and for each dirty repo print the path to the
>> repo and then execute `git status` within the repo.
>
> I think it should leave out the execute part. The command, say
> ls-repo, lists repositories in specified state. The execute part could
> be easily done by
>
> xargs -I{} git --git-dir={} status blah

Not so easily on windows, which I need to use at $WORK :(

--
larsh
--
To unsubscribe from this list: send the line "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] all: new command used for multi-repo operations

2013-01-23 Thread Lars Hjemli
[*git@vger.kernel.org accidentally dropped from cc *]

On Wed, Jan 23, 2013 at 7:52 AM, Junio C Hamano  wrote:
> Lars Hjemli  writes:
>
>> +static int walk(struct strbuf *path, int argc, const char **argv)
>> +{
>> + DIR *dir;
>> + struct dirent *ent;
>> + size_t len;
>> +
>> + dir = opendir(path->buf);
>> + if (!dir)
>> + return errno;
>> + strbuf_addstr(path, "/");
>> + len = path->len;
>> + while ((ent = readdir(dir))) {
>> + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
>> + continue;
>> + if (!strcmp(ent->d_name, ".git")) {
>
> This only looks for the top of working tree.  Have you considered if
> this "iterate over directories and list git repositories in them"
> may be useful for collection of bare repositories, and if it is, how
> to go about implementing the discovery process?

Yes, occasionally I've needed this, but implementing it in my original
shell script was cumbersome. Doing it in C should be as simple as
invoking is_git_directory() on each subdir.

>
>> + if (ent->d_type != DT_DIR)
>> + continue;
>
> I think this is wrong.
>
> On platforms that need a NO_D_TYPE_IN_DIRENT build, your compilation
> may fail here (you would need to lstat() it yourself).  See how
> dir.c does this without ugly #ifdef's in the code, especially around
> the use of get_dtype() and DTYPE() macro.
>

Thanks for the pointer, will fix.

--
larsh
--
To unsubscribe from this list: send the line "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] all: new command used for multi-repo operations

2013-01-23 Thread Duy Nguyen
On Wed, Jan 23, 2013 at 3:12 PM, Lars Hjemli  wrote:
> +NAME
> +
> +git-all - Execute a git command in multiple repositories

I agree with Junio "git-all" is too generic. Maybe "git-for-each-repo"

> +static int get_repo_state()
> +{
> +   const char *diffidx[] = {"diff", "--quiet", "--cached", NULL};
> +   const char *diffwd[] = {"diff", "--quiet", NULL};
> +
> +   if (run_command_v_opt(diffidx, RUN_GIT_CMD) != 0)
> +   return DIRTY;
> +   if (run_command_v_opt(diffwd, RUN_GIT_CMD) != 0)
> +   return DIRTY;
> +   return CLEAN;
> +}

Perhaps we could add the subrepo's object data to the in-memory object
database of git-all, then do the diff without launching new commands?

> +static int walk(struct strbuf *path, int argc, const char **argv)
> +{
> +   DIR *dir;
> +   struct dirent *ent;
> +   struct stat st;
> +   size_t len;
> +
> +   dir = opendir(path->buf);
> +   if (!dir)
> +   return errno;
> +   strbuf_addstr(path, "/");
> +   len = path->len;
> +   while ((ent = readdir(dir))) {
> +   if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
> +   continue;
> +   if (!strcmp(ent->d_name, ".git")) {
> +   strbuf_addstr(path, ent->d_name);
> +   setenv(GIT_DIR_ENVIRONMENT, path->buf, 1);
> +   strbuf_setlen(path, len - 1);
> +   setenv(GIT_WORK_TREE_ENVIRONMENT, path->buf, 1);
> +   handle_repo(path->buf, argv);
> +   strbuf_addstr(path, "/");
> +   continue;
> +   }
> +   strbuf_setlen(path, len);
> +   strbuf_addstr(path, ent->d_name);
> +   switch (DTYPE(ent)) {
> +   case DT_UNKNOWN:
> +   /* Use stat() instead of lstat(), since we want to
> +* know if we can follow this path into another
> +* directory - it's  not important if it's actually
> +* a symlink which gets us there.
> +*/
> +   if (stat(path->buf, &st) || !S_ISDIR(st.st_mode))
> +   break;
> +   /* fallthrough */
> +   case DT_DIR:
> +   walk(path, argc, argv);
> +   break;
> +   }
> +   strbuf_setlen(path, len);
> +   }
> +   closedir(dir);
> +   return 0;
> +}

I'm not a user of this command so this is more of bikeshedding. I
think we should have an option to list repos listed in index. For
directory walk, how about reusing fill_directory() to do the job for
you? You could then limit repositories by name. "ls-files -o" code
should be very similar.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Aw: Re: [PATCH v3 3/6] Change 'git' to 'Git' whenever the whole system is referred to #2

2013-01-23 Thread Thomas Ackermann
 > 
> Thomas, I do not want to see many rounds of entire rerolls of this
> series on the list (nobody will look at the whole series multiple
> times with fine toothed comb).  I do not think you want to do that
> either.  Can you collect remaining fixups like David's message, turn
> them into patch form when you have collected enough to be reviewed
> in one sitting (say, a patchfile at around 200 lines), and send them
> over to the list to apply on top of the tree of that commit?
> 
Sure!


---
Thomas
--
To unsubscribe from this list: send the line "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] all: new command used for multi-repo operations

2013-01-23 Thread Lars Hjemli
On Wed, Jan 23, 2013 at 9:55 AM, Duy Nguyen  wrote:
> Perhaps we could add the subrepo's object data to the in-memory object
> database of git-all, then do the diff without launching new commands?

The `git all` command is regularly invoked outside of git repos, so
I'm not sure if this would work.

>
> I'm not a user of this command so this is more of bikeshedding. I
> think we should have an option to list repos listed in index.

git-submodule uses something like `git ls-files --stage|grep
"^16"`. Having a better way to achieve this would be nice, but I
don't think it is a job for 'git [all|for-each-repo|ls-repo].

> For
> directory walk, how about reusing fill_directory() to do the job for
> you? You could then limit repositories by name. "ls-files -o" code
> should be very similar.

A cursory look into dir.c seems to indicate that this could work
(possibly except for get_index_dtype()), but it would also load the
complete directory tree (which could be extremly big) into ram,
including file entries (which is not necessary) while dropping '.git'
entries (which is what we're looking for).

-- 
larsh
--
To unsubscribe from this list: send the line "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: What's cooking in git.git (Jan 2013, #08; Tue, 22)

2013-01-23 Thread John Keeping
On Tue, Jan 22, 2013 at 04:11:59PM -0800, Junio C Hamano wrote:
> John Keeping  writes:
>> Would you mind holding off on this?  As it stands there are a couple of
>> issues with the cvsimport-3 script including: ...
> 
> Actually I do. I think this, at least the early part of it, should
> be merged to 'next' as soon as possible, *unless*
> 
>  (1) The cvsimport-2 & cvsps2 combo this series ships gives worse
>  experience than cvsimport we ship in v1.8.1 to end users of the
>  current cvsimport with cvsps2; and/or
> 
>  (2) The cvsimport-3 in this series, which is a copy of an older
>  version of what Eric has, is so broken that we are better off
>  starting cvsimport-3 by getting a fresh copy from Eric which
>  has been rewritten in a major way, than applying huge
>  incremental update patches that amounts to a total rewrite.
> 
> The point (1) is important from "no regression" point of view, and
> in a sense more important between the two because it is the first
> step in the overall transition plan.
> 
> Even though there may be remaining issues in cvsimport-3 and cvsps3
> (what new piece of software don't have issues?), my limited
> observation of the exchanges between you and Eric suggests me that
> the problem is not something that requires a total rewrite of how
> cvsimport-3 works, so I do not expect the point (2) to be true,
> either, but if I am mistaken, please let me know.

ESR's cvsimport.py in the cvsps repository has no fixes over what's
here.  I think his comment in [1] indicates that he won't do any more
work on git-cvsimport.

[1] http://article.gmane.org/gmane.comp.version-control.git/214057

In my opinion the incremental import support really is substantially
worse in cvsimport-3 than cvsimport-2.  cvsimport-2 looks at the output
of git-for-each-ref to calculate the dates from which to continue each
branch.  cvsps cannot be told this information and so the cvsimport-3
script just takes the date of the last commit on the current branch.

On top of that, the incremental switch to cvsps-3 just causes it to
output:

from: refs/heads/branch^0

on the first commit for each branch, which I can't see working if a new
branch is created in CVS.

> By advancing the topic to 'next', we will give people a more solid
> (read: not getting rewound) foundation to work with than "if you are
> really interested, grab the tip of 'pu', replace it with even newer
> copy from Eric's repository and try it out", so that more people can
> help us polish the scaffolding to let us ship two versions and also
> find issues in the new cvsimport-3 and help fixing them.  At least,
> that is what I've been hoping.

That's what I've done and it's convinced me that cvsps-3 is not ready
for use with incremental imports as it stands.

> I could stop at the first three patches, that is, introducing the
> version switch wrapper that switches between cvsps2+cvsimport-2
> combo and nothing, and then let you and Eric redo the "start adding
> cvsps 3.x support" and later patches when cvsimport-3 is ready.
> That would give you a larger lattitude to rework cvsimport-3.  Is
> that preferrable?

My preference would be for something like this, possibly with an
expanded examples section showing how to pipe the output of cvsps-3 or
cvs2git into git-fast-import:

-- >8 --

diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
index 9d5353e..20b846e 100644
--- a/Documentation/git-cvsimport.txt
+++ b/Documentation/git-cvsimport.txt
@@ -18,6 +18,11 @@ SYNOPSIS
 
 DESCRIPTION
 ---
+*WARNING:* `git cvsimport` uses cvsps version 2, which is considered
+deprecated; it does not work with cvsps version 3 and later.  If you are
+performing a one-shot import of a CVS repository consider using cvsps-3,
+cvs2git or parsecvs directly.
+
 Imports a CVS repository into git. It will either create a new
 repository, or incrementally import into an existing one.
 
-- 8< --


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


Re: [PATCH 0/3] fixup remaining cvsimport tests

2013-01-23 Thread Michael Haggerty
On 01/20/2013 09:17 PM, Chris Rorvick wrote:
> I probably won't be sending any more patches on this.  My hope was to
> get cvsimport-3 (w/ cvsps as the engine) in a state such that one
> could transition from the previous version seamlessly.  But the break
> in t9605 has convinced me this is not worth the effort--even in this
> trivial case cvsps is broken.  The fuzzing logic aggregates commits
> into patch sets that have timestamps within a specified window and
> otherwise matching attributes.  This aggregation causes file-level
> commit timestamps to be lost and we are left with a single timestamp
> for the patch set: the minimum for all contained CVS commits.  When
> all commits have been processed, the patch sets are ordered
> chronologically and printed.
> 
> The problem is that is that a CVS commit is rolled into a patch set
> regardless of whether the patch set's timestamp falls within the
> adjacent CVS file-level commits.  Even worse, since the patch set
> timestamp changes as subsequent commits are added (i.e., it's always
> picking the earliest) it is potentially indeterminate at the time a
> commit is added.  The result is that file revisions can be reordered
> in resulting Git import (see t9605.)  I spent some time last week
> trying to solve this but I coudln't think of anything that wasn't a
> substantial re-work of the code.
> 
> I have never used cvs2git, but I suspect Eric's efforts in making it a
> potential backend for cvsimport are a better use of time.

Thanks for your explanation of how cvsps works.

This is roughly how cvs2svn used to work years ago, prior to release
2.x.  In addition it did a number of things to try to tweak the
timestamp ordering to avoid committing file-level commits in the wrong
order.  It never worked 100%; each tweak that was made to fix one
problem created another problem in another scenario.

cvs2svn/cvs2git 2.x takes a very different approach.  It uses a
timestamp threshold along with author and commit-message matching to
find the biggest set of file-level commits that might constitute a
repository-level commit.  But then it checks the proto-commits to see if
they violate the ordering constraints imposed by the individual
file-level commits.  For example, if the initial grouping gives the
following proto-commits:

proto-commit 1: a.txt 1.1b.txt 1.2

proto-commit 2: a.txt 1.2b.txt 1.1

then it is apparent that something is wrong, because a.txt 1.1
necessarily comes before a.txt 1.2 whereas b.txt 1.1 necessarily comes
before b.txt 1.2 (CVS can at least be relied on to get this right!) and
therefore there is no consistent ordering of the two proto-commits.
More generally, the proto-commits have to form a directed acyclic graph,
whereas this graph has a cycle 1 -> 2 -> 1.  When cvs2svn/cvs2git finds
a cycle, it uses heuristics to break up one or more of the proto-commits
to break the cycle.  In this case it might break proto-commit 1 into two
commits:

proto-commit 1a: a.txt 1.1

proto-commit 2:  a.txt 1.2b.txt 1.1

proto-commit 1b:  b.txt 1.2

Now it is possible to commit them in the order 1a,2,1b.  (Exactly this
scenario is tested in t9603.)

Of course a typical proto-commit graph often contains far more
complicated cycles, but the approach remains the same: split
proto-commits up as necessary until the graph is acyclic.  One can
quibble about the heuristics that cvs2svn/cvs2git uses to break up
proto-commits.  But the final result of the algorithm is *guaranteed* to
be consistent with the file-level CVS history and also self-consistent.

I am skeptical that a simpler approach will ever work 100%.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] all: new command used for multi-repo operations

2013-01-23 Thread Duy Nguyen
On Wed, Jan 23, 2013 at 4:24 PM, Lars Hjemli  wrote:
> On Wed, Jan 23, 2013 at 9:55 AM, Duy Nguyen  wrote:
>> Perhaps we could add the subrepo's object data to the in-memory object
>> database of git-all, then do the diff without launching new commands?
>
> The `git all` command is regularly invoked outside of git repos, so
> I'm not sure if this would work.

We could initialize an empty object database in memory. But my
suggestion is probably off topic. I thought this was about a a git
repository with a collection of submodules. If there are no
connections between repos, perhaps git-all fits better as a subcommand
of git-repo:

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


git-svn problems with white-space in tag names

2013-01-23 Thread Hans-Juergen Euler
I have discussed already the problem a bit more in this thread
groups.google.com/d/topic/git-users/kfMFZ3uEFsM/discussion

-Operating system (specifically which version)
windows 7 64 bit

--Git version (git --version)
Git version 1.8.0 for windows obviously.
git bash and git gui installed and using

--Git configuration (system, home, repository)
hmm guess is covered with git bash and git gui. Using the standard config stuff

using subversion
TortoiseSVN 1.7.11
Subversion 1.7.8
Was typically always up-to-date (within 2 months or so) with previous versions

using an external subversion provider for storing the information
externally. guess the version there is older but do not know


I have tried to convert some of my external subversion data bases with
git-svn clone

I have encountered a problem with one of my subversion repos. I have
obviously introduced about 2 years ago a problem with an additional
white space at the end of tag name.

So there is an entry "tags/blabla "

in the subversion repos. The sequential handling of the svn repos with
git-svn gets stuck there. I could not find a way around this. My guess
is that the white-space was introduced by accident on windows by
Tortoise-SVN.
Unfortunately this occurs at revision 90 something and I have almost
1000 revisions stored.

Let me know if you need more details.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


git-svn does not like format=4

2013-01-23 Thread Hans-Juergen Euler
it's part of a sequence of problems you can find on
groups.google.com/d/topic/git-users/kfMFZ3uEFsM/discussion

windows 7 64 bit
Git version 1.8.0
git bash and git gui installed and using

using subversion
TortoiseSVN 1.7.11
Subversion 1.7.8
Was typically always up-to-date (within 2 months or so) with previous versions

using an external subversion provider for storing the information
externally. guess the version there is older but do not know

I have dumped the content of an external subversion repos and created
a local repos with aforementioned version.
when cloning the subversion repos with "git-svn clone" I received this
error message:
 Expected FS format '2'; found format '4' at
/usr/lib/perl5/site_perl/Git/SVN.pm line 148

Please let me know if you need more details.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: git-svn does not like format=4

2013-01-23 Thread Erik Faye-Lund
On Wed, Jan 23, 2013 at 11:43 AM, Hans-Juergen Euler
 wrote:
> it's part of a sequence of problems you can find on
> groups.google.com/d/topic/git-users/kfMFZ3uEFsM/discussion
>
> windows 7 64 bit
> Git version 1.8.0
> git bash and git gui installed and using
>
> using subversion
> TortoiseSVN 1.7.11
> Subversion 1.7.8
> Was typically always up-to-date (within 2 months or so) with previous versions
>
> using an external subversion provider for storing the information
> externally. guess the version there is older but do not know
>
> I have dumped the content of an external subversion repos and created
> a local repos with aforementioned version.
> when cloning the subversion repos with "git-svn clone" I received this
> error message:
>  Expected FS format '2'; found format '4' at
> /usr/lib/perl5/site_perl/Git/SVN.pm line 148

This isn't a problem with Git itself, but with Git for Windows not
having an up-to-date build of libsvn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] fixup remaining cvsimport tests

2013-01-23 Thread John Keeping
On Wed, Jan 23, 2013 at 10:54:36AM +0100, Michael Haggerty wrote:
> On 01/20/2013 09:17 PM, Chris Rorvick wrote:
>> I have never used cvs2git, but I suspect Eric's efforts in making it a
>> potential backend for cvsimport are a better use of time.

Is it possible to perform an incremental import with cvs2git?  This
seems to be the one use case where the old cvsimport script (with cvsps
2.x) still performs the best.

I suppose that just re-running the full import will do the right thing
since the commits in Git should be identical, but would it be possible
to do better given the right information about a previous run?


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


Re: Bug in latest gitk - can't click lines connecting commits

2013-01-23 Thread Paul Mackerras
On Tue, Jan 22, 2013 at 09:28:23AM -0800, Junio C Hamano wrote:
> 
> I notice that I have a handful of commits that I haven't pulled from
> your repository, and the last commit on your 'master' is about 20
> days old.  Is it safe for me to pull these now?

Yes, please pull them now.

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


Moving commits from one branch to another

2013-01-23 Thread Stefan Schulze
Hi,

my team uses a central git-repo since >1500 commits and now we have to sync
(only one-way is necessary for now) our repository every three weeks with an
external svn-repo.
I created the new base-directory (incl. trunk/tags/branches) in svn and
added it to my local repo using git svn init && git fetch.
Now I have two branches in my local repository (master and "svnbranch") and
cherry-picked the very first commit from master to svnbranch (it was
probably not necessary), tagged this commit as "publishedToSvn". Now I want
to add all commits publishedToSvn..master onto svnbranch. I didn't managed
to succeed using git-rebase (probably because of the missing common
commits?) and using git grafts / filter-branch modifies my already published
master.

Is there any way to move/copy commits from one branch to another without a
common base-commit and without a forced push of master?

Thanks in advance,
  Stefan Schulze

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


Re: [PATCH v3 0/2] Make git-svn work with gitdir links

2013-01-23 Thread Barry Wardell
On Wed, Jan 23, 2013 at 2:32 AM, Eric Wong  wrote:
>
> Barry Wardell  wrote:
> > These patches fix a bug which prevented git-svn from working with 
> > repositories
> > which use gitdir links.
> >
> > Changes since v2:
> >  - Rebased onto latest master.
> >  - Added test case which verifies that the problem has been fixed.
> >  - Fixed problems with git svn (init|clone|multi-init).
> >  - All git-svn test cases now pass (except two in t9101 which also failed
> >before these patches).
>
> t9101 did not fail for me before your patches.  However I have a
> patch on top of your 2/2 which should fix things.
>
> `git rev-parse --show-cdup` outputs nothing if GIT_DIR is set,
> so I unset GIT_DIR temporarily.
>
> I'm not sure why --show-cdup behaves like this, though..
>
> Does squashing this on top of your changes fix all your failures?
> I plan on squashing both your changes together with the below:
>
> diff --git a/git-svn.perl b/git-svn.perl
> index c232798..e5bd292 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
> $ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);
> } "Unable to find .git directory\n";
> my $cdup = undef;
> +   my $git_dir = delete $ENV{GIT_DIR};
> git_cmd_try {
> $cdup = command_oneline(qw/rev-parse --show-cdup/);
> chomp $cdup if ($cdup);
> $cdup = "." unless ($cdup && length $cdup);
> -   } "Already at toplevel, but $ENV{GIT_DIR} not found\n";
> +   } "Already at toplevel, but $git_dir not found\n";
> +   $ENV{GIT_DIR} = $git_dir;
> chdir $cdup or die "Unable to chdir up to '$cdup'\n";
> $_repository = Git->repository(Repository => $ENV{GIT_DIR});
>  }


Yes, I can confirm that applying this patch on top of mine makes all
git-svn tests pass again. I have also re-run the tests without my
patch applied and found that they do all indeed pass, so I apologize
for my previous incorrect comment.
--
To unsubscribe from this list: send the line "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: Moving commits from one branch to another

2013-01-23 Thread John Keeping
On Wed, Jan 23, 2013 at 01:04:03PM +0100, Stefan Schulze wrote:
> my team uses a central git-repo since >1500 commits and now we have to sync
> (only one-way is necessary for now) our repository every three weeks with an
> external svn-repo.
> I created the new base-directory (incl. trunk/tags/branches) in svn and
> added it to my local repo using git svn init && git fetch.
> Now I have two branches in my local repository (master and "svnbranch") and
> cherry-picked the very first commit from master to svnbranch (it was
> probably not necessary), tagged this commit as "publishedToSvn". Now I want
> to add all commits publishedToSvn..master onto svnbranch. I didn't managed
> to succeed using git-rebase (probably because of the missing common
> commits?) and using git grafts / filter-branch modifies my already published
> master.
> 
> Is there any way to move/copy commits from one branch to another without a
> common base-commit and without a forced push of master?

Did you try "git rebase" with "--onto"?  You probably want something
like this:

git rebase --onto svnbranch publishedToSvn master


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


Re: What's cooking in git.git (Jan 2013, #08; Tue, 22)

2013-01-23 Thread Chris Rorvick
On Wed, Jan 23, 2013 at 3:28 AM, John Keeping  wrote:
> In my opinion the incremental import support really is substantially
> worse in cvsimport-3 than cvsimport-2.  cvsimport-2 looks at the output
> of git-for-each-ref to calculate the dates from which to continue each
> branch.  cvsps cannot be told this information and so the cvsimport-3
> script just takes the date of the last commit on the current branch.

Do you really need a timestamp per branch, though?  If you have
branches A and B, and B has a commit timestamp 5 minutes after A, you
can infer that nothing happened on A for those five minutes, right?
So maybe a single timestamp is sufficient, it just may not be picking
the right one.  Instead cvsimport-3 should compute the latest
timestamp across all import branches.

Chris
--
To unsubscribe from this list: send the line "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/RFC] Revoke write access to refs and odb after importing another repo's odb

2013-01-23 Thread Nguyễn Thái Ngọc Duy
add_submodule_odb() can be used to import objects from another
repository temporarily. After this point we don't know which objects
are ours, which are external. If we create an object that refers to an
external object, next time git runs, it may find a hole in the object
graph because the external repository may not be imported. The same
goes for pointing a ref to an external SHA-1.

To protect ourselves, once add_submodule_odb() is used:

 - trees, tags and commits cannot be created
 - refs cannot be updated

In certain cases that submodule code knows that it's safe to write, it
can turn the readonly flag off.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 I think this is a good safety check. It catches at least a case in
 t7405.3. I did not investigate further though.

 cache.h  | 1 +
 refs.c   | 2 ++
 sha1_file.c  | 2 ++
 submodule.c  | 7 +++
 5 files changed, 16 insertions(+)

diff --git a/cache.h b/cache.h
index c257953..772d229 100644
--- a/cache.h
+++ b/cache.h
@@ -753,6 +753,7 @@ extern int force_object_loose(const unsigned char *sha1, 
time_t mtime);
 extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
 extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, 
unsigned long mapsize, void *buffer, unsigned long bufsiz);
 extern int parse_sha1_header(const char *hdr, unsigned long *sizep);
+extern int git_repo_readonly();
 
 /* global flag to enable extra checks when accessing packed objects */
 extern int do_check_packed_object_crc;
diff --git a/refs.c b/refs.c
index 541fec2..22b13f4 100644
--- a/refs.c
+++ b/refs.c
@@ -1711,6 +1711,8 @@ struct ref_lock *lock_ref_sha1(const char *refname, const 
unsigned char *old_sha
 struct ref_lock *lock_any_ref_for_update(const char *refname,
 const unsigned char *old_sha1, int 
flags)
 {
+   if (git_repo_readonly())
+   die("repository in read-only mode, cannot update refs");
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, NULL);
diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..b9e8b59 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2575,6 +2575,8 @@ int write_sha1_file(const void *buf, unsigned long len, 
const char *type, unsign
char hdr[32];
int hdrlen;
 
+   if (git_repo_readonly() && strcmp(type, "blob"))
+   die("repository in read-only mode, cannot update object 
database");
/* Normally if we have it in the pack then we do not bother writing
 * it out into .git/objects/??/?{38} file.
 */
diff --git a/submodule.c b/submodule.c
index 2f55436..5eba597 100644
--- a/submodule.c
+++ b/submodule.c
@@ -19,6 +19,7 @@ static struct string_list changed_submodule_paths;
 static int initialized_fetch_ref_tips;
 static struct sha1_array ref_tips_before_fetch;
 static struct sha1_array ref_tips_after_fetch;
+static int readonly;
 
 /*
  * The following flag is set if the .gitmodules file is unmerged. We then
@@ -30,6 +31,11 @@ static struct sha1_array ref_tips_after_fetch;
  */
 static int gitmodules_is_unmerged;
 
+int git_repo_readonly()
+{
+   return readonly;
+}
+
 static int add_submodule_odb(const char *path)
 {
struct strbuf objects_directory = STRBUF_INIT;
@@ -67,6 +73,7 @@ static int add_submodule_odb(const char *path)
/* add possible alternates from the submodule */
read_info_alternates(objects_directory.buf, 0);
prepare_alt_odb();
+   readonly = 1;
 done:
strbuf_release(&objects_directory);
return ret;
-- 
1.8.0.rc2.23.g1fb49df

--
To unsubscribe from this list: send the line "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] Revoke write access to refs and odb after importing another repo's odb

2013-01-23 Thread Duy Nguyen
On Wed, Jan 23, 2013 at 8:34 PM, Nguyễn Thái Ngọc Duy  wrote:
> add_submodule_odb() can be used to import objects from another
> repository temporarily. After this point we don't know which objects
> are ours, which are external. If we create an object that refers to an
> external object, next time git runs, it may find a hole in the object
> graph because the external repository may not be imported. The same
> goes for pointing a ref to an external SHA-1.
>
> To protect ourselves, once add_submodule_odb() is used:
>
>  - trees, tags and commits cannot be created
>  - refs cannot be updated

.. and soon after I pressed send, I realized I missed at least two
other places write should not be allowed:

 - index
 - reflog

But the general idea is probably more important than implementation
details at this stage.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jan 2013, #08; Tue, 22)

2013-01-23 Thread John Keeping
On Wed, Jan 23, 2013 at 07:26:24AM -0600, Chris Rorvick wrote:
> On Wed, Jan 23, 2013 at 3:28 AM, John Keeping  wrote:
> > In my opinion the incremental import support really is substantially
> > worse in cvsimport-3 than cvsimport-2.  cvsimport-2 looks at the output
> > of git-for-each-ref to calculate the dates from which to continue each
> > branch.  cvsps cannot be told this information and so the cvsimport-3
> > script just takes the date of the last commit on the current branch.
> 
> Do you really need a timestamp per branch, though?  If you have
> branches A and B, and B has a commit timestamp 5 minutes after A, you
> can infer that nothing happened on A for those five minutes, right?
> So maybe a single timestamp is sufficient, it just may not be picking
> the right one.  Instead cvsimport-3 should compute the latest
> timestamp across all import branches.

The problem is telling which is an import branch, since it currently
just used "refs/heads/".

I do have a change to write the timestamp to a file, which takes the
newest commit across all of the branches that have changed during an
import.  That may well be good enough but doesn't let you incrementally
update a repository that has been cloned from elsewhere.


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


Minor gitk bug - update on empty project : can't read "viewmainheadid_orig(1)": no such variable

2013-01-23 Thread Indigo Frankencastle
One of these "weird usage causes weird bugs".

One way to reproduce:

mkdir foo
cd foo
git init
git commit -am "Initial"
gitk &
# or
gitk --all &


Resulting in (gitk-git/gitk line 503 - 512):

can't read "viewmainheadid_orig(1)": no such variable
can't read "viewmainheadid_orig(1)": no such variable
while executing
"if {$mainheadid ne $viewmainheadid_orig($view)} {
if {$showlocalchanges} {
dohidelocalchanges
}
set viewmainheadid($view) $mainheadid
set vie..."
(procedure "updatecommits" line 13)
invoked from within
"updatecommits"
(command bound to event)

As "$mainheadid" is not set, should there be something in the
direction of this perhaps?

...
Subject: [PATCH] gitk catch missing head id on update

---
 gitk-git/gitk |5 +
 1 file changed, 5 insertions(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..fa869d7 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -499,6 +499,11 @@ proc updatecommits {} {

 set hasworktree [hasworktree]
 rereadrefs
+if {$mainheadid eq ""} {
+   # error_popup "[mc "Error updating commits:"] No main HEAD id"
+   show_status [mc "No main HEAD id"]
+   return {}
+}
 set view $curview
 if {$mainheadid ne $viewmainheadid_orig($view)} {
if {$showlocalchanges} {
-- 
1.7.10.4

...
Or perhaps some logic in:
proc rereadrefs {}


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


AW: Moving commits from one branch to another

2013-01-23 Thread Stefan Schulze
> > Is there any way to move/copy commits from one branch to another
> > without a common base-commit and without a forced push of master?
>
> Did you try "git rebase" with "--onto"?  You probably want something
> like this:
> 
> git rebase --onto svnbranch publishedToSvn master

I already tried this some days ago, but wasn't sure about the result. The
resulting history looks exactly what I expected, but all the commits are on
master after executing this commands and svnbranch only contains the
original two commits (svn-commit creating the root directory and the
cherry-picked commit from master)

Does the current branch matter if I call git-rebase with the
-argument?

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


segmentation fault (nullpointer) with git log --submodule -p

2013-01-23 Thread Armin
Hello dear git people.

I experience a reproducible segmentation fault on one of my repositories when 
doing a "git log --submodule -p", tested with newest version on Arch Linux (git 
version 1.8.1.1) and built fresh (git version 1.8.1.1.347.g9591fcc), tried on 2 
seperate systems:


Program terminated with signal 11, Segmentation fault.
#0  0x004b51e5 in parse_commit_header (context=0x769b6980) at 
pretty.c:752
752 for (i = 0; msg[i]; i++) {
(gdb) bt
#0  0x004b51e5 in parse_commit_header (context=0x769b6980) at 
pretty.c:752
#1  format_commit_one (context=, placeholder=0x526b1e "s", 
sb=0x769b6ad0) at pretty.c:1157
#2  format_commit_item (sb=0x769b6ad0, placeholder=0x526b1e "s", 
context=) at pretty.c:1224
#3  0x004dacd9 in strbuf_expand (sb=sb@entry=0x769b6ad0, 
format=0x526b1e "s", format@entry=0x526b18 "  %m %s", fn=fn@entry=0x4b4730 
, context=context@entry=0x769b6980)
at strbuf.c:247
#4  0x004b5816 in format_commit_message (commit=commit@entry=0x1ffafd8, 
format=format@entry=0x526b18 "  %m %s", sb=sb@entry=0x769b6ad0, 
pretty_ctx=pretty_ctx@entry=0x769b6af0) at pretty.c:1284
#5  0x004dde52 in print_submodule_summary (reset=0x754640 "\033[m", 
add=0x754708 "\033[32m", del=0x7546e0 "\033[31m", f=0x7f0685bac7a0, 
rev=0x769b6b40) at submodule.c:236
#6  show_submodule_summary (f=0x7f0685bac7a0, path=, 
one=one@entry=0x1ff2af0 
"\020\\vC\371\070\vJ\352\344\205\340\226u\273\021\372\330\234\004", 
two=two@entry=0x2030a60 "\301a(\350\371\372\340mb[խo_\272\301\223V˙", 
dirty_submodule=, meta=meta@entry=0x754690 "\033[1m", 
del=del@entry=0x7546e0 "\033[31m", add=0x754708 "\033[32m", 
reset=reset@entry=0x754640 "\033[m") at submodule.c:307
#7  0x0048dd1d in builtin_diff (name_a=name_a@entry=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", name_b=name_b@entry=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", 
one=one@entry=0x1ff2af0, two=two@entry=0x2030a60, xfrm_msg=0x2039a20 
"\033[1mindex 105c764..c16128e 16\033[m\n", 
must_show_header=must_show_header@entry=0, o=o@entry=0x769b7b88, 
complete_rewrite=complete_rewrite@entry=0) at diff.c:2267
#8  0x0048e60e in run_diff_cmd (pgm=pgm@entry=0x0, name=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", other=, 
attr_path=attr_path@entry=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", one=one@entry=0x1ff2af0, 
two=two@entry=0x2030a60, msg=msg@entry=0x769b74b0, 
o=o@entry=0x769b7b88, p=p@entry=0x20371b0)
at diff.c:3057

(gdb) bt
#0  0x004b51e5 in parse_commit_header (context=0x769b6980) at 
pretty.c:752
#1  format_commit_one (context=, placeholder=0x526b1e "s", 
sb=0x769b6ad0) at pretty.c:1157
#2  format_commit_item (sb=0x769b6ad0, placeholder=0x526b1e "s", 
context=) at pretty.c:1224
#3  0x004dacd9 in strbuf_expand (sb=sb@entry=0x769b6ad0, 
format=0x526b1e "s", format@entry=0x526b18 "  %m %s", fn=fn@entry=0x4b4730 
, context=context@entry=0x769b6980)
at strbuf.c:247
#4  0x004b5816 in format_commit_message (commit=commit@entry=0x1ffafd8, 
format=format@entry=0x526b18 "  %m %s", sb=sb@entry=0x769b6ad0, 
pretty_ctx=pretty_ctx@entry=0x769b6af0) at pretty.c:1284
#5  0x004dde52 in print_submodule_summary (reset=0x754640 "\033[m", 
add=0x754708 "\033[32m", del=0x7546e0 "\033[31m", f=0x7f0685bac7a0, 
rev=0x769b6b40) at submodule.c:236
#6  show_submodule_summary (f=0x7f0685bac7a0, path=, 
one=one@entry=0x1ff2af0 
"\020\\vC\371\070\vJ\352\344\205\340\226u\273\021\372\330\234\004", 
two=two@entry=0x2030a60 "\301a(\350\371\372\340mb[խo_\272\301\223V˙", 
dirty_submodule=, meta=meta@entry=0x754690 "\033[1m", 
del=del@entry=0x7546e0 "\033[31m", add=0x754708 "\033[32m", 
reset=reset@entry=0x754640 "\033[m") at submodule.c:307
#7  0x0048dd1d in builtin_diff (name_a=name_a@entry=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", name_b=name_b@entry=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", 
one=one@entry=0x1ff2af0, two=two@entry=0x2030a60, xfrm_msg=0x2039a20 
"\033[1mindex 105c764..c16128e 16\033[m\n", 
must_show_header=must_show_header@entry=0, o=o@entry=0x769b7b88, 
complete_rewrite=complete_rewrite@entry=0) at diff.c:2267
#8  0x0048e60e in run_diff_cmd (pgm=pgm@entry=0x0, name=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", other=, 
attr_path=attr_path@entry=0x1ff2b50 
"Packages/Application/Amadeus.Somea.Dialog", one=one@entry=0x1ff2af0, 
two=two@entry=0x2030a60, msg=msg@entry=0x769b74b0, 
o=o@entry=0x769b7b88, p=p@entry=0x20371b0)
at diff.c:3057
#9  0x0048eb3d in run_diff (o=0x769b7b88, p=0x20371b0) at 
diff.c:3145
#10 diff_flush_patch (o=0x769b7b88, p=0x20371b0) at diff.c:3979
#11 diff_flush_patch (p=0x20371b0, o=0x769b7b88) at diff.c:3970
#12 0x0048f15f in diff_flush (options=options@entry=0x769b7b88) at 
diff.c:4500
#13 0x00

Re: GIT get corrupted on lustre

2013-01-23 Thread Sébastien Boisvert

On 01/22/2013 05:14 PM, Thomas Rast wrote:

Eric Chamberland  writes:


So, hum, do we have some sort of conclusion?

Shall it be a fix for git to get around that lustre "behavior"?

If something can be done in git it would be great: it is a *lot*
easier to change git than the lustre filesystem software for a cluster
in running in production mode... (words from cluster team) :-/


I thought you already established that simply disabling the progress
display is a sufficient workaround?  If that doesn't help, you can try
patching out all use of SIGALRM within git.



In git (9591fcc6d66), I have found these SIGALRM signal handling:

builtin/log.c:268:  sigaction(SIGALRM, &sa, NULL);
builtin/log.c:285:  signal(SIGALRM, SIG_IGN);
compat/mingw.c:1590:mingw_raise(SIGALRM);
compat/mingw.c:1666:if (sig != SIGALRM)
compat/mingw.c:1668:error("sigaction only implemented for 
SIGALRM");
compat/mingw.c:1683:case SIGALRM:
compat/mingw.c:1702:case SIGALRM:
compat/mingw.c:1706:exit(128 + SIGALRM);
compat/mingw.c:1708:timer_fn(SIGALRM);
compat/mingw.h:42:#define SIGALRM 14
perl/Git/SVN.pm:2121:   SIGALRM, SIGUSR1, SIGUSR2);
progress.c:56:  sigaction(SIGALRM, &sa, NULL);
progress.c:68:  signal(SIGALRM, SIG_IGN);


I suppose that compat/mingw.{h,c} and SVN.pm can be ignored as our patch to work
around this problem won't be pushed upstream because the real problem is not in 
git, right ?

If I understand correctly, some VFS system calls get interrupted by SIGALRM, 
but when
they resume (via SA_RESTART) they return EINTR. Thomas said that these failed 
calls may need to be retried,
but that open(O_CREAT|O_EXCL) is still tricky around this case.


progress.c SIGALRM code paths are for progress and therefore are required, 
right ?

builtin/log.c SIGALRM code paths are for early output, and the comments in the 
code say that

   "If we can get the whole output in less than a tenth of a second, don't even 
bother doing the
early-output thing."


So where do I start for the patch ?


Other than that I agree with Junio, from what we've seen so far, Lustre
returns EINTR on all sorts of calls that simply aren't allowed to do so.




--
---
Spécialiste en granularité (1 journée / semaine)
Calcul Québec / Calcul Canada
Pavillon Adrien-Pouliot, Université Laval, Québec (Québec), Canada
--
To unsubscribe from this list: send the line "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: Moving commits from one branch to another

2013-01-23 Thread John Keeping
On Wed, Jan 23, 2013 at 03:13:19PM +0100, Stefan Schulze wrote:
> > > Is there any way to move/copy commits from one branch to another
> > > without a common base-commit and without a forced push of master?
> >
> > Did you try "git rebase" with "--onto"?  You probably want something
> > like this:
> > 
> > git rebase --onto svnbranch publishedToSvn master
> 
> I already tried this some days ago, but wasn't sure about the result. The
> resulting history looks exactly what I expected, but all the commits are on
> master after executing this commands and svnbranch only contains the
> original two commits (svn-commit creating the root directory and the
> cherry-picked commit from master)

Ah, I missed that you wanted to update svnbranch.  I don't think there's
a single command that will do that, but this should work:

git rebase --onto svnbranch publishedToSvn master^0
git checkout -B svnbranch HEAD

This uses a detached head to avoid modifying the wrong branch and then
updates "svnbranch" to point at that after the rebase.

> Does the current branch matter if I call git-rebase with the
> -argument?

No it will checkout that branch first.


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


Re: GIT get corrupted on lustre

2013-01-23 Thread Sébastien Boisvert

[I forgot to subscribe to the git mailing list, sorry for that]

On 01/22/2013 05:14 PM, Thomas Rast wrote:

Eric Chamberland  writes:


So, hum, do we have some sort of conclusion?

Shall it be a fix for git to get around that lustre "behavior"?

If something can be done in git it would be great: it is a *lot*
easier to change git than the lustre filesystem software for a cluster
in running in production mode... (words from cluster team) :-/


I thought you already established that simply disabling the progress
display is a sufficient workaround?  If that doesn't help, you can try
patching out all use of SIGALRM within git.



In git (9591fcc6d66), I have found these SIGALRM signal handling:

builtin/log.c:268:sigaction(SIGALRM, &sa, NULL);
builtin/log.c:285:signal(SIGALRM, SIG_IGN);
compat/mingw.c:1590:mingw_raise(SIGALRM);
compat/mingw.c:1666:if (sig != SIGALRM)
compat/mingw.c:1668:error("sigaction only implemented for SIGALRM");
compat/mingw.c:1683:case SIGALRM:
compat/mingw.c:1702:case SIGALRM:
compat/mingw.c:1706:exit(128 + SIGALRM);
compat/mingw.c:1708:timer_fn(SIGALRM);
compat/mingw.h:42:#define SIGALRM 14
perl/Git/SVN.pm:2121:SIGALRM, SIGUSR1, SIGUSR2);
progress.c:56:sigaction(SIGALRM, &sa, NULL);
progress.c:68:signal(SIGALRM, SIG_IGN);


I suppose that compat/mingw.{h,c} and SVN.pm can be ignored as our patch to work
around this problem won't be pushed upstream because the real problem is not in 
git, right ?

If I understand correctly, some VFS system calls get interrupted by SIGALRM, 
but when
they resume (via SA_RESTART) they return EINTR. Thomas said that these failed 
calls may need to be retried,
but that open(O_CREAT|O_EXCL) is still tricky around this case.


progress.c SIGALRM code paths are for progress and therefore are required, 
right ?

builtin/log.c SIGALRM code paths are for early output, and the comments in the 
code say that

   "If we can get the whole output in less than a tenth of a second, don't even 
bother doing the
early-output thing."


So where do I start for the patch ?


Other than that I agree with Junio, from what we've seen so far, Lustre
returns EINTR on all sorts of calls that simply aren't allowed to do so.




--
---
Spécialiste en granularité (1 journée / semaine)
Calcul Québec / Calcul Canada
Pavillon Adrien-Pouliot, Université Laval, Québec (Québec), Canada
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: GIT get corrupted on lustre

2013-01-23 Thread Erik Faye-Lund
On Tue, Jan 22, 2013 at 11:14 PM, Thomas Rast  wrote:
> Eric Chamberland  writes:
>
> Other than that I agree with Junio, from what we've seen so far, Lustre
> returns EINTR on all sorts of calls that simply aren't allowed to do so.

I don't think this analysis is 100% accurate, POSIX allows error codes
to be generated other than those defined. From
http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html:

"Implementations may support additional errors not included in this
list, *may generate errors included in this list under circumstances
other than those described here*, or may contain extensions or
limitations that prevent some errors from occurring."

So I don't think Lustre violates POSIX by erroring with errno=EINTR,
but I also think guarding every single function call for EINTR just to
be safe to be insane :)

However, looking at Eric's log, I can't see that being what has
happened here - grepping it for EINTR does not produce a single match.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: GIT get corrupted on lustre

2013-01-23 Thread Thomas Rast
Erik Faye-Lund  writes:

> On Tue, Jan 22, 2013 at 11:14 PM, Thomas Rast  wrote:
>> Eric Chamberland  writes:
>>
>> Other than that I agree with Junio, from what we've seen so far, Lustre
>> returns EINTR on all sorts of calls that simply aren't allowed to do so.
>
> I don't think this analysis is 100% accurate, POSIX allows error codes
> to be generated other than those defined. From
> http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html:
>
> "Implementations may support additional errors not included in this
> list, *may generate errors included in this list under circumstances
> other than those described here*, or may contain extensions or
> limitations that prevent some errors from occurring."

That same page says, however:

  For functions under the Threads option for which [EINTR] is not listed
  as a possible error condition in this volume of IEEE Std 1003.1-2001,
  an implementation shall not return an error code of [EINTR].

-- 
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: GIT get corrupted on lustre

2013-01-23 Thread Erik Faye-Lund
On Wed, Jan 23, 2013 at 4:32 PM, Thomas Rast  wrote:
> Erik Faye-Lund  writes:
>
>> On Tue, Jan 22, 2013 at 11:14 PM, Thomas Rast  wrote:
>>> Eric Chamberland  writes:
>>>
>>> Other than that I agree with Junio, from what we've seen so far, Lustre
>>> returns EINTR on all sorts of calls that simply aren't allowed to do so.
>>
>> I don't think this analysis is 100% accurate, POSIX allows error codes
>> to be generated other than those defined. From
>> http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html:
>>
>> "Implementations may support additional errors not included in this
>> list, *may generate errors included in this list under circumstances
>> other than those described here*, or may contain extensions or
>> limitations that prevent some errors from occurring."
>
> That same page says, however:
>
>   For functions under the Threads option for which [EINTR] is not listed
>   as a possible error condition in this volume of IEEE Std 1003.1-2001,
>   an implementation shall not return an error code of [EINTR].

Yes, but surely that's for pthreads functions, no? utime is not one of
those functions...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


AW: Moving commits from one branch to another

2013-01-23 Thread Schulze, Stefan
> git rebase --onto svnbranch publishedToSvn master^0
> git checkout -B svnbranch HEAD

Great! This does exactly what I want!

Thanks for your support,
  Stefan Schulze

--
ckc ag
Sitz:
Am Alten Bahnhof 13
38122 Braunschweig

Telefon +49 (0)531 / 80 110 0
Telefax +49 (0)531 / 80 110 18444
http://www.ckc-group.de

Amtsgericht Braunschweig
HRB 5405

Vorstand:
H.-G. Christian Krentel
(Vorsitzender)
Karsten Kisser

Aufsichtsrat:
Dr. Heinz-Werner Weinrich
(Vorsitzender)
Jens Fokuhl
(stellv. Vorsitzender)
Prof. Dr. Reinhold Haux
Cäsar Jaworski
Dr. Rita Schulz
Thorsten Sponholz
--
N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: GIT get corrupted on lustre

2013-01-23 Thread Thomas Rast
Erik Faye-Lund  writes:

> On Wed, Jan 23, 2013 at 4:32 PM, Thomas Rast  wrote:
>> Erik Faye-Lund  writes:
>>
>>> POSIX allows error codes
>>> to be generated other than those defined. From
>>> http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html:
>>>
>>> "Implementations may support additional errors not included in this
>>> list, *may generate errors included in this list under circumstances
>>> other than those described here*, or may contain extensions or
>>> limitations that prevent some errors from occurring."
>>
>> That same page says, however:
>>
>>   For functions under the Threads option for which [EINTR] is not listed
>>   as a possible error condition in this volume of IEEE Std 1003.1-2001,
>>   an implementation shall not return an error code of [EINTR].
>
> Yes, but surely that's for pthreads functions, no? utime is not one of
> those functions...

Ah, my bad.  In fact in

  http://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html

there is a paragraph "Signal Effects on Other Functions", which says

  The most common behavior of an interrupted function after a
  signal-catching function returns is for the interrupted function to
  give an [EINTR] error unless the SA_RESTART flag is in effect for the
  signal. However, there are a number of specific exceptions, including
  sleep() and certain situations with read() and write().

  The historical implementations of many functions defined by IEEE Std
  1003.1-2001 are not interruptible[...]

  Functions not mentioned explicitly as interruptible may be so on some
  implementations, possibly as an extension where the function gives an
  [EINTR] error. There are several functions (for example, getpid(),
  getuid()) that are specified as never returning an error, which can
  thus never be extended in this way.

  If a signal-catching function returns while the SA_RESTART flag is in
  effect, an interrupted function is restarted at the point it was
  interrupted. Conforming applications cannot make assumptions about the
  internal behavior of interrupted functions, even if the functions are
  async-signal-safe. For example, suppose the read() function is
  interrupted with SA_RESTART in effect, the signal-catching function
  closes the file descriptor being read from and returns, and the read()
  function is then restarted; in this case the application cannot assume
  that the read() function will give an [EBADF] error, since read()
  might have checked the file descriptor for validity before being
  interrupted.

Taken together this should mean that the bug is in fact simply that the
calls do not *restart*.  They are (like you say) allowed to return EINTR
despite not being specified to, *but* SA_RESTART should restart it.

Now, does that make it a lustre bug or a glibc bug? :-)

-- 
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: GIT get corrupted on lustre

2013-01-23 Thread Erik Faye-Lund
On Wed, Jan 23, 2013 at 4:44 PM, Thomas Rast  wrote:
> Erik Faye-Lund  writes:
>
>> On Wed, Jan 23, 2013 at 4:32 PM, Thomas Rast  wrote:
>>> Erik Faye-Lund  writes:
>>>
 POSIX allows error codes
 to be generated other than those defined. From
 http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html:

 "Implementations may support additional errors not included in this
 list, *may generate errors included in this list under circumstances
 other than those described here*, or may contain extensions or
 limitations that prevent some errors from occurring."
>>>
>>> That same page says, however:
>>>
>>>   For functions under the Threads option for which [EINTR] is not listed
>>>   as a possible error condition in this volume of IEEE Std 1003.1-2001,
>>>   an implementation shall not return an error code of [EINTR].
>>
>> Yes, but surely that's for pthreads functions, no? utime is not one of
>> those functions...
>
> Ah, my bad.  In fact in
>
>   http://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html
>
> there is a paragraph "Signal Effects on Other Functions", which says
>
> 
>
> Taken together this should mean that the bug is in fact simply that the
> calls do not *restart*.  They are (like you say) allowed to return EINTR
> despite not being specified to, *but* SA_RESTART should restart it.
>

Right, thanks for clearing that up.

> Now, does that make it a lustre bug or a glibc bug? :-)

That's kind of uninteresting, the important bit is that it is indeed a
bug (outside of Git).
--
To unsubscribe from this list: send the line "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 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE

2013-01-23 Thread Junio C Hamano
Jeff King  writes:

> On Mon, Jan 21, 2013 at 10:30:29PM -0800, Junio C Hamano wrote:
>
>> When we push to update an existing ref, if:
>> 
>>  * we do not have the object at the tip of the remote; or
>>  * the object at the tip of the remote is not a commit; or
>>  * the object we are pushing is not a commit,
>> 
>> there is no point suggesting to fetch, integrate and push again.
>> 
>> If we do not have the current object at the tip of the remote, we
>> should tell the user to fetch first and evaluate the situation
>> before deciding what to do next.
>
> Should we? I know that it is more correct to do so, because we do not
> even know for sure that the remote object is a commit, and fetching
> _might_ lead to us saying "hey, this is not something that can be
> fast-forwarded".
>
> But by far the common case will be that it _is_ a commit, and the right
> thing is going to be to pull
> Is the extra hassle in the common case worth it for the off chance that
> we might give a more accurate message? Should the "fetch first" message
> be some hybrid that covers both cases accurately, but still points the
> user towards "git pull" (which will fail anyway if the remote ref is not
> a commit)?

I was actually much less happy with "needs force" than this one, as
you have to assume too many things for the message to be a useful
and a safe advise: the user has actually examined the situation and
forcing the push is the right thing to do.  Both old and new objects
exist, so the user _could_ have done so, but did he really check
them, thought about the situation and made the right decision?
Perhaps the attempted push had a typo in the object name it wanted
to update the other end with, and the right thing to do is not to
force but to fix the refspec instead?  "You need --force to perform
this push" was a very counter-productive advice in this case, but I
didn't think of a better wording.

The "fetch first and inspect" was an attempt to reduce the risk of
that "needs force" message that could encourage brainless forced
pushes.  Perhaps if we reword "needs force" to something less risky,
we do not have to be so explicit in "You have to fetch first and
examine".

How about doing this?

For "needs force" cases, we say this instead:

 hint: you cannot update a ref that points at a non-commit object, or
 hint: update a ref to point at a non-commit object, without --force.

Being explicit about "non-commit" twice will catch user's eyes and
cause him to double check that it is not a mistyped LHS of the push
refspec (if he is sending a non-commit) or mistyped RHS (if the ref
is pointing at a non-commit).  If he _is_ trying to push a blob out,
the advice makes it clear what to do next: he does want to force it.

If we did that, then we could loosen the "You should fetch first"
case to say something like this:

 hint: you do not have the object at the tip of the remote ref;
 hint: perhaps you want to pull from there first?

This explicitly denies one of Chris's wish "we shouldn't suggest to
merge something that we may not be able to", but in the "You should
fetch first" case, we cannot fundamentally know if we can merge
until we fetch.  I agree with you that the most common case is that
the unknown object is a commit, and that suggesting to pull is a
good compromise.

Note that you _could_ split the "needs force" case into two, namely,
"cannot replace a non-commit" and "cannot push a non-commit".  You
could even further split them into combinations (e.g. an attempt to
replace an annotated tag with a commit and an attempt to replace a
tree with a commit may be different situations), but I think the
advices we can give to these cases would end up being the same, so I
tend to think it is not worth it.  That is what I meant by "I do not
expect me doing the type-based policy myself" in the concluding
message of the series.
--
To unsubscribe from this list: send the line "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: Bug in latest gitk - can't click lines connecting commits

2013-01-23 Thread Junio C Hamano
Paul Mackerras  writes:

> On Tue, Jan 22, 2013 at 09:28:23AM -0800, Junio C Hamano wrote:
>> 
>> I notice that I have a handful of commits that I haven't pulled from
>> your repository, and the last commit on your 'master' is about 20
>> days old.  Is it safe for me to pull these now?
>
> Yes, please pull them now.

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


Re: [PATCH v2] all: new command used for multi-repo operations

2013-01-23 Thread Junio C Hamano
Lars Hjemli  writes:

> +static int walk(struct strbuf *path, int argc, const char **argv)
> +{
> + DIR *dir;
> + struct dirent *ent;
> + struct stat st;
> + size_t len;
> +
> + dir = opendir(path->buf);
> + if (!dir)
> + return errno;
> + strbuf_addstr(path, "/");
> + len = path->len;
> + while ((ent = readdir(dir))) {
> + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
> + continue;
> + if (!strcmp(ent->d_name, ".git")) {
> + strbuf_addstr(path, ent->d_name);
> + setenv(GIT_DIR_ENVIRONMENT, path->buf, 1);
> + strbuf_setlen(path, len - 1);
> + setenv(GIT_WORK_TREE_ENVIRONMENT, path->buf, 1);
> + handle_repo(path->buf, argv);
> + strbuf_addstr(path, "/");
> + continue;
> + }
> + strbuf_setlen(path, len);
> + strbuf_addstr(path, ent->d_name);
> + switch (DTYPE(ent)) {
> + case DT_UNKNOWN:
> + /* Use stat() instead of lstat(), since we want to
> +  * know if we can follow this path into another
> +  * directory - it's  not important if it's actually
> +  * a symlink which gets us there.
> +  */

This is wrong if you are on a platform that does have d_type, no?
It may say it is a symbolic link, and until you stat you wouldn't
know if it may lead to a directory.  You can add "case DT_LNK:" that
behaves the same as DT_UNKNOWN, I think.

> + if (stat(path->buf, &st) || !S_ISDIR(st.st_mode))
> + break;
> + /* fallthrough */
> + case DT_DIR:
> + walk(path, argc, argv);
> + break;
> + }
> + strbuf_setlen(path, len);
> + }

But I still do not think this loop is correct.  In a repository that
has a working tree, you would learn that directory $D has $D/.git in
it, feed $D to handle_repo(), and then descend into $D/.git/objects/,
$D/.git/refs, and other random directories to see if you can find
other repositories.  That is just not right.

If this check were doing something like "The directory $D is worth
handing to handle_repo() if it has all of the following: objects/,
refs/ and HEAD that either points inside refs/ or 40-hex.", then it
would make a lot more sense to me, including the part that goes on
to check sibling directories.  As a bonus side effect, it will give
you a support for bare repositories for free.
--
To unsubscribe from this list: send the line "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] Revoke write access to refs and odb after importing another repo's odb

2013-01-23 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> add_submodule_odb() can be used to import objects from another
> repository temporarily. After this point we don't know which objects
> are ours, which are external. If we create an object that refers to an
> external object, next time git runs, it may find a hole in the object
> graph because the external repository may not be imported. The same
> goes for pointing a ref to an external SHA-1.
>
> To protect ourselves, once add_submodule_odb() is used:
>
>  - trees, tags and commits cannot be created
>  - refs cannot be updated
>
> In certain cases that submodule code knows that it's safe to write, it
> can turn the readonly flag off.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  I think this is a good safety check.

Two step implementation to bring "read-only" support into a testable
shape and then flip that bit in add_submdule_odb() would be a
sensible approach.

I however have this suspicion that this will become a losing battle
and we would be better off getting rid of add_submodule_odb();
instead operations that work across repositories will be done as a
subprocess, which will get us back closer to one of the original
design goals of submodule support to have a clear separation between
the superproject and its submodules.
--
To unsubscribe from this list: send the line "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] all: new command used for multi-repo operations

2013-01-23 Thread Junio C Hamano
Junio C Hamano  writes:

> But I still do not think this loop is correct.  In a repository that
> has a working tree, you would learn that directory $D has $D/.git in
> it, feed $D to handle_repo(), and then descend into $D/.git/objects/,
> $D/.git/refs, and other random directories to see if you can find
> other repositories

Ahh, no, you don't.

I still think calling is_git_directory() on $D + "/.git" would be a
better implementation, though.
--
To unsubscribe from this list: send the line "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: What's cooking in git.git (Jan 2013, #08; Tue, 22)

2013-01-23 Thread Junio C Hamano
John Keeping  writes:

> My preference would be for something like this, possibly with an
> expanded examples section showing how to pipe the output of cvsps-3 or
> cvs2git into git-fast-import:
>
> -- >8 --
>
> diff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt
> index 9d5353e..20b846e 100644
> --- a/Documentation/git-cvsimport.txt
> +++ b/Documentation/git-cvsimport.txt
> @@ -18,6 +18,11 @@ SYNOPSIS
>  
>  DESCRIPTION
>  ---
> +*WARNING:* `git cvsimport` uses cvsps version 2, which is considered
> +deprecated; it does not work with cvsps version 3 and later.  If you are
> +performing a one-shot import of a CVS repository consider using cvsps-3,
> +cvs2git or parsecvs directly.
> +
>  Imports a CVS repository into git. It will either create a new
>  repository, or incrementally import into an existing one.
>  
> -- 8< --

OK, that is certainly a lot simpler to explain.

Is it "it does not work yet with cvsps3", or "it will not ever work
with cvsps3"?  The impression I am getting is that it is the latter.

Also, should we have a suggestion to people who are *not* performing
a one-shot import, i.e. doing incremental or bidirectional?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: GIT get corrupted on lustre

2013-01-23 Thread Jonathan Nieder
Thomas Rast wrote:

> Taken together this should mean that the bug is in fact simply that the
> calls do not *restart*.  They are (like you say) allowed to return EINTR
> despite not being specified to, *but* SA_RESTART should restart it.
>
> Now, does that make it a lustre bug or a glibc bug? :-)

The kernel takes care of SA_RESTART, if I remember correctly.  (See
arch/x86/kernel/signal.c::handle_signal() case -ERESTARTSYS.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Defensive publication on Git replication

2013-01-23 Thread Don Marti
Just wanted to get this simple scheme out there in the hope of minimizing
patent troll risks for people working on replication.

You run an update hook that blocks pushes unless the branch reference in
the repository matches the corresponding reference stored in a synchronized
system, and flood the objects out using an inexpensive, simple system.

(Apologies if this is too obvious for the list; I didn't want to take
chances with the patent office definition of "obvious.")

 Replication Mechanism for a Distributed Version Control System 
 http://ip.com/IPCOM/000225058

 Disclosed is a mechanism for replication of content repositories of a
 distributed version control system (DVCS), which is unique in that objects
 and branch references are transferred between nodes by independent means,
 with only branch references subject to synchronization among nodes. The
 object of this disclosure is to simplify deployment of a DVCS in a
 replicated configuration. Replication can be performed efficiently by
 transferring objects using a key-value store, such as a distributed hash
 table (DHT), independently of a repository's branch references, which are
 transferred using a synchronized data store.

-- 
Don Marti   Perforce Software  +1-510-473-3142
http://www.perforce.com/git-fusion
--
To unsubscribe from this list: send the line "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] all: new command used for multi-repo operations

2013-01-23 Thread Lars Hjemli
On Wed, Jan 23, 2013 at 5:52 PM, Junio C Hamano  wrote:
> Lars Hjemli  writes:
>
>> +static int walk(struct strbuf *path, int argc, const char **argv)
>> +{
>> + DIR *dir;
>> + struct dirent *ent;
>> + struct stat st;
>> + size_t len;
>> +
>> + dir = opendir(path->buf);
>> + if (!dir)
>> + return errno;
>> + strbuf_addstr(path, "/");
>> + len = path->len;
>> + while ((ent = readdir(dir))) {
>> + if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, ".."))
>> + continue;
>> + if (!strcmp(ent->d_name, ".git")) {
>> + strbuf_addstr(path, ent->d_name);
>> + setenv(GIT_DIR_ENVIRONMENT, path->buf, 1);
>> + strbuf_setlen(path, len - 1);
>> + setenv(GIT_WORK_TREE_ENVIRONMENT, path->buf, 1);
>> + handle_repo(path->buf, argv);
>> + strbuf_addstr(path, "/");
>> + continue;
>> + }
>> + strbuf_setlen(path, len);
>> + strbuf_addstr(path, ent->d_name);
>> + switch (DTYPE(ent)) {
>> + case DT_UNKNOWN:
>> + /* Use stat() instead of lstat(), since we want to
>> +  * know if we can follow this path into another
>> +  * directory - it's  not important if it's actually
>> +  * a symlink which gets us there.
>> +  */
>
> This is wrong if you are on a platform that does have d_type, no?
> It may say it is a symbolic link, and until you stat you wouldn't
> know if it may lead to a directory.  You can add "case DT_LNK:" that
> behaves the same as DT_UNKNOWN, I think.

Yeah, that seems right, thanks.

-- 
larsh
--
To unsubscribe from this list: send the line "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] all: new command used for multi-repo operations

2013-01-23 Thread Lars Hjemli
On Wed, Jan 23, 2013 at 6:04 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> But I still do not think this loop is correct.  In a repository that
>> has a working tree, you would learn that directory $D has $D/.git in
>> it, feed $D to handle_repo(), and then descend into $D/.git/objects/,
>> $D/.git/refs, and other random directories to see if you can find
>> other repositories
>
> Ahh, no, you don't.
>
> I still think calling is_git_directory() on $D + "/.git" would be a
> better implementation, though.

Except for the .gitfile case, which is_git_directory() doesn't seem to
handle. I guess I can invoke read_gitfile() when i see that .git is a
file.

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


Re: GIT get corrupted on lustre

2013-01-23 Thread Sébastien Boisvert

Hello,

Here is a patch (with git format-patch) that removes any timer if NO_SETITIMER 
is set.


Éric:

To test it with your workflow:

$ module load apps/git/1.8.1.1.348.g78eb407-NO_SETITIMER-patch

$ git clone ...


  Sébastien


On 01/22/2013 05:14 PM, Thomas Rast wrote:

Eric Chamberland  writes:


So, hum, do we have some sort of conclusion?

Shall it be a fix for git to get around that lustre "behavior"?

If something can be done in git it would be great: it is a *lot*
easier to change git than the lustre filesystem software for a cluster
in running in production mode... (words from cluster team) :-/


I thought you already established that simply disabling the progress
display is a sufficient workaround?  If that doesn't help, you can try
patching out all use of SIGALRM within git.

Other than that I agree with Junio, from what we've seen so far, Lustre
returns EINTR on all sorts of calls that simply aren't allowed to do so.




--
---
Spécialiste en granularité (1 journée / semaine)
Calcul Québec / Calcul Canada
Pavillon Adrien-Pouliot, Université Laval, Québec (Québec), Canada
>From 78eb4075d98eb9cdc57210c63b8d8de8a3d0cd9e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Boisvert?= 
Date: Wed, 23 Jan 2013 13:10:57 -0500
Subject: [PATCH] don't use timers if NO_SETITIMER is set
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With NO_SETITIMER, the user experience on legacy Lustre is fixed,
but there is no early progress.

The patch has no effect on the resulting git executable if NO_SETITIMER is
not set (the default). So by default this patch has no effect at all, which
is good.

git tests:

$ make clean
$ make NO_SETITIMER=YesPlease
$ make test NO_SETITIMER=YesPlease &> make-test.log

$ grep "^not ok" make-test.log |grep -v "# TODO known breakage"|wc -l
0
$ grep "^ok" make-test.log |wc -l
9531
$ grep "^not ok" make-test.log |wc -l
65

No timers with NO_SETITIMER:

$ objdump -d ./git|grep setitimer|wc -l
0
$ objdump -d ./git|grep alarm|wc -l
0

Timers without NO_SETITIMER:

$ objdump -d /software/apps/git/1.8.1/bin/git|grep setitimer|wc -l
5
$ objdump -d /software/apps/git/1.8.1/bin/git|grep alarm|wc -l
0

Signed-off-by: Sébastien Boisvert 
---
 builtin/log.c |7 +++
 daemon.c  |6 ++
 progress.c|8 
 upload-pack.c |2 ++
 4 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..f8321c7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -198,7 +198,9 @@ static void show_early_header(struct rev_info *rev, const char *stage, int nr)
 	printf(_("Final output: %d %s\n"), nr, stage);
 }
 
+#ifndef NO_SETITIMER
 static struct itimerval early_output_timer;
+#endif
 
 static void log_show_early(struct rev_info *revs, struct commit_list *list)
 {
@@ -240,9 +242,12 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)
 	 * trigger every second even if we're blocked on a
 	 * reader!
 	 */
+
+	#ifndef NO_SETITIMER
 	early_output_timer.it_value.tv_sec = 0;
 	early_output_timer.it_value.tv_usec = 50;
 	setitimer(ITIMER_REAL, &early_output_timer, NULL);
+	#endif
 }
 
 static void early_output(int signal)
@@ -274,9 +279,11 @@ static void setup_early_output(struct rev_info *rev)
 	 *
 	 * This is a one-time-only trigger.
 	 */
+	#ifndef NO_SETITIMER
 	early_output_timer.it_value.tv_sec = 0;
 	early_output_timer.it_value.tv_usec = 10;
 	setitimer(ITIMER_REAL, &early_output_timer, NULL);
+	#endif
 }
 
 static void finish_early_output(struct rev_info *rev)
diff --git a/daemon.c b/daemon.c
index 4602b46..eb82c19 100644
--- a/daemon.c
+++ b/daemon.c
@@ -611,9 +611,15 @@ static int execute(void)
 	if (addr)
 		loginfo("Connection from %s:%s", addr, port);
 
+	#ifndef NO_SETITIMER
 	alarm(init_timeout ? init_timeout : timeout);
+	#endif
+
 	pktlen = packet_read_line(0, line, sizeof(line));
+
+	#ifndef NO_SETITIMER
 	alarm(0);
+	#endif
 
 	len = strlen(line);
 	if (pktlen != len)
diff --git a/progress.c b/progress.c
index 3971f49..b84ccc7 100644
--- a/progress.c
+++ b/progress.c
@@ -45,7 +45,10 @@ static void progress_interval(int signum)
 static void set_progress_signal(void)
 {
 	struct sigaction sa;
+
+	#ifndef NO_SETITIMER
 	struct itimerval v;
+	#endif
 
 	progress_update = 0;
 
@@ -55,16 +58,21 @@ static void set_progress_signal(void)
 	sa.sa_flags = SA_RESTART;
 	sigaction(SIGALRM, &sa, NULL);
 
+	#ifndef NO_SETITIMER
 	v.it_interval.tv_sec = 1;
 	v.it_interval.tv_usec = 0;
 	v.it_value = v.it_interval;
 	setitimer(ITIMER_REAL, &v, NULL);
+	#endif
 }
 
 static void clear_progress_signal(void)
 {
+	#ifndef NO_SETITIMER
 	struct itimerval v = {{0,},};
 	setitimer(ITIMER_REAL, &v, NULL);
+	#endif
+
 	signal(SIGALRM, SIG_IGN);
 	progress_update = 0;
 }
diff --git a/upload-pack.c b/upload-pack.c
index 95d8313..e0b8b32 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -47,7 +47,9 @@ static int stateless_rpc;
 
 static void

Re: [PATCH v3 1/8] git_remote_helpers: Allow building with Python 3

2013-01-23 Thread Sverre Rabbelier
On Sun, Jan 20, 2013 at 5:15 AM, John Keeping  wrote:
> Change inline Python to call "print" as a function not a statement.
>
> This is harmless because Python 2 will see the parentheses as redundant
> grouping but they are necessary to run this code with Python 3.
>
> Signed-off-by: John Keeping 

Acked-by: Sverre Rabbelier 

--
Cheers,

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


Re: [PATCH v3 3/8] git_remote_helpers: Force rebuild if python version changes

2013-01-23 Thread Sverre Rabbelier
On Sun, Jan 20, 2013 at 5:15 AM, John Keeping  wrote:
> When different version of python are used to build via distutils, the
> behaviour can change.  Detect changes in version and pass --force in
> this case.
>
> Signed-off-by: John Keeping 

Someone else's review on this would be appreciated, the idea sounds
sane but I can't really comment on the implementation.

--
Cheers,

Sverre Rabbelier
--
To unsubscribe from this list: send the line "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] Ignore gitk-wish buildproduct

2013-01-23 Thread Lars Hjemli
After running `make` on latest master, gitk-git/gitk-wish shows up as
untracked. This fixes it.

Signed-off-by: Lars Hjemli 

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index aa258a6..63d4904 100644
--- a/.gitignore
+++ b/.gitignore
@@ -171,6 +171,7 @@
 /git-whatchanged
 /git-write-tree
 /git-core-*/?*
+/gitk-git/gitk-wish
 /gitweb/GITWEB-BUILD-OPTIONS
 /gitweb/gitweb.cgi
 /gitweb/static/gitweb.js
-- 
1.8.1.1.296.g725455c

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


Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

2013-01-23 Thread Sverre Rabbelier
On Sun, Jan 20, 2013 at 5:15 AM, John Keeping  wrote:
> Although 2to3 will fix most issues in Python 2 code to make it run under
> Python 3, it does not handle the new strict separation between byte
> strings and unicode strings.  There is one instance in
> git_remote_helpers where we are caught by this, which is when reading
> refs from "git for-each-ref".
>
> Fix this by operating on the returned string as a byte string rather
> than a unicode string.  As this method is currently only used internally
> by the class this does not affect code anywhere else.
>
> Note that we cannot use byte strings in the source as the 'b' prefix is
> not supported before Python 2.7 so in order to maintain compatibility
> with the maximum range of Python versions we use an explicit call to
> encode().

The three patches that deal with .encode() stuff (2, 7, 8) make me a
bit uncomfortable, as they add some significant complexity to our
python code. Is this the recommended way to deal with this (similar to
the other patch where you linked to the python wiki explaining)?

As one datapoint, it seems that it's actually Python 2.6 that
introduces the b prefix.

http://www.python.org/dev/peps/pep-3112/

When did we last revisit what minimal python version we are ok with requiring?

--
Cheers,

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


Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

2013-01-23 Thread John Keeping
On Wed, Jan 23, 2013 at 11:20:39AM -0800, Sverre Rabbelier wrote:
> On Sun, Jan 20, 2013 at 5:15 AM, John Keeping  wrote:
> > Although 2to3 will fix most issues in Python 2 code to make it run under
> > Python 3, it does not handle the new strict separation between byte
> > strings and unicode strings.  There is one instance in
> > git_remote_helpers where we are caught by this, which is when reading
> > refs from "git for-each-ref".
> >
> > Fix this by operating on the returned string as a byte string rather
> > than a unicode string.  As this method is currently only used internally
> > by the class this does not affect code anywhere else.
> >
> > Note that we cannot use byte strings in the source as the 'b' prefix is
> > not supported before Python 2.7 so in order to maintain compatibility
> > with the maximum range of Python versions we use an explicit call to
> > encode().
> 
> The three patches that deal with .encode() stuff (2, 7, 8) make me a
> bit uncomfortable, as they add some significant complexity to our
> python code. Is this the recommended way to deal with this (similar to
> the other patch where you linked to the python wiki explaining)?

The best I can offer is this:

http://docs.python.org/3/howto/pyporting.html#deal-with-the-bytes-string-dichotomy

Their recommendation is to use the b() function from the six project,
but given that we don't need it in too many places I prefer the approach
I took here to adding a thirdparty dependency.

> As one datapoint, it seems that it's actually Python 2.6 that
> introduces the b prefix.
> 
> http://www.python.org/dev/peps/pep-3112/
> 
> When did we last revisit what minimal python version we are ok with requiring?

I was wondering if people would weigh in discussing that in response to
[1] but no one has commented on that part of it.  As another datapoint,
Brandon Casey was suggesting patching git-p4.py to support Python 2.4
[2].

[1] http://article.gmane.org/gmane.comp.version-control.git/213920
[2] http://article.gmane.org/gmane.comp.version-control.git/214048


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


RE: Question re. git remote repository

2013-01-23 Thread Lang, David
Thanks Matt and Dave and everyone else for your feedback on this.

Ok, I've done some more reading in the Pro Git manual and I think I have an 
idea of how to get started. Could I run this by you just in case I'm missing 
anything? Currently (pre-git status) what we have is two developers both 
working in Visual Studio, occasionally on the same project (hence the need for 
git). All the VS projects exist on a server and are accessible to both 
developers via a network share. Currently, if one of us needs to work on a 
project we turn around and ask our colleague if he's currently in it...this is 
how we avoid both being in at the same time. We run VS locally on each of our 
PC's and load the VS project into Visual Studio from the network share. Easy 
enough...

So to get this all set up with git, here's what I think I have to do...

1. Download and install git for Windows on the 2 networked developer's PC's and 
the 1 networked server.

2. On the server...
a) Initialize the Visual Studio folder for a particular project as a 
git repository using 'git init'
b) Using the git rep just created (above), create a bare repository on 
the server to act as the remote/master repository using 'git clone --bare'

3. On each of the PC's...
a) Clone the remote repository from the network server using 'git 
clone' (this will automatically create 'origin' as a remote source on the PC's)

Couple of questions...

1. Anyone see any problems/issues with the above?

2. Is it sufficient to use the local protocol for transferring files? Seems 
like the most straightforward.

3. On p.84 of the guide there's a section entitled "Putting the Bare Repository 
on a Server" but since the first two rep's (original and bare) are already on 
the server, this is unnecessary, correct?

4. The original Visual Studio project folder essentially remains untouched, 
correct? The 'git init' and 'git clone' commands just make copies and 
references of whatever data is in the VS project folder, right?

David

> -Original Message-
> From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On
> 
> But ultimately, there shouldn't be a question of "if" you have a 
> master repository but "where" you have the master repository, correct?
> Or in other words, it doesn't seem like you'd want to designate any 
> one developer's local repository as also being the master repository, right?

You have two options:

1.  Central model:  
a. each developer has their own private repository b. each developer uses "git 
commit" to commit changes into their own private repository c. in addition, you 
also have a shared master repository d. each developer uses "git push" to push 
their changes from their private repository to the shared master repository e. 
each developer uses "git pull" to pull other developers' changes from the 
shared master repository

2.  Peer-to-peer model:
a. each developer has their own private repository b. each developer uses "git 
commit" to commit changes into their own private repository c. each developer 
uses "git pull" to pull other developers' changes from other developers' 
private repositories

You can even mix these models.  Say you have a 5 member team, and 2 members are 
working on a feature together.  The 2 people working on the feature may use 
"git pull" to pull changes from each other's private repositories.  Then, when 
the feature is ready, one of them can use "git push" to push the final version 
from their private repository into the team's shared repository.

What you don't want to do is this:

Single repository, multiple developers:  just one repository, and every 
developer uses "git commit" to commit their changes into the same repository.

> My sense is that would defeat the purpose of the DVCS.

Not at all.  The purpose of the DVCS is to allow each developer to have their 
own private repository where they can commit changes, while still allowing 
people to share changes from one repository to another.  That's true whether 
you use the central model or the peer-to-peer model.  

The traditional VCS has just one repository, and everyone has to commit their 
changes into that one central repository.

> We have access to many servers on our
> company's network, some of which we have full rights to, so there's no 
> issue in regards to storage space.

That will work fine.

> I suppose another idea would be to have the master simply reside on 
> one of the two developers local machines, so one of us would have both 
> a local rep and the master rep and the other of us would have just a 
> local rep.

That will also work.  You could even omit the master rep. and just have each 
developer have a local repository.  Each developer could then commit changes to 
their own local repository, and pull the other developer's changes from the 
other developer's local repository (the peer-to-peer model mentioned above).

> Or is it best to
> always have the master hosted on a machine with no 

Re: [PATCH] Ignore gitk-wish buildproduct

2013-01-23 Thread Junio C Hamano
Lars Hjemli  writes:

> After running `make` on latest master, gitk-git/gitk-wish shows up as
> untracked. This fixes it.
>
> Signed-off-by: Lars Hjemli 

The removal was very much deliberate [*1*]; Christian was going to
send a corresponding updates to gitk maintainer [*2*, *3*] but I
guess we haven't sync'ed up yet.

Paul, I'll resend another copy of [*3*] to you as a follow-up;
please apply, thanks.


[References]

*1* http://thread.gmane.org/gmane.comp.version-control.git/211773
*2* http://thread.gmane.org/gmane.comp.version-control.git/211641/focus=211751
*3* http://thread.gmane.org/gmane.comp.version-control.git/213067

>
> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.gitignore b/.gitignore
> index aa258a6..63d4904 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -171,6 +171,7 @@
>  /git-whatchanged
>  /git-write-tree
>  /git-core-*/?*
> +/gitk-git/gitk-wish
>  /gitweb/GITWEB-BUILD-OPTIONS
>  /gitweb/gitweb.cgi
>  /gitweb/static/gitweb.js
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 0/2] Add git-for-each-repo

2013-01-23 Thread Lars Hjemli
Lars Hjemli (2):
  for-each-repo: new command used for multi-repo operations
  git: rewrite `git -a` to become a git-for-each-repo command

 .gitignore  |   1 +
 Documentation/git-for-each-repo.txt |  62 +++
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/for-each-repo.c | 119 
 git.c   |  37 +++
 t/t6400-for-each-repo.sh|  54 
 7 files changed, 275 insertions(+)
 create mode 100644 Documentation/git-for-each-repo.txt
 create mode 100644 builtin/for-each-repo.c
 create mode 100755 t/t6400-for-each-repo.sh

-- 
1.8.1.1.350.g3346805

--
To unsubscribe from this list: send the line "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 v3 1/2] for-each-repo: new command used for multi-repo operations

2013-01-23 Thread Lars Hjemli
When working with multiple, unrelated (or loosly related) git repos,
there is often a need to locate all repos with uncommitted work and
perform some action on them (say, commit and push). Before this patch,
such tasks would require manually visiting all repositories, running
`git status` within each one and then decide what to do next.

This mundane task can now be automated by e.g. `git for-each-repo --dirty
status`, which will find all git repositories below the current directory
(even nested ones), check if they are dirty (as defined by `git diff --quiet
&& git diff --cached --quiet`), and for each dirty repo print the path to
the repo and then execute `git status` within the repo.

The command also honours the option '--clean' which restricts the set of
repos to those which '--dirty' would skip.

Finally, the command to execute within each repo is optional. If none is
given, git-for-each-repo will just print the path to each repo found.

Signed-off-by: Lars Hjemli 
---
 .gitignore  |   1 +
 Documentation/git-for-each-repo.txt |  62 +++
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/for-each-repo.c | 119 
 git.c   |   1 +
 t/t6400-for-each-repo.sh|  48 +++
 7 files changed, 233 insertions(+)
 create mode 100644 Documentation/git-for-each-repo.txt
 create mode 100644 builtin/for-each-repo.c
 create mode 100755 t/t6400-for-each-repo.sh

diff --git a/.gitignore b/.gitignore
index 63d4904..5036b84 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,6 +56,7 @@
 /git-filter-branch
 /git-fmt-merge-msg
 /git-for-each-ref
+/git-for-each-repo
 /git-format-patch
 /git-fsck
 /git-fsck-objects
diff --git a/Documentation/git-for-each-repo.txt 
b/Documentation/git-for-each-repo.txt
new file mode 100644
index 000..be49e96
--- /dev/null
+++ b/Documentation/git-for-each-repo.txt
@@ -0,0 +1,62 @@
+git-for-each-repo(1)
+
+
+NAME
+
+git-for-each-repo - Execute a git command in multiple repositories
+
+SYNOPSIS
+
+[verse]
+'git for-each-repo' [--all|--clean|--dirty] [command]
+
+DESCRIPTION
+---
+The git-for-each-repo command is used to locate all git repositoris
+within the current directory tree, and optionally execute a git command
+in each of the found repos.
+
+OPTIONS
+---
+-a::
+--all::
+   Include both clean and dirty repositories (this is the default
+   behaviour of `git-for-each-repo`).
+
+-c::
+--clean::
+   Only include repositories with a clean worktree.
+
+-d::
+--dirty::
+   Only include repositories with a dirty worktree.
+
+EXAMPLES
+
+
+Various ways to exploit this command::
++
+
+$ git for-each-repo<1>
+$ git for-each-repo fetch  <2>
+$ git for-each-repo -d gui <3>
+$ git for-each-repo -c push<4>
+
++
+<1> Print the path to all repos found below the current directory.
+
+<2> Fetch updates from default remote in all repos.
+
+<3> Start linkgit:git-gui[1] in each repo containing uncommitted changes.
+
+<4> Push the current branch in each repo with no uncommited changes.
+
+NOTES
+-
+
+For the purpose of `git-for-each-repo`, a dirty worktree is defined as a
+worktree with uncommitted changes.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index a786d4c..8c42c17 100644
--- a/Makefile
+++ b/Makefile
@@ -870,6 +870,7 @@ BUILTIN_OBJS += builtin/fetch-pack.o
 BUILTIN_OBJS += builtin/fetch.o
 BUILTIN_OBJS += builtin/fmt-merge-msg.o
 BUILTIN_OBJS += builtin/for-each-ref.o
+BUILTIN_OBJS += builtin/for-each-repo.o
 BUILTIN_OBJS += builtin/fsck.o
 BUILTIN_OBJS += builtin/gc.o
 BUILTIN_OBJS += builtin/grep.o
diff --git a/builtin.h b/builtin.h
index 7e7bbd6..02fc712 100644
--- a/builtin.h
+++ b/builtin.h
@@ -73,6 +73,7 @@ extern int cmd_fetch(int argc, const char **argv, const char 
*prefix);
 extern int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
 extern int cmd_for_each_ref(int argc, const char **argv, const char *prefix);
+extern int cmd_for_each_repo(int argc, const char **argv, const char *prefix);
 extern int cmd_format_patch(int argc, const char **argv, const char *prefix);
 extern int cmd_fsck(int argc, const char **argv, const char *prefix);
 extern int cmd_gc(int argc, const char **argv, const char *prefix);
diff --git a/builtin/for-each-repo.c b/builtin/for-each-repo.c
new file mode 100644
index 000..9bdeb4a
--- /dev/null
+++ b/builtin/for-each-repo.c
@@ -0,0 +1,119 @@
+/*
+ * "git for-each-repo" builtin command.
+ *
+ * Copyright (c) 2013 Lars Hjemli 
+ */
+#include "cache.h"
+#include "color.h"
+#include "builtin.h"
+#include "run-command.h"
+#include "parse-options.h"
+
+#define ALL 0
+#define DIRTY 1
+#define CLEAN 2
+
+static int match;
+
+static const char * con

[PATCH v3 2/2] git: rewrite `git -a` to become a git-for-each-repo command

2013-01-23 Thread Lars Hjemli
With this rewriting, it is now possible to run e.g. `git -ad gui` to
start up git-gui in each repo within the current directory which
contains uncommited work.

Signed-off-by: Lars Hjemli 
---
 git.c| 36 
 t/t6400-for-each-repo.sh |  6 ++
 2 files changed, 42 insertions(+)

diff --git a/git.c b/git.c
index 6b53169..f933b5d 100644
--- a/git.c
+++ b/git.c
@@ -31,8 +31,42 @@ static void commit_pager_choice(void) {
}
 }
 
+/*
+ * Rewrite 'git -ad status' to 'git for-each-repo -d status'
+ */
+static int rewrite_foreach_repo(const char ***orig_argv,
+   const char **curr_argv,
+   int *curr_argc)
+{
+   const char **new_argv;
+   char *tmp;
+   int new_argc, curr_pos, i, j;
+
+   curr_pos = curr_argv - *orig_argv;
+   if (strlen(curr_argv[0]) == 2) {
+   curr_argv[0] = "for-each-repo";
+   return curr_pos - 1;
+   }
+
+   new_argc = curr_pos + *curr_argc + 1;
+   new_argv = xmalloc(new_argc * sizeof(void *));
+   for (i = j = 0; j < new_argc; i++, j++) {
+   if (i == curr_pos) {
+   asprintf(&tmp, "-%s", (*orig_argv)[i] + 2);
+   new_argv[j] = "for-each-repo";
+   new_argv[++j] = tmp;
+   } else {
+   new_argv[j] = (*orig_argv)[i];
+   }
+   }
+   *orig_argv = new_argv;
+   (*curr_argc)++;
+   return curr_pos;
+}
+
 static int handle_options(const char ***argv, int *argc, int *envchanged)
 {
+   const char ***pargv = argv;
const char **orig_argv = *argv;
 
while (*argc > 0) {
@@ -143,6 +177,8 @@ static int handle_options(const char ***argv, int *argc, 
int *envchanged)
setenv(GIT_LITERAL_PATHSPECS_ENVIRONMENT, "0", 1);
if (envchanged)
*envchanged = 1;
+   } else if (!strncmp(cmd, "-a", 2)) {
+   return rewrite_foreach_repo(pargv, *argv, argc);
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
diff --git a/t/t6400-for-each-repo.sh b/t/t6400-for-each-repo.sh
index 4797629..b501605 100755
--- a/t/t6400-for-each-repo.sh
+++ b/t/t6400-for-each-repo.sh
@@ -27,6 +27,8 @@ test_expect_success "without flags, all repos are included" '
echo "dirty-idx" >>expect &&
echo "dirty-wt" >>expect &&
git for-each-repo | sort >actual &&
+   test_cmp expect actual &&
+   git -a | sort >actual &&
test_cmp expect actual
 '
 
@@ -35,6 +37,8 @@ test_expect_success "--dirty only includes dirty repos" '
echo "dirty-idx" >>expect &&
echo "dirty-wt" >>expect &&
git for-each-repo --dirty | sort >actual &&
+   test_cmp expect actual &&
+   git -ad | sort >actual &&
test_cmp expect actual
 '
 
@@ -42,6 +46,8 @@ test_expect_success "--clean only includes clean repos" '
echo "." >expect &&
echo "clean" >>expect &&
git for-each-repo --clean | sort >actual &&
+   test_cmp expect actual &&
+   git -ac | sort >actual &&
test_cmp expect actual
 '
 
-- 
1.8.1.1.350.g3346805

--
To unsubscribe from this list: send the line "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] Ignore gitk-wish buildproduct

2013-01-23 Thread Junio C Hamano
From: Christian Couder 

gitk, when bound into the git.git project tree, used to live at the
root level, but in 62ba514 (Move gitk to its own subdirectory,
2007-11-17) it was moved to a subdirectory.  The code used to track
changes to TCLTK_PATH (which should cause gitk to be rebuilt to
point at the new interpreter) was left in the main Makefile instead
of being moved to the new Makefile that was created for the gitk
project.

Also add .gitignore file to list build artifacts for the gitk
project.

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

 Paul, this is relative to the tip of your tree, 386befb (gitk:
 Display important heads even when there are many, 2013-01-02).
 Could you consider applying it?

 Also I notice that you have many patches I still do not have
 there, and I'd appreciate a "Go ahead and pull from me!".

 Thanks.

 .gitignore |  2 ++
 Makefile   | 16 ++--
 2 files changed, 16 insertions(+), 2 deletions(-)
 create mode 100644 .gitignore

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..d7ebcaf
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,2 @@
+/GIT-TCLTK-VARS
+/gitk-wish
diff --git a/Makefile b/Makefile
index e1b6045..5acdc90 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,16 @@ DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 bindir_SQ = $(subst ','\'',$(bindir))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 
+### Detect Tck/Tk interpreter path changes
+TRACK_TCLTK = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
+
+GIT-TCLTK-VARS: FORCE
+   @VARS='$(TRACK_TCLTK)'; \
+   if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
+   echo 1>&2 "* new Tcl/Tk interpreter location"; \
+   echo "$$VARS" >$@; \
+   fi
+
 ## po-file creation rules
 XGETTEXT   ?= xgettext
 ifdef NO_MSGFMT
@@ -49,9 +59,9 @@ uninstall::
$(RM) '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
 
 clean::
-   $(RM) gitk-wish po/*.msg
+   $(RM) gitk-wish po/*.msg GIT-TCLTK-VARS
 
-gitk-wish: gitk
+gitk-wish: gitk GIT-TCLTK-VARS
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) 
"$$0"|' $@+ && \
chmod +x $@+ && \
@@ -65,3 +75,5 @@ $(ALL_MSGFILES): %.msg : %.po
@echo Generating catalog $@
$(MSGFMT) --statistics --tcl $< -l $(basename $(notdir $<)) -d $(dir $@)
 
+.PHONY: all install uninstall clean update-po
+.PHONY: FORCE
-- 
1.8.1.336.g866ceff

--
To unsubscribe from this list: send the line "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: segmentation fault (nullpointer) with git log --submodule -p

2013-01-23 Thread Jeff King
On Wed, Jan 23, 2013 at 03:38:16PM +0100, Armin wrote:

> Hello dear git people.
> 
> I experience a reproducible segmentation fault on one of my
> repositories when doing a "git log --submodule -p", tested with newest
> version on Arch Linux (git version 1.8.1.1) and built fresh (git
> version 1.8.1.1.347.g9591fcc), tried on 2 seperate systems:
> 
> 
> Program terminated with signal 11, Segmentation fault.
> #0  0x004b51e5 in parse_commit_header (context=0x769b6980) at 
> pretty.c:752
> 752 for (i = 0; msg[i]; i++) {
> [...]
> (gdb) l
> 747 static void parse_commit_header(struct format_commit_context *context)
> 748 {
> 749 const char *msg = context->message;
> 750 int i;
> 751 
> 752 for (i = 0; msg[i]; i++) {
> 753 int eol;
> 754 for (eol = i; msg[eol] && msg[eol] != '\n'; eol++)
> 755 ; /* do nothing */
> 756 
> (gdb) p msg
> $7 = 
> (gdb) p context->message
> $8 = 0x0

Yeah, that should definitely not be NULL. I can't reproduce here with a
few simple examples, though.

Does it fail with older versions of git? If so, can you bisect?

Is it possible for you to make your repo available?

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


Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

2013-01-23 Thread Sverre Rabbelier
On Wed, Jan 23, 2013 at 11:47 AM, John Keeping  wrote:
>> When did we last revisit what minimal python version we are ok with 
>> requiring?
>
> I was wondering if people would weigh in discussing that in response to
> [1] but no one has commented on that part of it.  As another datapoint,
> Brandon Casey was suggesting patching git-p4.py to support Python 2.4
> [2].
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/213920
> [2] http://article.gmane.org/gmane.comp.version-control.git/214048

I for one would be happy to kill off support for anything older than
2.6 (which had it's latest release on October 1st, 2008).

Junio, how have we decided in the past which version of x to support?

--
Cheers,

Sverre Rabbelier
--
To unsubscribe from this list: send the line "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] all: new command used for multi-repo operations

2013-01-23 Thread Jens Lehmann
Am 23.01.2013 09:55, schrieb Duy Nguyen:
> On Wed, Jan 23, 2013 at 3:12 PM, Lars Hjemli  wrote:
>> +NAME
>> +
>> +git-all - Execute a git command in multiple repositories
> 
> I agree with Junio "git-all" is too generic.

+1

>> +static int get_repo_state()
>> +{
>> +   const char *diffidx[] = {"diff", "--quiet", "--cached", NULL};
>> +   const char *diffwd[] = {"diff", "--quiet", NULL};
>> +
>> +   if (run_command_v_opt(diffidx, RUN_GIT_CMD) != 0)
>> +   return DIRTY;
>> +   if (run_command_v_opt(diffwd, RUN_GIT_CMD) != 0)
>> +   return DIRTY;
>> +   return CLEAN;
>> +}
> 
> Perhaps we could add the subrepo's object data to the in-memory object
> database of git-all, then do the diff without launching new commands?

You could do that for the "--cached" case, but not for the plain diff.
But I think forking a "git status --porcelain -uno" and testing if it
produced any output should do the trick with a single fork.
--
To unsubscribe from this list: send the line "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: Question re. git remote repository

2013-01-23 Thread Junio C Hamano
"Lang, David"  writes:

> Thanks Matt and Dave and everyone else for your feedback on this.

[administrivia: please wrap your lines to reasonable length]

> 1. Download and install git for Windows on the 2 networked developer's
> PC's and the 1 networked server.
>
> 2. On the server...
>   A) Initialize the Visual Studio folder for a particular
> project as a git repository using 'git init'
>   b) Using the git rep just created (above), create a bare
> repository on the server to act as the remote/master repository using
> git clone --bare'

optionally:

C) remove the original directory (A)

D) make a non-bare clone on the server with "git clone", if
   you would like to have a single build environment on the
   server box.

E) Use "git pull" from the bare repository you created in
   step (2.B) to update the repository you created in step
   (2.D) as necessary in order to build the latest in this
   repository.

> 3. On each of the PC's...
>   A) Clone the remote repository from the network server using
> git clone' (this will automatically create 'origin' as a remote source
> on the PC's)

B) Each developer works in his repository; use either "git
   pull" or "git pull --rebase" to sync up with the tip of
   the master repository as necessary;

C) When a developer's work reaches a point where it is good
   enough to update the master repository, use "git push" to
   update the bare repository you created on the server in
   step (2.B).  This may need to trigger step (2.E).

> Couple of questions...
> ...
> 4. The original Visual Studio project folder essentially remains
> untouched, correct? The 'git init' and 'git clone' commands just make
> copies and references of whatever data is in the VS project folder,
> right?

These operations make copies and after making copies they do not
ever refer to the original, so you can take a back-up of the
original and remove it (i.e. optional step (c)).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3

2013-01-23 Thread Junio C Hamano
Sverre Rabbelier  writes:

> On Wed, Jan 23, 2013 at 11:47 AM, John Keeping  wrote:
>>> When did we last revisit what minimal python version we are ok with 
>>> requiring?
>>
>> I was wondering if people would weigh in discussing that in response to
>> [1] but no one has commented on that part of it.  As another datapoint,
>> Brandon Casey was suggesting patching git-p4.py to support Python 2.4
>> [2].
>>
>> [1] http://article.gmane.org/gmane.comp.version-control.git/213920
>> [2] http://article.gmane.org/gmane.comp.version-control.git/214048
>
> I for one would be happy to kill off support for anything older than
> 2.6 (which had it's latest release on October 1st, 2008).
>
> Junio, how have we decided in the past which version of x to support?

I do not think there was any conclusion.  $gmane/212215 claiming 2.4
support matters for RHEL 5.x users was the last on the topic as far
as I can tell, so it boils down to another question: do users on
RHEL 5.x matter?

I can read from $gmane/212215 that users of the said platform can
safely keep using Python 2.4 under their vendor support contract
until 2017.  But let's focus on what do these users expect of their
system and software they run on it a bit.

When they want to run a piece software that is not shipped with
RHEL, either by writing their own or by importing from elsewhere,
that needs 2.6 features, what are their options?

 (a) The platform vendor optionally supplies 2.6 with or without
 support;

 (b) The users can and do install 2.6 as /usr/local/bin/python2.6,
 which may even be community-supported, but the vendor does not
 support it; or

 (c) The vendor terminates the support contract for users who choose
 to go (b).

I think we can safely discard (c); if that is the case, the users on
the said platform will not choose to update Git either, so it does
not matter where the future versions of Git sets the lower bound of
Python version at.

If we are not talking about the situation (c), then the users can
choose to use 2.6, and more importantly, Python being a popular
software, I would imagine that there are reputable sources of
prepackaged RPMs for them to do so without going too much hassle of
configuring, compiling and installing.

Now how does the decision we make today for releases of Git that
haven't yet happened will affect these users?  As these versions of
newer Git were not shipped with RHEL 5.x, and also I am assuming
that Git is a more niche product than Python is, I would imagine
that it is very unlikely that the vendor gives it the users as an
optional package.  The users will have to do the same thing to be
able to use such versions of Git as whatever they do in order to use
Python 2.6.

Given that, what the vendor originally shipped and officially
supports does not affect the choices we would make today for newer
versions of Git.  The users in a shop where additional third-party
software in /usr/local/bin is strictly forbidden, they are stuck
with the version of Git that the vendor shipped anyway, because they
won't be able to install an updated Git in /usr/local/bin, either.

That is, unless installing 2.6 as /usr/local/bin/python2.6 (or if
you are really paranoid, /usr/local/only-for-git/bin/python2.6 where
nobody's $PATH points at) is impossible.

So personally I do not think dropping 2.4 is a huge problem for
future versions of Git, but I'd like to hear from those working in
IT support for large and slow-moving organizations (aka RHEL 5
customers).





--
To unsubscribe from this list: send the line "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] Revoke write access to refs and odb after importing another repo's odb

2013-01-23 Thread Jens Lehmann
Am 23.01.2013 18:01, schrieb Junio C Hamano:
> Nguyễn Thái Ngọc Duy   writes:
> 
>> add_submodule_odb() can be used to import objects from another
>> repository temporarily. After this point we don't know which objects
>> are ours, which are external. If we create an object that refers to an
>> external object, next time git runs, it may find a hole in the object
>> graph because the external repository may not be imported. The same
>> goes for pointing a ref to an external SHA-1.
>>
>> To protect ourselves, once add_submodule_odb() is used:
>>
>>  - trees, tags and commits cannot be created
>>  - refs cannot be updated
>>
>> In certain cases that submodule code knows that it's safe to write, it
>> can turn the readonly flag off.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>>  I think this is a good safety check.
> 
> Two step implementation to bring "read-only" support into a testable
> shape and then flip that bit in add_submdule_odb() would be a
> sensible approach.

I agree this is a worthwhile change so nobody accidentally screws
things up.

>>  It catches at least a case in
>>  t7405.3. I did not investigate further though.

This is a false positive. The merge algorithm picked a fast-forward
in a submodule as a proper merge result and records that in a
gitlink. But as Duy pointed out this could be easily fixed by
turning the readonly flag off in that case.

> I however have this suspicion that this will become a losing battle
> and we would be better off getting rid of add_submodule_odb();
> instead operations that work across repositories will be done as a
> subprocess, which will get us back closer to one of the original
> design goals of submodule support to have a clear separation between
> the superproject and its submodules.

Please don't. While I agree with your goal, I'd be unhappy to do
that because of the performance drop (especially on fork-challenged
operating systems).
--
To unsubscribe from this list: send the line "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: [PATCHv2 5/8] submodule: use parse_config_key when parsing config

2013-01-23 Thread Jens Lehmann
Am 23.01.2013 07:25, schrieb Jeff King:
> This makes the code a lot simpler to read by dropping a
> whole bunch of constant offsets.
> 
> As a bonus, it means we also feed the whole config variable
> name to our error functions:
> 
>   [before]
>   $ git -c submodule.foo.fetchrecursesubmodules=bogus checkout
>   fatal: bad foo.fetchrecursesubmodules argument: bogus
> 
>   [after]
>   $ git -c submodule.foo.fetchrecursesubmodules=bogus checkout
>   fatal: bad submodule.foo.fetchrecursesubmodules argument: bogus

Thanks, that makes lots of sense!

Acked-by: Jens Lehmann 

> Signed-off-by: Jeff King 
> ---
>  submodule.c | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 2f55436..25413de 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -126,15 +126,16 @@ int parse_submodule_config_option(const char *var, 
> const char *value)
>  
>  int parse_submodule_config_option(const char *var, const char *value)
>  {
> - int len;
>   struct string_list_item *config;
>   struct strbuf submodname = STRBUF_INIT;
> + const char *name, *key;
> + int namelen;
>  
> - var += 10;  /* Skip "submodule." */
> + if (parse_config_key(var, "submodule", &name, &namelen, &key) < 0 || 
> !name)
> + return 0;
>  
> - len = strlen(var);
> - if ((len > 5) && !strcmp(var + len - 5, ".path")) {
> - strbuf_add(&submodname, var, len - 5);
> + if (!strcmp(key, "path")) {
> + strbuf_add(&submodname, name, namelen);
>   config = unsorted_string_list_lookup(&config_name_for_path, 
> value);
>   if (config)
>   free(config->util);
> @@ -142,22 +143,22 @@ int parse_submodule_config_option(const char *var, 
> const char *value)
>   config = string_list_append(&config_name_for_path, 
> xstrdup(value));
>   config->util = strbuf_detach(&submodname, NULL);
>   strbuf_release(&submodname);
> - } else if ((len > 23) && !strcmp(var + len - 23, 
> ".fetchrecursesubmodules")) {
> - strbuf_add(&submodname, var, len - 23);
> + } else if (!strcmp(key, "fetchrecursesubmodules")) {
> + strbuf_add(&submodname, name, namelen);
>   config = 
> unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, 
> submodname.buf);
>   if (!config)
>   config = 
> string_list_append(&config_fetch_recurse_submodules_for_name,
>   strbuf_detach(&submodname, 
> NULL));
>   config->util = (void 
> *)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);
>   strbuf_release(&submodname);
> - } else if ((len > 7) && !strcmp(var + len - 7, ".ignore")) {
> + } else if (!strcmp(key, "ignore")) {
>   if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
>   strcmp(value, "all") && strcmp(value, "none")) {
>   warning("Invalid parameter \"%s\" for config option 
> \"submodule.%s.ignore\"", value, var);
>   return 0;
>   }
>  
> - strbuf_add(&submodname, var, len - 7);
> + strbuf_add(&submodname, name, namelen);
>   config = unsorted_string_list_lookup(&config_ignore_for_name, 
> submodname.buf);
>   if (config)
>   free(config->util);
> 

--
To unsubscribe from this list: send the line "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: [PATCHv2 6/8] submodule: simplify memory handling in config parsing

2013-01-23 Thread Jens Lehmann
Am 23.01.2013 07:26, schrieb Jeff King:
> We keep a strbuf for the name of the submodule, even though
> we only ever add one string to it. Let's just use xmemdupz
> instead, which is slightly more efficient and makes it
> easier to follow what is going on.
> 
> Unfortunately, we still end up having to deal with some
> memory ownership issues in some code branches, as we have to
> allocate the string in order to do a string list lookup, and
> then only sometimes want to hand ownership of that string
> over to the string_list. Still, making that explicit in the
> code (as opposed to sometimes detaching the strbuf, and then
> always releasing it) makes it a little more obvious what is
> going on.

Thanks, this helps until I some day find the time to refactor
that code into a more digestible shape ;-)

Acked-by: Jens Lehmann 

> Signed-off-by: Jeff King 
> ---
>  submodule.c | 30 ++
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/submodule.c b/submodule.c
> index 25413de..9ba1496 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -127,7 +127,6 @@ int parse_submodule_config_option(const char *var, const 
> char *value)
>  int parse_submodule_config_option(const char *var, const char *value)
>  {
>   struct string_list_item *config;
> - struct strbuf submodname = STRBUF_INIT;
>   const char *name, *key;
>   int namelen;
>  
> @@ -135,37 +134,36 @@ int parse_submodule_config_option(const char *var, 
> const char *value)
>   return 0;
>  
>   if (!strcmp(key, "path")) {
> - strbuf_add(&submodname, name, namelen);
>   config = unsorted_string_list_lookup(&config_name_for_path, 
> value);
>   if (config)
>   free(config->util);
>   else
>   config = string_list_append(&config_name_for_path, 
> xstrdup(value));
> - config->util = strbuf_detach(&submodname, NULL);
> - strbuf_release(&submodname);
> + config->util = xmemdupz(name, namelen);
>   } else if (!strcmp(key, "fetchrecursesubmodules")) {
> - strbuf_add(&submodname, name, namelen);
> - config = 
> unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, 
> submodname.buf);
> + char *name_cstr = xmemdupz(name, namelen);
> + config = 
> unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, 
> name_cstr);
>   if (!config)
> - config = 
> string_list_append(&config_fetch_recurse_submodules_for_name,
> - strbuf_detach(&submodname, 
> NULL));
> + config = 
> string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);
> + else
> + free(name_cstr);
>   config->util = (void 
> *)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);
> - strbuf_release(&submodname);
>   } else if (!strcmp(key, "ignore")) {
> + char *name_cstr;
> +
>   if (strcmp(value, "untracked") && strcmp(value, "dirty") &&
>   strcmp(value, "all") && strcmp(value, "none")) {
>   warning("Invalid parameter \"%s\" for config option 
> \"submodule.%s.ignore\"", value, var);
>   return 0;
>   }
>  
> - strbuf_add(&submodname, name, namelen);
> - config = unsorted_string_list_lookup(&config_ignore_for_name, 
> submodname.buf);
> - if (config)
> + name_cstr = xmemdupz(name, namelen);
> + config = unsorted_string_list_lookup(&config_ignore_for_name, 
> name_cstr);
> + if (config) {
>   free(config->util);
> - else
> - config = string_list_append(&config_ignore_for_name,
> - strbuf_detach(&submodname, 
> NULL));
> - strbuf_release(&submodname);
> + free(name_cstr);
> + } else
> + config = string_list_append(&config_ignore_for_name, 
> name_cstr);
>   config->util = xstrdup(value);
>   return 0;
>   }
> 

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


Re: [PATCH v3 1/2] for-each-repo: new command used for multi-repo operations

2013-01-23 Thread Junio C Hamano
Lars Hjemli  writes:

> diff --git a/Documentation/git-for-each-repo.txt 
> b/Documentation/git-for-each-repo.txt
> new file mode 100644
> index 000..be49e96
> --- /dev/null
> +++ b/Documentation/git-for-each-repo.txt
> @@ -0,0 +1,62 @@
> +git-for-each-repo(1)
> +
> +
> +NAME
> +
> +git-for-each-repo - Execute a git command in multiple repositories

"multiple non-bare repositories", I think.

> +
> +SYNOPSIS
> +
> +[verse]
> +'git for-each-repo' [--all|--clean|--dirty] [command]
> +
> +DESCRIPTION
> +---
> +The git-for-each-repo command is used to locate all git repositoris

Likewise; "all non-bare Git repositories".

> diff --git a/t/t6400-for-each-repo.sh b/t/t6400-for-each-repo.sh
> new file mode 100755
> index 000..4797629
> --- /dev/null
> +++ b/t/t6400-for-each-repo.sh
> @@ -0,0 +1,48 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2013 Lars Hjemli
> +#
> +
> +test_description='Test the git-for-each-repo command'
> +
> +. ./test-lib.sh
> +
> +test_expect_success "setup" '
> + test_create_repo clean &&
> + (cd clean && test_commit foo) &&
> + git init --separate-git-dir=.cleansub clean/gitfile &&
> + (cd clean/gitfile && test_commit foo && echo bar >>foo.t) &&
> + test_create_repo dirty-wt &&
> + (cd dirty-wt && mv .git .linkedgit && ln -s .linkedgit .git &&
> +   test_commit foo && rm foo.t) &&
> + test_create_repo dirty-idx &&
> + (cd dirty-idx && test_commit foo && git rm foo.t) &&
> + mkdir fakedir && mkdir fakedir/.git
> +'
> +
> +test_expect_success "without flags, all repos are included" '
> + echo "." >expect &&
> + echo "clean" >>expect &&
> + echo "clean/gitfile" >>expect &&
> + echo "dirty-idx" >>expect &&
> + echo "dirty-wt" >>expect &&
> + git for-each-repo | sort >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success "--dirty only includes dirty repos" '
> + echo "clean/gitfile" >expect &&
> + echo "dirty-idx" >>expect &&
> + echo "dirty-wt" >>expect &&
> + git for-each-repo --dirty | sort >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success "--clean only includes clean repos" '
> + echo "." >expect &&
> + echo "clean" >>expect &&
> + git for-each-repo --clean | sort >actual &&
> + test_cmp expect actual
> +'

Please add tests to show some command executions (e.g. test output
from "git ls-files", or something).

> +static void handle_repo(char *path, const char **argv)
> +{
> + if (path[0] == '.' && path[1] == '/')
> + path += 2;
> + if (match != ALL && match != get_repo_state())
> + return;
> + if (*argv) {
> + color_fprintf_ln(stdout, GIT_COLOR_YELLOW, "[%s]", path);
> + run_command_v_opt(argv, RUN_GIT_CMD);

This seems to allow people to run only a single Git subcommand,
which is probably not what most people want to see.  Don't we want
to support something as simple as this?

git for-each-repository sh -c "ls *.c"

> + } else
> + printf("%s\n", path);

Assuming that the non *argv case is for consumption by programs and
scripts (similar to the way "ls-files" output is piped to downstream),
we prefer to (1) support "-z" so that "xargs -0" can read paths with
funny characters, and (2) use quote_c_style() from quote.c when "-z"
is not in effect.

> +}
> + ...
> + setenv(GIT_DIR_ENVIRONMENT, gitdir, 1);
> + strbuf_setlen(path, len - 1);
> + setenv(GIT_WORK_TREE_ENVIRONMENT, path->buf, 1);
> + handle_repo(path->buf, argv);

When you are only showing the path to a repository, I do not think
you want setenv() or chdir() at all. Shouldn't these be done inside
handle_repo() function?  As you are only dealing with non-bare
repositories (and that is what you print in "listing only" mode
anyway), handle_repo() can borrow path (not path->buf) and append
and strip "/.git" as needed.

Also, while it is a good idea to protect this program from stray
GIT_DIR/GIT_WORK_TREE the user may have in the environment when this
program is started, I think this is not enough, if you allow the
*argv commands to run worktree related operations in each repository
you discover.  You would need to chdir() to the top of the working
tree.

The run-command API lets you specify custom environment only for the
child process without affecting yourself by setting .env member of
the child_process structure, so we may want to use that instead of
doing setenv() on ourselves (and letting it inherited by the child).

--
To unsubscribe from this list: send the line "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] Revoke write access to refs and odb after importing another repo's odb

2013-01-23 Thread Junio C Hamano
Jens Lehmann  writes:

> This is a false positive. The merge algorithm picked a fast-forward
> in a submodule as a proper merge result and records that in a
> gitlink. But as Duy pointed out this could be easily fixed by
> turning the readonly flag off in that case.

I see that as "easily circumvented and not an effective protection",
though.

In theory, adding a gitlink to the index, removing a gitlink to the
index and modifying an existing gitlink in the index to another
gitlink in the index and writing the resulting in-core index out to
the on-disk index should be allowed, even after objects from the
submodule object database have contaminated our in-core object pool,
as long as you do not run cache_tree_update().  I am not sure if that
single loophole would be sufficient, though.

--
To unsubscribe from this list: send the line "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: What's cooking in git.git (Jan 2013, #08; Tue, 22)

2013-01-23 Thread John Keeping
On Wed, Jan 23, 2013 at 09:13:27AM -0800, Junio C Hamano wrote:
> John Keeping  writes:
> 
> > My preference would be for something like this, possibly with an
> > expanded examples section showing how to pipe the output of cvsps-3 or
> > cvs2git into git-fast-import:
> >
> > -- >8 --
> >
> > diff --git a/Documentation/git-cvsimport.txt 
> > b/Documentation/git-cvsimport.txt
> > index 9d5353e..20b846e 100644
> > --- a/Documentation/git-cvsimport.txt
> > +++ b/Documentation/git-cvsimport.txt
> > @@ -18,6 +18,11 @@ SYNOPSIS
> >  
> >  DESCRIPTION
> >  ---
> > +*WARNING:* `git cvsimport` uses cvsps version 2, which is considered
> > +deprecated; it does not work with cvsps version 3 and later.  If you are
> > +performing a one-shot import of a CVS repository consider using cvsps-3,
> > +cvs2git or parsecvs directly.
> > +
> >  Imports a CVS repository into git. It will either create a new
> >  repository, or incrementally import into an existing one.
> >  
> > -- 8< --
> 
> OK, that is certainly a lot simpler to explain.
> 
> Is it "it does not work yet with cvsps3", or "it will not ever work
> with cvsps3"?  The impression I am getting is that it is the latter.

The existing script (git-cvsimport.perl) won't ever work with cvsps-3
since features it relies on have been removed.

> Also, should we have a suggestion to people who are *not* performing
> a one-shot import, i.e. doing incremental or bidirectional?

As far as I know cvsps is the only backend that attempts to support
partial exports but the support for that in its fast-export mode needs
work before I would consider it reliable.  For now the existing
git-cvsimport is the best option I'm aware of.


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


[PATCH] parse_object: clear "parsed" when freeing buffers

2013-01-23 Thread Jonathon Mah
Add a new function "free_object_buffer", which marks the object as
un-parsed and frees the buffer. Only trees and commits have buffers;
other types are not affected. If the tree or commit buffer is already
NULL, the "parsed" flag is still cleared so callers can control the free
themselves (index-pack.c uses this).

Several areas of code would free buffers for object structs that
contained them ("struct tree" and "struct commit"), but without clearing
the "parsed" flag. parse_object would clear the flag for "struct tree",
but commits would remain in an invalid state (marked as parsed but with
a NULL buffer). Because the objects are uniqued (ccdc6037fee), the
invalid objects stay around and can lead to bad behavior.

In particular, running pickaxe on all refs in a repository with a cached
textconv could segfault. If the textconv cache ref was evaluated first
by cmd_log_walk, a subsequent notes_cache_match_validity call would
dereference NULL.

Signed-off-by: Jonathon Mah 
---

I found an old email where Jeff noted that this would be bad (yet the buffer 
manipulation remained).


 builtin/fsck.c   | 17 ++---
 builtin/index-pack.c |  3 +++
 builtin/log.c|  9 +++--
 builtin/reflog.c |  3 +--
 builtin/rev-list.c   |  3 +--
 http-push.c  |  3 +--
 list-objects.c   |  3 +--
 object.c | 25 +++--
 object.h |  3 +++
 reachable.c  |  3 +--
 revision.c   |  3 +--
 t/t4042-diff-textconv-caching.sh | 11 +++
 upload-pack.c|  3 +--
 walker.c |  5 +
 14 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index bb9a2cd..82b3612 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -133,10 +133,7 @@ static int traverse_one_object(struct object *obj)
return 1; /* error already displayed */
}
result = fsck_walk(obj, mark_object, obj);
-   if (tree) {
-   free(tree->buffer);
-   tree->buffer = NULL;
-   }
+   free_object_buffer(obj);
return result;
 }
 
@@ -303,26 +300,16 @@ static int fsck_obj(struct object *obj)
if (fsck_object(obj, check_strict, fsck_error_func))
return -1;
 
-   if (obj->type == OBJ_TREE) {
-   struct tree *item = (struct tree *) obj;
-
-   free(item->buffer);
-   item->buffer = NULL;
-   }
+   free_object_buffer(obj);
 
if (obj->type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
-
-   free(commit->buffer);
-   commit->buffer = NULL;
-
if (!commit->parents && show_root)
printf("root %s\n", sha1_to_hex(commit->object.sha1));
}
 
if (obj->type == OBJ_TAG) {
struct tag *tag = (struct tag *) obj;
-
if (show_tags && tag->tagged) {
printf("tagged %s %s", typename(tag->tagged->type), 
sha1_to_hex(tag->tagged->sha1));
printf(" (%s) in %s\n", tag->tag, 
sha1_to_hex(tag->object.sha1));
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 43d364b..0eb39ae 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -750,13 +750,16 @@ static void sha1_object(const void *data, struct 
object_entry *obj_entry,
if (fsck_walk(obj, mark_link, NULL))
die(_("Not all child objects of %s are 
reachable"), sha1_to_hex(obj->sha1));
 
+   /* set buffer to NULL so it isn't freed */
if (obj->type == OBJ_TREE) {
struct tree *item = (struct tree *) obj;
item->buffer = NULL;
+   free_object_buffer(obj);
}
if (obj->type == OBJ_COMMIT) {
struct commit *commit = (struct commit *) obj;
commit->buffer = NULL;
+   free_object_buffer(obj);
}
obj->flags |= FLAG_CHECKED;
}
diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..433b874 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -305,11 +305,9 @@ static int cmd_log_walk(struct rev_info *rev)
 * but we didn't actually show the commit.
 */
rev->max_count++;
-   if (!rev->reflog_info) {
+   if (!rev->reflog_info)
/* we allow cycles in reflog ancestry */
-   free(commit->buffer);
-   commit-

Re: Bug in EOL conversion?

2013-01-23 Thread Philip Oakley
The msysgit list msys...@googlegroups.com may be a better place for 
this.


It is likely that you have a windows specific EOL conversion set within 
the wider config's (i.e.  --system, --global). You may have 
core.safecrlf set which does a round trip test so tests the conversion 
both ways.


The normal canonical line ending choice is LF in the repo.

I don't have a W7 install to compare against.

Philip

- Original Message - 
From: "Stefan Norgren" 

To: 
Sent: Wednesday, January 23, 2013 2:44 AM
Subject: Bug in EOL conversion?



Hi,

The EOL conversion does not behave as indicated by output message from
add and commit. Here is my test case executed on Windows 7 64 bit.


$ git --version
git version 1.8.0.msysgit.0
$ which git
/cygdrive/c/Program Files (x86)/Git/cmd/git
$ git config --list
core.symlinks=false
core.autocrlf=true
color.diff=auto
color.status=auto
color.branch=auto
color.interactive=true
pack.packsizelimit=2g
help.format=html
http.sslcainfo=/bin/curl-ca-bundle.crt
sendemail.smtpserver=/bin/msmtp.exe
diff.astextplain.textconv=astextplain
rebase.autosquash=true
user.name=Stefan
user.email=ste...@---.com
core.repositoryformatversion=0
core.filemode=false
core.bare=false
core.logallrefupdates=true
core.symlinks=false
core.ignorecase=true
core.hidedotfiles=dotGitOnly

-- Note core.autocrlf=true.
-- Created withcrlf.txt with one character and one CRLF line feed.
File size 3 bytes.
-- Created withlf.txt with one character and one LF line feed. File
size 2 bytes.
-- Now let's init repository.

$ git init
Initialized empty Git repository in D:/Dev/workspaces/gittest/.git/
$ ls -la
total 10
d-+ 1 Stefan None 0 Jan 23 02:12 .
d-+ 1 Stefan None 0 Jan 23 02:10 ..
d-+ 1 Stefan None 0 Jan 23 02:13 .git
--+ 1 Stefan None 3 Jan 23 01:55 withcrlf.txt
--+ 1 Stefan None 2 Jan 23 01:55 withlf.txt

-- Note no .gitattributes file that will affect EOL conversion.

$ ls -la .git/info/
total 5
d-+ 1 Stefan None   0 Jan 23 02:12 .
d-+ 1 Stefan None   0 Jan 23 02:13 ..
--+ 1 Stefan None 240 Jan 23 02:12 exclude

-- Note no attribute file in .git/info/ that will affect EOL 
conversion.


$ git add *
warning: LF will be replaced by CRLF in withlf.txt.
The file will have its original line endings in your working 
directory.

$ git commit -m 'Testing EOL'
[master (root-commit) 9a0b2f5] Testing EOL
2 files changed, 2 insertions(+)
create mode 100644 withcrlf.txt
create mode 100644 withlf.txt
warning: LF will be replaced by CRLF in withlf.txt.
The file will have its original line endings in your working 
directory.

$ ls -la
total 10
d-+ 1 Stefan None 0 Jan 23 02:12 .
d-+ 1 Stefan None 0 Jan 23 02:10 ..
d-+ 1 Stefan None 0 Jan 23 02:22 .git
--+ 1 Stefan None 3 Jan 23 01:55 withcrlf.txt
--+ 1 Stefan None 2 Jan 23 01:55 withlf.txt

-- So no changes (see file size) to files in working directory. This
is expected and according to output warning from add and commit.

-- Now lets look in the repository

$ git ls-tree -l HEAD withcrlf.txt
100644 blob d00491fd7e5bb6fa28c517a0bb32b8b506539d4d   2 
withcrlf.txt

$ git ls-tree -l HEAD withlf.txt
100644 blob d00491fd7e5bb6fa28c517a0bb32b8b506539d4d   2 
withlf.txt


-- Note that size of withlf.txt is 2 in repository indicating that LF
was not replaced by CRLF in withlf.txt as indicated in output from add
and commit. Also note that size of withcrlf.txt is also 2 in
repository so it looks like CRLF was replaced by LF in withcrlf.txt.
To verify I will delete the files from working directory, turn off EOL
conversion, checkout files and look at file endings in the working
directory.

$ rm with*
$ ls -la
total 8
d-+ 1 Stefan None 0 Jan 23 02:31 .
d-+ 1 Stefan None 0 Jan 23 02:10 ..
d-+ 1 Stefan None 0 Jan 23 02:22 .git
$ git status
# On branch master
# Changes not staged for commit:
#   (use "git add/rm ..." to update what will be committed)
#   (use "git checkout -- ..." to discard changes in working 
directory)

#
#   deleted:withcrlf.txt
#   deleted:withlf.txt
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git config --global core.autocrlf false
$ git config --global core.autocrlf
false
$ git checkout -- with*
$ ls -la
total 10
d-+ 1 Stefan None 0 Jan 23 02:38 .
d-+ 1 Stefan None 0 Jan 23 02:10 ..
d-+ 1 Stefan None 0 Jan 23 02:38 .git
--+ 1 Stefan None 2 Jan 23 02:38 withcrlf.txt
--+ 1 Stefan None 2 Jan 23 02:38 withlf.txt

-- Both files in working directory files now have LF line endings
confirming that files in repository have LF file endings. Either the
output message of add and commit is wrong or the behavior of the EOL
conversion is wrong... or... have I missed something...?

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

[PATCH v4 0/3] Finishing touches to "push" advises

2013-01-23 Thread Junio C Hamano
This builds on Chris Rorvick's earlier effort to forbid unforced
updates to refs/tags/ hierarchy and giving sensible error and advise
messages for that case (we are not rejecting such a push due to fast
forwardness, and suggesting to fetch and integrate before pushing
again does not make sense).

The series applies on top of 256b9d7 (push: fix "refs/tags/
hierarchy cannot be updated without --force", 2013-01-16).

This fourth round swaps the order of clean-up patches and now the
bottom two are clean-up patches.  The main change is in the third
one.

When the object at the tip of the remote is not a committish, or the
object we are pushing is not a committish, the existing code already
rejects such a push on the client end, but we used the same error
and advice messages as the ones used when rejecting a push that does
not fast-forward, i.e. pull from there and integrate before pushing
again.  Introduce a new rejection reason NEEDS_FORCE and explain why
the push was rejected, stressing the fact that --force is required
when non committish objects are involved, so that the user can (1)
notice a possibly mistyped source object name or destination ref
name, when the user is trying to push an ordinary commit, or (2)
learn that "--force" is an appropriate thing to use when the user is
sure that s/he wants to push a non-committish (which is unusual).

Unlike the third round, we do not say "fetch first, inspect the
situation to decide what to do", when we do not have the object
sitting at the tip of the remote.  Most likely, it is a commit
somebody who has been working on the same branch pushed that we
haven't fetched yet, so suggesting to pull is often sufficient and
appropriate, and in a more uncommon case in which the unknown object
is not a committish, the suggested pull will fail without making
permanent damage anywhere.  Next atttempt to push without changing
anything (e.g. "reset --hard") will then trigger the NEEDS_FORCE
"Your push involves non-commit objects" case.


Junio C Hamano (3):
  push: further clean up fields of "struct ref"
  push: further simplify the logic to assign rejection reason
  push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE

 Documentation/config.txt | 12 +++-
 advice.c |  4 
 advice.h |  2 ++
 builtin/push.c   | 29 +
 builtin/send-pack.c  | 10 ++
 cache.h  |  6 +++---
 remote.c | 42 +++---
 send-pack.c  |  2 ++
 transport-helper.c   | 10 ++
 transport.c  | 14 +-
 transport.h  |  2 ++
 11 files changed, 105 insertions(+), 28 deletions(-)

-- 
1.8.1.1.517.g0318d2b

--
To unsubscribe from this list: send the line "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 v4 1/3] push: further clean up fields of "struct ref"

2013-01-23 Thread Junio C Hamano
The "nonfastforward" and "update" fields are only used while
deciding what value to assign to the "status" locally in a single
function.  Remove them from the "struct ref".

The "requires_force" field is not used to decide if the proposed
update requires a --force option to succeed, or to record such a
decision made elsewhere.  It is used by status reporting code that
the particular update was "forced".  Rename it to "forced_udpate",
and move the code to assign to it around to further clarify how it
is used and what it is used for.

Signed-off-by: Junio C Hamano 
---
 cache.h |  4 +---
 remote.c| 16 ++--
 transport.c |  2 +-
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/cache.h b/cache.h
index a942bbd..12631a1 100644
--- a/cache.h
+++ b/cache.h
@@ -1001,10 +1001,8 @@ struct ref {
char *symref;
unsigned int
force:1,
-   requires_force:1,
+   forced_update:1,
merge:1,
-   nonfastforward:1,
-   update:1,
deletion:1;
enum {
REF_STATUS_NONE = 0,
diff --git a/remote.c b/remote.c
index d3a1ca2..3375914 100644
--- a/remote.c
+++ b/remote.c
@@ -1317,27 +1317,23 @@ void set_ref_status_for_push(struct ref *remote_refs, 
int send_mirror,
 * passing the --force argument
 */
 
-   ref->update =
-   !ref->deletion &&
-   !is_null_sha1(ref->old_sha1);
-
-   if (ref->update) {
-   ref->nonfastforward =
+   if (!ref->deletion && !is_null_sha1(ref->old_sha1)) {
+   int nonfastforward =
!has_sha1_file(ref->old_sha1)
- || !ref_newer(ref->new_sha1, ref->old_sha1);
+   || !ref_newer(ref->new_sha1, ref->old_sha1);
 
if (!prefixcmp(ref->name, "refs/tags/")) {
-   ref->requires_force = 1;
if (!force_ref_update) {
ref->status = 
REF_STATUS_REJECT_ALREADY_EXISTS;
continue;
}
-   } else if (ref->nonfastforward) {
-   ref->requires_force = 1;
+   ref->forced_update = 1;
+   } else if (nonfastforward) {
if (!force_ref_update) {
ref->status = 
REF_STATUS_REJECT_NONFASTFORWARD;
continue;
}
+   ref->forced_update = 1;
}
}
}
diff --git a/transport.c b/transport.c
index 2673d27..585ebcd 100644
--- a/transport.c
+++ b/transport.c
@@ -659,7 +659,7 @@ static void print_ok_ref_status(struct ref *ref, int 
porcelain)
const char *msg;
 
strcpy(quickref, status_abbrev(ref->old_sha1));
-   if (ref->requires_force) {
+   if (ref->forced_update) {
strcat(quickref, "...");
type = '+';
msg = "forced update";
-- 
1.8.1.1.517.g0318d2b

--
To unsubscribe from this list: send the line "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 v4 2/3] push: further simplify the logic to assign rejection reason

2013-01-23 Thread Junio C Hamano
First compute the reason why this push would fail if done without
"--force", and then fail it by assigning that reason when the push
was not forced (or if there is no reason to require force, allow it
to succeed).

Record the fact that the push was forced in the forced_update field
only when the push would have failed without the option.

The code becomes shorter, less repetitive and easier to read this
way, especially given that the set of rejection reasons will be
extended in a later patch.

Signed-off-by: Junio C Hamano 
---
 remote.c | 27 +++
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/remote.c b/remote.c
index 3375914..969aa11 100644
--- a/remote.c
+++ b/remote.c
@@ -1318,23 +1318,18 @@ void set_ref_status_for_push(struct ref *remote_refs, 
int send_mirror,
 */
 
if (!ref->deletion && !is_null_sha1(ref->old_sha1)) {
-   int nonfastforward =
-   !has_sha1_file(ref->old_sha1)
-   || !ref_newer(ref->new_sha1, ref->old_sha1);
-
-   if (!prefixcmp(ref->name, "refs/tags/")) {
-   if (!force_ref_update) {
-   ref->status = 
REF_STATUS_REJECT_ALREADY_EXISTS;
-   continue;
-   }
-   ref->forced_update = 1;
-   } else if (nonfastforward) {
-   if (!force_ref_update) {
-   ref->status = 
REF_STATUS_REJECT_NONFASTFORWARD;
-   continue;
-   }
+   int why = 0; /* why would this push require --force? */
+
+   if (!prefixcmp(ref->name, "refs/tags/"))
+   why = REF_STATUS_REJECT_ALREADY_EXISTS;
+   else if (!has_sha1_file(ref->old_sha1)
+|| !ref_newer(ref->new_sha1, ref->old_sha1))
+   why = REF_STATUS_REJECT_NONFASTFORWARD;
+
+   if (!force_ref_update)
+   ref->status = why;
+   else if (why)
ref->forced_update = 1;
-   }
}
}
 }
-- 
1.8.1.1.517.g0318d2b

--
To unsubscribe from this list: send the line "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 v4 3/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE

2013-01-23 Thread Junio C Hamano
When we push to update an existing ref, if:

 * the object at the tip of the remote is not a commit; or
 * the object we are pushing is not a commit,

it won't be correct to suggest to fetch, integrate and push again,
as the old and new objects will not "merge".

If we do not have the current object at the tip of the remote, we do
not even know that object, when fetched, is something that can be
merged.  In such a case, suggesting to pull first just like
non-fast-forward case may not be technically correct, but in
practice, most such failures are seen when you try to push your work
to a branch without knowing that somebody else already pushed to
update the same branch since you forked, so "pull first" would work
as a suggestion most of the time.

In these cases, the current code already rejects such a push on the
client end, but we used the same error and advice messages as the
ones used when rejecting a non-fast-forward push, i.e. pull from
there and integrate before pushing again.  Introduce new
rejection reasons and reword the messages appropriately.

Signed-off-by: Junio C Hamano 
---
 Documentation/config.txt | 12 +++-
 advice.c |  4 
 advice.h |  2 ++
 builtin/push.c   | 29 +
 builtin/send-pack.c  | 10 ++
 cache.h  |  2 ++
 remote.c | 11 ---
 send-pack.c  |  2 ++
 transport-helper.c   | 10 ++
 transport.c  | 12 
 transport.h  |  2 ++
 11 files changed, 92 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 90e7d10..1f47761 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -143,7 +143,8 @@ advice.*::
pushUpdateRejected::
Set this variable to 'false' if you want to disable
'pushNonFFCurrent', 'pushNonFFDefault',
-   'pushNonFFMatching', and 'pushAlreadyExists'
+   'pushNonFFMatching', 'pushAlreadyExists',
+   'pushFetchFirst', and 'pushNeedsForce'
simultaneously.
pushNonFFCurrent::
Advice shown when linkgit:git-push[1] fails due to a
@@ -162,6 +163,15 @@ advice.*::
pushAlreadyExists::
Shown when linkgit:git-push[1] rejects an update that
does not qualify for fast-forwarding (e.g., a tag.)
+   pushFetchFirst::
+   Shown when linkgit:git-push[1] rejects an update that
+   tries to overwrite a remote ref that points at an
+   object we do not have.
+   pushNeedsForce::
+   Shown when linkgit:git-push[1] rejects an update that
+   tries to overwrite a remote ref that points at an
+   object that is not a committish, or make the remote
+   ref point at an object that is not a committish.
statusHints::
Show directions on how to proceed from the current
state in the output of linkgit:git-status[1] and in
diff --git a/advice.c b/advice.c
index d287927..780f58d 100644
--- a/advice.c
+++ b/advice.c
@@ -5,6 +5,8 @@ int advice_push_non_ff_current = 1;
 int advice_push_non_ff_default = 1;
 int advice_push_non_ff_matching = 1;
 int advice_push_already_exists = 1;
+int advice_push_fetch_first = 1;
+int advice_push_needs_force = 1;
 int advice_status_hints = 1;
 int advice_commit_before_merge = 1;
 int advice_resolve_conflict = 1;
@@ -20,6 +22,8 @@ static struct {
{ "pushnonffdefault", &advice_push_non_ff_default },
{ "pushnonffmatching", &advice_push_non_ff_matching },
{ "pushalreadyexists", &advice_push_already_exists },
+   { "pushfetchfirst", &advice_push_fetch_first },
+   { "pushneedsforce", &advice_push_needs_force },
{ "statushints", &advice_status_hints },
{ "commitbeforemerge", &advice_commit_before_merge },
{ "resolveconflict", &advice_resolve_conflict },
diff --git a/advice.h b/advice.h
index 8bf6356..fad36df 100644
--- a/advice.h
+++ b/advice.h
@@ -8,6 +8,8 @@ extern int advice_push_non_ff_current;
 extern int advice_push_non_ff_default;
 extern int advice_push_non_ff_matching;
 extern int advice_push_already_exists;
+extern int advice_push_fetch_first;
+extern int advice_push_needs_force;
 extern int advice_status_hints;
 extern int advice_commit_before_merge;
 extern int advice_resolve_conflict;
diff --git a/builtin/push.c b/builtin/push.c
index 8491e43..92ca3d7 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -220,10 +220,21 @@ static const char message_advice_checkout_pull_push[] =
   "(e.g. 'git pull') before pushing again.\n"
   "See the 'Note about fast-forwards' in 'git push --help' for 
details.");
 
+static const char message_advice_ref_fetch_first[] =
+   N_("Updates were rejected because you do not have the object at the 
tip\n"
+  "of the remote. You may want to fir

Re: [PATCH] parse_object: clear "parsed" when freeing buffers

2013-01-23 Thread Junio C Hamano
Jonathon Mah  writes:

> Add a new function "free_object_buffer", which marks the object as
> un-parsed and frees the buffer. Only trees and commits have buffers;
> other types are not affected. If the tree or commit buffer is already
> NULL, the "parsed" flag is still cleared so callers can control the free
> themselves (index-pack.c uses this).
>
> Several areas of code would free buffers for object structs that
> contained them ("struct tree" and "struct commit"), but without clearing
> the "parsed" flag. parse_object would clear the flag for "struct tree",
> but commits would remain in an invalid state (marked as parsed but with
> a NULL buffer). Because the objects are uniqued (ccdc6037fee), the
> invalid objects stay around and can lead to bad behavior.
>
> In particular, running pickaxe on all refs in a repository with a cached
> textconv could segfault. If the textconv cache ref was evaluated first
> by cmd_log_walk, a subsequent notes_cache_match_validity call would
> dereference NULL.

Conceptually this is a right thing to do, but it is unclear why this
conversion is safe in the existing code.

A codepath that used to free() and assign NULL to a buffer without
resetting .parsed would have assumed that it can find out the parsed
properties of the object (e.g. .parents) without re-parsing the
object, and much more importantly, the modifications made by that
codepath will not be clobbered by later call to parse_object().

IIRC, revision traversal machinery rewrites commit->parents but
discards buffer when it knows that the log message is not needed
(save_commit_buffer controls this behaviour).  I do not offhand know
if there are other instances of existing code that depend on the
current behaviour, but have you audited all the codepaths that are
affected by this patch and codepaths that work on objects this patch
unmarks their .parsed field will not have such a problem?

--
To unsubscribe from this list: send the line "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: [msysGit] Re: Bug in EOL conversion?

2013-01-23 Thread Erik Faye-Lund
On Wed, Jan 23, 2013 at 10:55 PM, Philip Oakley  wrote:
> The msysgit list msys...@googlegroups.com may be a better place for this.
>
> It is likely that you have a windows specific EOL conversion set within the
> wider config's (i.e.  --system, --global). You may have core.safecrlf set
> which does a round trip test so tests the conversion both ways.

The default for core.safecrlf is "warn", so one does not need a
setting to get that warning.
--
To unsubscribe from this list: send the line "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: Bug in EOL conversion?

2013-01-23 Thread Thomas Rast
Stefan Norgren  writes:

> $ git add *
> warning: LF will be replaced by CRLF in withlf.txt.
> The file will have its original line endings in your working directory.
[...]
> $ ls -la
> total 10
> d-+ 1 Stefan None 0 Jan 23 02:12 .
> d-+ 1 Stefan None 0 Jan 23 02:10 ..
> d-+ 1 Stefan None 0 Jan 23 02:22 .git
> --+ 1 Stefan None 3 Jan 23 01:55 withcrlf.txt
> --+ 1 Stefan None 2 Jan 23 01:55 withlf.txt
[...]
> $ git ls-tree -l HEAD withcrlf.txt
> 100644 blob d00491fd7e5bb6fa28c517a0bb32b8b506539d4d   2withcrlf.txt
> $ git ls-tree -l HEAD withlf.txt
> 100644 blob d00491fd7e5bb6fa28c517a0bb32b8b506539d4d   2withlf.txt

Isn't that what would be expected?  It's a combination of

- the canonical representation of a newline is LF, so the repository
  stores LF

- with safecrlf, checkout converts LF->CRLF and add converts CRLF->LF

So from the user's POV, running

  git add withlf.txt
  rm withlf.txt
  git checkout -- withlf.txt

would appear to replace LF with CRLF in the worktree.  That's what the
message says.

-- 
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: [msysGit] Re: Bug in EOL conversion?

2013-01-23 Thread Philip Oakley

From: "Erik Faye-Lund" 
Sent: Wednesday, January 23, 2013 10:36 PM
On Wed, Jan 23, 2013 at 10:55 PM, Philip Oakley  
wrote:
The msysgit list msys...@googlegroups.com may be a better place for 
this.


It is likely that you have a windows specific EOL conversion set 
within the
wider config's (i.e.  --system, --global). You may have core.safecrlf 
set

which does a round trip test so tests the conversion both ways.


The default for core.safecrlf is "warn", so one does not need a
setting to get that warning.



Thank you confirming the Git for Windows default, which I don't believe 
Stefan had realised was active.


I had responded to Stefan's original 'bug' report as no one had picked 
up on it, and suspected it (core.safecrlf ) was set in Git for Windows, 
though wasn't able to immediately check it myself.


I did not think it was a bug at all, merely a misunderstanding by Stefan 
about the safety features within Git (for Windows). 


--
To unsubscribe from this list: send the line "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] parse_object: clear "parsed" when freeing buffers

2013-01-23 Thread Jonathon Mah
[Adding Jeff King to CC; I meant to copy you in the original but forgot, sorry]

On 2013-01-23, at 14:19, Junio C Hamano  wrote:

> Jonathon Mah  writes:
> 
>> Add a new function "free_object_buffer", which marks the object as
>> un-parsed and frees the buffer. Only trees and commits have buffers;
>> other types are not affected. If the tree or commit buffer is already
>> NULL, the "parsed" flag is still cleared so callers can control the free
>> themselves (index-pack.c uses this).
>> 
>> Several areas of code would free buffers for object structs that
>> contained them ("struct tree" and "struct commit"), but without clearing
>> the "parsed" flag. parse_object would clear the flag for "struct tree",
>> but commits would remain in an invalid state (marked as parsed but with
>> a NULL buffer). Because the objects are uniqued (ccdc6037fee), the
>> invalid objects stay around and can lead to bad behavior.
>> 
>> In particular, running pickaxe on all refs in a repository with a cached
>> textconv could segfault. If the textconv cache ref was evaluated first
>> by cmd_log_walk, a subsequent notes_cache_match_validity call would
>> dereference NULL.
> 
> Conceptually this is a right thing to do, but it is unclear why this
> conversion is safe in the existing code.
> 
> A codepath that used to free() and assign NULL to a buffer without
> resetting .parsed would have assumed that it can find out the parsed
> properties of the object (e.g. .parents) without re-parsing the
> object, and much more importantly, the modifications made by that
> codepath will not be clobbered by later call to parse_object().
> 
> IIRC, revision traversal machinery rewrites commit->parents but
> discards buffer when it knows that the log message is not needed
> (save_commit_buffer controls this behaviour).  I do not offhand know
> if there are other instances of existing code that depend on the
> current behaviour, but have you audited all the codepaths that are
> affected by this patch and codepaths that work on objects this patch
> unmarks their .parsed field will not have such a problem?

No, I haven't audited the code paths (I have only the loosest familiarity with 
the source). Indeed, I found that clearing the 'parsed' flag in fsck.c 
(traverse_one_object()) is incorrect and causes test failures.

With the object cache, isn't modifying the object unsafe in general? Instead of 
auditing code paths, it's now necessary to audit _all_ code that uses "struct 
object", which seems infeasible.

Anyway, I don't care about the implementation (Junio does that extremely well), 
I'm just trying to fix the segfault demonstrated in the test attached to the 
patch.



Jonathon Mah
m...@jonathonmah.com


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


Re: [PATCH] parse_object: clear "parsed" when freeing buffers

2013-01-23 Thread Junio C Hamano
Jonathon Mah  writes:

> No, I haven't audited the code paths (I have only the loosest
> familiarity with the source). Indeed, I found that clearing the
> 'parsed' flag in fsck.c (traverse_one_object()) is incorrect and
> causes test failures.
>
> With the object cache, isn't modifying the object unsafe in
> general? Instead of auditing code paths, it's now necessary to
> audit _all_ code that uses "struct object", which seems
> infeasible.

The object layer was designed with "run one thing and one thing
well, and then let the _exit(2) take care of the clean-up" model in
mind; modifying the object, e.g. updating commit->parents list,
in-core by revision traversal machinery is very much within the
scope of its intended uses.

> I'm just trying to fix the segfault demonstrated in the test
> attached to the patch.

Can offending readers that dereference NULL without checking if
buffer has been freed be updated so that they read_sha1_file(), read
the contents from the result returned from the function (instead of
reading from .buffer), and free the memory when they are done?

That would be a fix that would be very much isolated and easy to
audit.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/2] Make git-svn work with gitdir links

2013-01-23 Thread Eric Wong
Barry Wardell  wrote:
> On Wed, Jan 23, 2013 at 2:32 AM, Eric Wong  wrote:
> > Does squashing this on top of your changes fix all your failures?
> > I plan on squashing both your changes together with the below:
> 
> Yes, I can confirm that applying this patch on top of mine makes all
> git-svn tests pass again. I have also re-run the tests without my patch
> applied and found that they do all indeed pass, so I apologize for my
> previous incorrect comment.

Thanks, squashed, tested and pushed (have another unrelated patch coming)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-svn: cleanup sprintf usage for uppercasing hex

2013-01-23 Thread Eric Wong
We do not need to call uc() separately for sprintf("%x")
as sprintf("%X") is available.

Signed-off-by: Eric Wong 
---
 perl/Git/SVN.pm| 4 ++--
 perl/Git/SVN/Editor.pm | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 59215fa..490e330 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -490,7 +490,7 @@ sub refname {
#
# Additionally, % must be escaped because it is used for escaping
# and we want our escaped refname to be reversible
-   $refname =~ s{([ \%~\^:\?\*\[\t])}{uc sprintf('%%%02x',ord($1))}eg;
+   $refname =~ s{([ \%~\^:\?\*\[\t])}{sprintf('%%%02X',ord($1))}eg;
 
# no slash-separated component can begin with a dot .
# /.* becomes /%2E*
@@ -2377,7 +2377,7 @@ sub map_path {
 
 sub uri_encode {
my ($f) = @_;
-   $f =~ s#([^a-zA-Z0-9\*!\:_\./\-])#uc sprintf("%%%02x",ord($1))#eg;
+   $f =~ s#([^a-zA-Z0-9\*!\:_\./\-])#sprintf("%%%02X",ord($1))#eg;
$f
 }
 
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 3db1521..fa0d3c6 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -146,7 +146,7 @@ sub url_path {
my ($self, $path) = @_;
if ($self->{url} =~ m#^https?://#) {
# characters are taken from subversion/libsvn_subr/path.c
-   $path =~ s#([^~a-zA-Z0-9_./!$&'()*+,-])#uc 
sprintf("%%%02x",ord($1))#eg;
+   $path =~ 
s#([^~a-zA-Z0-9_./!$&'()*+,-])#sprintf("%%%02X",ord($1))#eg;
}
$self->{url} . '/' . $self->repo_path($path);
 }
-- 
Eric Wong
--
To unsubscribe from this list: send the line "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] Revoke write access to refs and odb after importing another repo's odb

2013-01-23 Thread Duy Nguyen
On Thu, Jan 24, 2013 at 12:01 AM, Junio C Hamano  wrote:
> I however have this suspicion that this will become a losing battle
> and we would be better off getting rid of add_submodule_odb();
> instead operations that work across repositories will be done as a
> subprocess, which will get us back closer to one of the original
> design goals of submodule support to have a clear separation between
> the superproject and its submodules.

It does not have to be subprocess. Thomas Rast did some work on
support multithread access to object db by basically replicating all
datastructure per thread. If that work is complete, we have something
like "odb container" that could be used to access objects from another
repository and it won't contaminate the original odb. The same thing
can be done for ref and index access.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PULL] git-svn updates for master

2013-01-23 Thread Eric Wong
The following changes since commit ec3ae6ec46ed48383ae40643990f169b65a563cc:

  Merge git://ozlabs.org/~paulus/gitk (2013-01-23 08:35:03 -0800)

are available in the git repository at:


  git://bogomips.org/git-svn master

for you to fetch changes up to 812ed405ac961093b7eb916246d5f288630edfb2:

  git-svn: cleanup sprintf usage for uppercasing hex (2013-01-24 01:30:04 +)


Barry Wardell (1):
  git-svn: Simplify calculation of GIT_DIR

Eric Wong (1):
  git-svn: cleanup sprintf usage for uppercasing hex

 git-svn.perl | 38 +++---
 perl/Git/SVN.pm  |  4 ++--
 perl/Git/SVN/Editor.pm   |  2 +-
 t/t9100-git-svn-basic.sh |  8 
 4 files changed, 26 insertions(+), 26 deletions(-)

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


Re: [PATCH] git-svn: cleanup sprintf usage for uppercasing hex

2013-01-23 Thread Jonathan Nieder
Eric Wong wrote:

> We do not need to call uc() separately for sprintf("%x")
> as sprintf("%X") is available.

For what it's worth,
Reviewed-by: Jonathan Nieder 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] don't use timers if NO_SETITIMER is set

2013-01-23 Thread Sébastien Boisvert
With NO_SETITIMER, the user experience on legacy Lustre is fixed,
but there is no early progress.

The patch has no effect on the resulting git executable if NO_SETITIMER is
not set (the default). So by default this patch has no effect at all, which
is good.

git tests:

$ make clean
$ make NO_SETITIMER=YesPlease
$ make test NO_SETITIMER=YesPlease &> make-test.log

$ grep "^not ok" make-test.log |grep -v "# TODO known breakage"|wc -l
0
$ grep "^ok" make-test.log |wc -l
9531
$ grep "^not ok" make-test.log |wc -l
65

No timers with NO_SETITIMER:

$ objdump -d ./git|grep setitimer|wc -l
0
$ objdump -d ./git|grep alarm|wc -l
0

Timers without NO_SETITIMER:

$ objdump -d /software/apps/git/1.8.1/bin/git|grep setitimer|wc -l
5
$ objdump -d /software/apps/git/1.8.1/bin/git|grep alarm|wc -l
0

Signed-off-by: Sébastien Boisvert 
---
 builtin/log.c |7 +++
 daemon.c  |6 ++
 progress.c|8 
 upload-pack.c |2 ++
 4 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..f8321c7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -198,7 +198,9 @@ static void show_early_header(struct rev_info *rev, const 
char *stage, int nr)
printf(_("Final output: %d %s\n"), nr, stage);
 }
 
+#ifndef NO_SETITIMER
 static struct itimerval early_output_timer;
+#endif
 
 static void log_show_early(struct rev_info *revs, struct commit_list *list)
 {
@@ -240,9 +242,12 @@ static void log_show_early(struct rev_info *revs, struct 
commit_list *list)
 * trigger every second even if we're blocked on a
 * reader!
 */
+
+   #ifndef NO_SETITIMER
early_output_timer.it_value.tv_sec = 0;
early_output_timer.it_value.tv_usec = 50;
setitimer(ITIMER_REAL, &early_output_timer, NULL);
+   #endif
 }
 
 static void early_output(int signal)
@@ -274,9 +279,11 @@ static void setup_early_output(struct rev_info *rev)
 *
 * This is a one-time-only trigger.
 */
+   #ifndef NO_SETITIMER
early_output_timer.it_value.tv_sec = 0;
early_output_timer.it_value.tv_usec = 10;
setitimer(ITIMER_REAL, &early_output_timer, NULL);
+   #endif
 }
 
 static void finish_early_output(struct rev_info *rev)
diff --git a/daemon.c b/daemon.c
index 4602b46..eb82c19 100644
--- a/daemon.c
+++ b/daemon.c
@@ -611,9 +611,15 @@ static int execute(void)
if (addr)
loginfo("Connection from %s:%s", addr, port);
 
+   #ifndef NO_SETITIMER
alarm(init_timeout ? init_timeout : timeout);
+   #endif
+
pktlen = packet_read_line(0, line, sizeof(line));
+
+   #ifndef NO_SETITIMER
alarm(0);
+   #endif
 
len = strlen(line);
if (pktlen != len)
diff --git a/progress.c b/progress.c
index 3971f49..b84ccc7 100644
--- a/progress.c
+++ b/progress.c
@@ -45,7 +45,10 @@ static void progress_interval(int signum)
 static void set_progress_signal(void)
 {
struct sigaction sa;
+
+   #ifndef NO_SETITIMER
struct itimerval v;
+   #endif
 
progress_update = 0;
 
@@ -55,16 +58,21 @@ static void set_progress_signal(void)
sa.sa_flags = SA_RESTART;
sigaction(SIGALRM, &sa, NULL);
 
+   #ifndef NO_SETITIMER
v.it_interval.tv_sec = 1;
v.it_interval.tv_usec = 0;
v.it_value = v.it_interval;
setitimer(ITIMER_REAL, &v, NULL);
+   #endif
 }
 
 static void clear_progress_signal(void)
 {
+   #ifndef NO_SETITIMER
struct itimerval v = {{0,},};
setitimer(ITIMER_REAL, &v, NULL);
+   #endif
+
signal(SIGALRM, SIG_IGN);
progress_update = 0;
 }
diff --git a/upload-pack.c b/upload-pack.c
index 95d8313..e0b8b32 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -47,7 +47,9 @@ static int stateless_rpc;
 
 static void reset_timeout(void)
 {
+   #ifndef NO_SETITIMER
alarm(timeout);
+   #endif
 }
 
 static int strip(char *line, int len)
-- 
1.7.4.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: auto packing with simultaneous pushes: "error: Could not stat 'objects/[…]/[…]'"

2013-01-23 Thread Ivan D Vasin
On Wed, Jan 23, 2013 at 7:28 PM, Junio C Hamano  wrote:
> Ivan D Vasin  writes:
>
>> my suggestion is that an auto pack should lock the repository,
>> preventing at least other auto packs (and perhaps other operations)
>> ...
>>
>> ``git fsck`` is successful on both of our repos and on the bare repo
>> to which we pushed.
>
> Successful after you pushed, or before you pushed, or both?

both

>
> I suspect both.
>
> I do not think such a lock is necessary for correctness of the
> operation, but running two auto packing sumultaneously is wasteful,
> so it would help performance.  But that would produce a larger
> problem.  What if your modified auto-packer takes a lock and then
> dies without relinquishing the lock?  The repository will never be
> repacked after such an event forever?

perhaps the lock could contain the PID of the auto pack process.  if
that PID has gone away, the lock is ignored and replaced with a new
one.

that's what comes to my mind.  of course, there could be other ways to
handle this that i'm not thinking of.

in any case, the error messages, though spurious, are alarming to the
uninformed user.  it looks like Git is saying that there is actual
data loss, where in fact there is none.  if Git doesn't prevent these
messages from appearing (via locking behavior or otherwise), then it
should at least annotate them with a message that describes their
possibly spurious nature and perhaps instructs the user to verify
everything with ``git fsck``.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] fixup remaining cvsimport tests

2013-01-23 Thread Michael Haggerty
On 01/23/2013 12:03 PM, John Keeping wrote:
> On Wed, Jan 23, 2013 at 10:54:36AM +0100, Michael Haggerty wrote:
>> On 01/20/2013 09:17 PM, Chris Rorvick wrote:
>>> I have never used cvs2git, but I suspect Eric's efforts in making it a
>>> potential backend for cvsimport are a better use of time.
> 
> Is it possible to perform an incremental import with cvs2git?  This
> seems to be the one use case where the old cvsimport script (with cvsps
> 2.x) still performs the best.
> 
> I suppose that just re-running the full import will do the right thing
> since the commits in Git should be identical, but would it be possible
> to do better given the right information about a previous run?

No, cvs2git does not support incremental imports.  One user has reported
that he *usually* gets identical commits for the overlapping history
when he re-runs a full import, and last I heard he was using this as a
kind of incremental import.  We make an effort to make imports
reproducible, at least when using a single version of cvs2git (for
example, we process things in deterministic order rather than the order
they happen come out of a file directory or Python hashmap).  But the
cycle-breaking heuristics in particular can give different results if
history is added, not to mention the fact that CVS allows the user to
make changes with retroactive and non-timestamped effects (e.g., adding
or removing files from an existing branch/tag, changing a file's default
branch from vendor to HEAD, changing the log messages of old revisions,
obsoleting revisions).  And if your repository is large, a full import
can take a while.

It would be possible to enhance cvs2git to handle incremental imports
(well, at least if you rule out a few CVS commands that change deep
history).  But I don't believe anybody is working on this.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 01/10] wildmatch: fix "**" special case

2013-01-23 Thread Junio C Hamano
Duy Nguyen  writes:

> If we do that, we need to do the same in tree_entry_interesting(). In
> other words, pathspec learns the new glob syntax. It's fine for an
> experimental flag like USE_WILDMATCH. But after fnmatch is replaced by
> wildmatch unconditionally (thus USE_WILDMATCH becomes obsolete), then
> what? Should people who expects new syntax with USE_WILDMATCH continue
> to have new syntax? How does a user know which syntax may be used in
> his friend's "git" binary?

Good point.

> On a related subject, I've been meaning to write a mail about the
> other use of patterns in git (e.g. in git-branch, git-tag,
> git-apply...) but never got around to. Some use without FNM_PATHNAME,
> some do and the document does not go into details about the
> differences. We might want to unify the syntax there too. It'll
> probably break backward compatibility so we can do the same when we
> switch pathspec syntax.

Right now, I think for-each-ref is the only one with FNM_PATHNAME.
With the experimental USE_WILDMATCH, "for-each-ref refs/**/master"
will give us what is naturally expected.  With a working wildmatch,
I think it probably makes sense to globally enable FNM_PATHNAME;
it would probably be nice if we could do so at Git 2.0 version bump
boundary, but I suspect we are not ready yet (as you pointed out,
there are still codepaths that need to be adjusted).

> The only problem I see is, without the version string, there's no way
> to know if "**" is supported. Old git versions will happily take "**"
> and interpret as "*". When you advise someone to use "**" you might
> need to add "check if you have this version of git". This problem does
> not exist with pathspec magic like :(glob)

OK, so what do we want to do when we do the real "USE_WILDMATCH"
that is not the current experimental curiosity?  Use ":(wild)" or
something?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jan 2013, #08; Tue, 22)

2013-01-23 Thread Junio C Hamano
John Keeping  writes:

>> Is it "it does not work yet with cvsps3", or "it will not ever work
>> with cvsps3"?  The impression I am getting is that it is the latter.
>
> The existing script (git-cvsimport.perl) won't ever work with cvsps-3
> since features it relies on have been removed.

I think you knew I already knew that.  I was hoping that cvsimport-3
that has multiple backend support may be able to start working by
reading the fast-import stream cvsps3 produces, once you sort out
the "last exported timestamp" issue out.  As far as the end users
are concerned, they would still be using cvsimport, even though the
wrapper may redirect the invocation to cvsimport-3.

In any case, something like that will not happen in the near term,
if ever, so "cvsimport will not work if you only have cvsps3" is a
good thing to add to its documentation.

Care to roll a proper patch with a log message?  I'll discard the
topic for now and replace it with your documentation update.

>> Also, should we have a suggestion to people who are *not* performing
>> a one-shot import, i.e. doing incremental or bidirectional?
>
> As far as I know cvsps is the only backend that attempts to support
> partial exports but the support for that in its fast-export mode needs
> work before I would consider it reliable.  For now the existing
> git-cvsimport is the best option I'm aware of.

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


Re: [PATCH v3 0/2] Make git-svn work with gitdir links

2013-01-23 Thread Junio C Hamano
Eric Wong  writes:

> diff --git a/git-svn.perl b/git-svn.perl
> index c232798..e5bd292 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
>   $ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);
>   } "Unable to find .git directory\n";
>   my $cdup = undef;
> + my $git_dir = delete $ENV{GIT_DIR};
>   git_cmd_try {
>   $cdup = command_oneline(qw/rev-parse --show-cdup/);
>   chomp $cdup if ($cdup);
>   $cdup = "." unless ($cdup && length $cdup);
> - } "Already at toplevel, but $ENV{GIT_DIR} not found\n";
> + } "Already at toplevel, but $git_dir not found\n";
> + $ENV{GIT_DIR} = $git_dir;
>   chdir $cdup or die "Unable to chdir up to '$cdup'\n";
>   $_repository = Git->repository(Repository => $ENV{GIT_DIR});
>  }

This does not look quite right, though.

Can't the user have his own $GIT_DIR when this command is invoked?
The first command_oneline() runs rev-parse with that environment and
get the user specified value of GIT_DIR in $ENV{GIT_DIR}, but by
doing a "delete" before running --show-cdup, you are not honoring
that GIT_DIR (and GIT_WORK_TREE if exists) the user gave you.  You
already used that GIT_DIR when you asked rev-parse --git-dir to find
what the GIT_DIR value should be, so you would be operating with
values of $git_dir and $cdup that you discovered in an inconsistent
way, no?

Shouldn't it be more like this instead?

my ($git_dir, $cdup) = undef;
try {
$git_dir = command_oneline(qw(rev-parse --git-dir));
} "Unable to ...";
try {
$cdup = command_oneline(qw(rev-parse --show-cdup));
... tweak $cdup ...
} "Unable to ...";
if (defined $git_dir) { $ENV{GIT_DIR} = $git_dir; }
chdir $cdup;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 01/10] wildmatch: fix "**" special case

2013-01-23 Thread Duy Nguyen
On Thu, Jan 24, 2013 at 11:49 AM, Junio C Hamano  wrote:
>> The only problem I see is, without the version string, there's no way
>> to know if "**" is supported. Old git versions will happily take "**"
>> and interpret as "*". When you advise someone to use "**" you might
>> need to add "check if you have this version of git". This problem does
>> not exist with pathspec magic like :(glob)
>
> OK, so what do we want to do when we do the real "USE_WILDMATCH"
> that is not the current experimental curiosity?  Use ":(wild)" or
> something?

I don't think we need to support two different sets of wildcards in
the long run. I'm thinking of adding ":(glob)" with WM_PATHNAME.
Pathspec without :(glob) still uses the current wildcards (i.e. no
FNM_PATHNAME). At some point, like 2.0, we either switch the behavior
of patterns-without-:(glob) to WM_PATHNAME, or just disable wildcards
when :(glob) is not present.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] Revoke write access to refs and odb after importing another repo's odb

2013-01-23 Thread Duy Nguyen
On Thu, Jan 24, 2013 at 4:06 AM, Junio C Hamano  wrote:
> Jens Lehmann  writes:
>
>> This is a false positive. The merge algorithm picked a fast-forward
>> in a submodule as a proper merge result and records that in a
>> gitlink. But as Duy pointed out this could be easily fixed by
>> turning the readonly flag off in that case.
>
> I see that as "easily circumvented and not an effective protection",
> though.
>
> In theory, adding a gitlink to the index, removing a gitlink to the
> index and modifying an existing gitlink in the index to another
> gitlink in the index and writing the resulting in-core index out to
> the on-disk index should be allowed, even after objects from the
> submodule object database have contaminated our in-core object pool,
> as long as you do not run cache_tree_update().  I am not sure if that
> single loophole would be sufficient, though.

The problem is we don't know which entries are updated in index. We
don't keep track of them. And I think in the unpack-trees case, we
scape the whole index then copy over, making it look like the whole
index is updated (even with the same content). One way to check this
is verify the source of all non-gitlink entries in index before
writing to disk (only when readonly flag is on, of course).
sha1_object_info_extended() should help (or be extended to do the
job). Hmm.. if we do this, we could also verify if new sha-1 objects
do not refer to an external source, if so allow them to be created.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] parse_object: clear "parsed" when freeing buffers

2013-01-23 Thread Jeff King
On Wed, Jan 23, 2013 at 04:25:01PM -0800, Junio C Hamano wrote:

> > With the object cache, isn't modifying the object unsafe in
> > general? Instead of auditing code paths, it's now necessary to
> > audit _all_ code that uses "struct object", which seems
> > infeasible.
> 
> The object layer was designed with "run one thing and one thing
> well, and then let the _exit(2) take care of the clean-up" model in
> mind; modifying the object, e.g. updating commit->parents list,
> in-core by revision traversal machinery is very much within the
> scope of its intended uses.

Yeah, although I think the "parsed" flag really is a bit overloaded for
commits.

Most code which uses "struct commit" will want the parents, timestamp,
and tree information filled in. And so they call parse_commit to make
sure that is the case. Afterwards, those fields are valid and the
"parsed" flag is set. The buffer field may or may not be valid
afterwards, depending on save_commit_buffer.

Some other code may also care about the rest of the commit information.
It also calls parse_commit, but with save_commit_buffer turned on.
However, that may or may not actually initialize the buffer, depending
on who has been using the object before us.

It works in practice most of the time because we only perform one "task"
per invocation, and that task either keeps the commit messages around
the first time, or doesn't. But it is a bit of a ticking time bomb
anytime we violate that assumption.

I think it would be saner for callers who only care about ancestry info
call to call parse_commit, and then anybody who is about to access the
buffer to call a new ensure_commit_buffer function, which would make
sure the buffer is accessible (even if save_commit_buffer is false).

Then save_commit_buffer becomes just an optimization: save it for later
during parsing, so we don't have to do it later. That would also let us
optimize better if we end up with a commit ancestry cache which can do
parse_commit much cheaper than accessing the full object.

For example, my commit cache patches hook into parse_commit, but they
can only do so safely when save_commit_buffer is false. So git-rev-list
benefits, but git-log does not. However, if we knew that log would
lazily load the commit data when needed, we could use the cache
there, too. It would not be a win if you are showing every commit
anyway, but if you have limiting that does not depend on the commit
object itself (e.g., path limiting in the tree), you could avoid loading
some commits entirely.

> > I'm just trying to fix the segfault demonstrated in the test
> > attached to the patch.
> 
> Can offending readers that dereference NULL without checking if
> buffer has been freed be updated so that they read_sha1_file(), read
> the contents from the result returned from the function (instead of
> reading from .buffer), and free the memory when they are done?

Looks like builtin/blame.c:get_commit_info does this already, although
it leaves the buffer attached to the commit and does not free it.

The ensure_commit_buffer function could look something like:

  int ensure_commit_buffer(struct commit *item)
  {
  enum object_type type;
  unsigned long size;

  if (!item)
  return -1;
  if (!item->object.parsed)
  return parse_commit(item);
  if (item->buffer)
  return 0;

  item->buffer = read_sha1_file(item->object.sha1, &type, &size);
  if (!item->buffer)
  return error("Could not read %s",
   sha1_to_hex(item->object.sha1);
  return 0;
  }

and blame could replace its in-line processing with that. It does leave
open the question of whether callers should free() the result. But that
would be up to each user of the object (and it would be an optimization
issue, not a correctness issue, as long as each user called
ensure_commit_buffer before accessing it).

But the first step there would be to audit all of the accesses of
commit->buffer to make sure that they call ensure_commit_buffer
(presumably they already call parse_commit).

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


Re: [PATCH v2 2/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE

2013-01-23 Thread Jeff King
On Wed, Jan 23, 2013 at 08:28:49AM -0800, Junio C Hamano wrote:

> How about doing this?
> 
> For "needs force" cases, we say this instead:
> 
>  hint: you cannot update a ref that points at a non-commit object, or
>  hint: update a ref to point at a non-commit object, without --force.
> 
> Being explicit about "non-commit" twice will catch user's eyes and
> cause him to double check that it is not a mistyped LHS of the push
> refspec (if he is sending a non-commit) or mistyped RHS (if the ref
> is pointing at a non-commit).  If he _is_ trying to push a blob out,
> the advice makes it clear what to do next: he does want to force it.

Yeah, I think that is sensible.

> Note that you _could_ split the "needs force" case into two, namely,
> "cannot replace a non-commit" and "cannot push a non-commit".  You
> could even further split them [...etc...]

I do not think it is worth worrying too much about. This should really
not happen very often, and the user should be able to investigate and
figure out what is going on. I think making the error message extremely
specific is just going to end up making it harder to understand.

> If we did that, then we could loosen the "You should fetch first"
> case to say something like this:
> 
>  hint: you do not have the object at the tip of the remote ref;
>  hint: perhaps you want to pull from there first?

Yeah, better. I'll comment on the specific message you used in response
to the patch itself.

> This explicitly denies one of Chris's wish "we shouldn't suggest to
> merge something that we may not be able to", but in the "You should
> fetch first" case, we cannot fundamentally know if we can merge
> until we fetch.  I agree with you that the most common case is that
> the unknown object is a commit, and that suggesting to pull is a
> good compromise.

I thought the wish was more about "we shouldn't suggest to merge
something we _know_ we will not be able to", and you are still handling
that (i.e., the "needs force" case).

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


Re: [PATCH v4 3/3] push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCE

2013-01-23 Thread Jeff King
On Wed, Jan 23, 2013 at 01:55:30PM -0800, Junio C Hamano wrote:

> If we do not have the current object at the tip of the remote, we do
> not even know that object, when fetched, is something that can be
> merged.  In such a case, suggesting to pull first just like
> non-fast-forward case may not be technically correct, but in
> practice, most such failures are seen when you try to push your work
> to a branch without knowing that somebody else already pushed to
> update the same branch since you forked, so "pull first" would work
> as a suggestion most of the time.
> 
> In these cases, the current code already rejects such a push on the
> client end, but we used the same error and advice messages as the
> ones used when rejecting a non-fast-forward push, i.e. pull from
> there and integrate before pushing again.  Introduce new
> rejection reasons and reword the messages appropriately.

So obviously from our previous discussion, I agree with the general
behavior of this patch. Let me get nit-picky on the message itself,
though:

> +static const char message_advice_ref_fetch_first[] =
> + N_("Updates were rejected because you do not have the object at the 
> tip\n"
> +"of the remote. You may want to first merge the remote changes 
> (e.g.\n"
> +" 'git pull') before pushing again.\n"
> +"See the 'Note about fast-forwards' in 'git push --help' for 
> details.");
> +

The condition that triggers this message is going to come up fairly
often for new git users (e.g., anyone using a central repo model), which
I think is why the original message_advice_pull_before_push has gotten
so much attention.  And in most cases, users will be seeing this message
now instead of "pull before push", because the common triggering cause
is somebody else pushing unrelated work.

The existing message says:

  Updates were rejected because a pushed branch tip is behind its remote
  counterpart. Check out this branch and merge the remote changes
  (e.g. 'git pull') before pushing again.

I wonder: will the new message be as comprehensible to a new user as the
old?

They are quite similar, but something about the presence of the word
"behind" in the latter makes me think it helps explain what is going on
a bit more. When I read the new one, my first question is "why don't I
have that object?". Of course, saying "behind" in this case would not be
strictly accurate, because we do not even know the remote has a commit.

I wonder if we can reword it to explain more about why we do not have
the object, without getting too inaccurate. Something like:

  Updates were rejected because the remote contains objects that you do
  not have locally. This is usually caused by another repository pushing
  to the same ref. You may want to first merge the remote changes (e.g.,
  'git pull') before pushing again.

I was also tempted to s/objects/work/, which is more vague, but is less
jargon-y for new users who do not know how git works.

Also, how should this interact with the checkout-then-pull-then-push
advice? We make a distinction for the non-fastforward case between HEAD
and other refs. Should we be making the same distinction here?

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


Re: [PATCH] parse_object: clear "parsed" when freeing buffers

2013-01-23 Thread Jeff King
On Wed, Jan 23, 2013 at 01:25:04PM -0800, Jonathon Mah wrote:

> Several areas of code would free buffers for object structs that
> contained them ("struct tree" and "struct commit"), but without clearing
> the "parsed" flag. parse_object would clear the flag for "struct tree",
> but commits would remain in an invalid state (marked as parsed but with
> a NULL buffer). Because the objects are uniqued (ccdc6037fee), the
> invalid objects stay around and can lead to bad behavior.
> 
> In particular, running pickaxe on all refs in a repository with a cached
> textconv could segfault. If the textconv cache ref was evaluated first
> by cmd_log_walk, a subsequent notes_cache_match_validity call would
> dereference NULL.

Just to be sure I understand, what is going on is something like this?

  Log reads all refs, including notes. After showing the notes commit,
  log frees the buffer to save memory.  Later, doing the diff on a
  commit causes us to use the notes_cache mechanism. That will look at
  the commit at the tip of the notes ref, which log has already
  processed. It will call parse_commit, but that will do nothing, as the
  parsed flag is already set, but the commit buffer remains NULL.

I wonder if this could be the source of the segfault here, too:

  http://thread.gmane.org/gmane.comp.version-control.git/214322/focus=214355

If you have refs pointing to both a submodule and its superproject, a
"log --all --submodule -p" might process the submodule's commits first,
free their buffers, and then want to show them again as part of the
submodule diff. This code in builtin/log.c:

if (!rev->reflog_info) {
/* we allow cycles in reflog ancestry */
free(commit->buffer);
commit->buffer = NULL;
}

assumes we will only ever look at a commit once (except for
reflog-walking), but submodule diff violates that.

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


links of london sale developed because of a person

2013-01-23 Thread falimie
Through 1990,  links of london sale
   developed
because of a person obtain few of perch cufflinks on a hometown restaurateur
what individuals sought after an extraordinary item for the purpose of
routine shoppers. After that, typically the fine English tongue label seems
to have prospered with the help of all sorts of gold not to mention 18-carat
old watches diamond jewelry who gets critical recognition through boutiques
not to mention stores definitely not residential. Typically the Shortcuts
label is thought for the purpose of personalised such things as expensive
jewelry, extra, charms, not to mention earrings which were want not to
mention laid back all at one time. The foremost familiar device throughout
their broad gallery will be darling bracelet, a different bracelet who
presents expensive jewelry and / or is often placed per se. The theory
virtually outstanding towards Shortcuts from The uk, as well as so far his
or her's more popular bit of diamond jewelry. Typically the association
bracelet might be a second develop who is well known designed by model
http://www.linksoflondonbraceletsweetie.co.uk/ 



--
View this message in context: 
http://git.661346.n2.nabble.com/links-of-london-sale-developed-because-of-a-person-tp7575932.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What makes links of london friendship bracelet have out from the herd

2013-01-23 Thread falimie
Inbound links connected with Liverpool undoubtedly are a well known service
provider connected with both ladies and men necklaces. Recognized the
government financial aid 1990,  links of london sale
   happened on
account of an effective ask a couple species of fish cufflinks. This demand
got their start in some sort of diner manager exactly who needed to
attraction in addition to pay back his or her dependable shoppers that has a
trademark two of silver cufflinks. Harvey Nichols discovered the structure
in addition to promptly fell into excited about the item, many people
requested a full bunch of cuff inbound links for being manufactured only
with regards to Liverpool vogue retail store. And so Inbound links connected
with Liverpool appeared, in addition to presently this model is usually
realised around the world in addition to can be obtained from outlets along
the GREAT BRITAIN, Hong Kong, STATES in addition to The us.
http://www.cheaplinksoflondonbracelet.co.uk/



--
View this message in context: 
http://git.661346.n2.nabble.com/What-makes-links-of-london-friendship-bracelet-have-out-from-the-herd-tp7575933.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] mergetool--lib: fix startup options for gvimdiff tool

2013-01-23 Thread Alexey Shumkin
Options are taken from /mergetools/vim

Signed-off-by: Alexey Shumkin 
---
 git-gui/lib/mergetool.tcl | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index 3c8e73b..4fc1cab 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -211,7 +211,13 @@ proc merge_resolve_tool2 {} {
}
}
gvimdiff {
-   set cmdline [list "$merge_tool_path" -f "$LOCAL" "$MERGED" 
"$REMOTE"]
+   if {$base_stage ne {}} {
+   set cmdline [list "$merge_tool_path" -f -d -c "wincmd 
J" \
+   "$MERGED" "$LOCAL" "$BASE" "$REMOTE"]
+   } else {
+   set cmdline [list "$merge_tool_path" -f -d -c "wincmd 
l" \
+   "$LOCAL" "$MERGED" "$REMOTE"]
+   }
}
kdiff3 {
if {$base_stage ne {}} {
-- 
1.8.1.1.10.g9255f3f

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


[PATCH 2/3] mergetool--lib: Add diffuse as a tool

2013-01-23 Thread Alexey Shumkin
Signed-off-by: Alexey Shumkin 
---
 git-gui/lib/mergetool.tcl | 9 +
 1 file changed, 9 insertions(+)

diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
index 4fc1cab..837ce17 100644
--- a/git-gui/lib/mergetool.tcl
+++ b/git-gui/lib/mergetool.tcl
@@ -194,6 +194,15 @@ proc merge_resolve_tool2 {} {
set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" 
-mergeoutput="$MERGED"]
}
}
+   diffuse {
+   if {$base_stage ne {}} {
+   set cmdline [list "$merge_tool_path" \
+   "$LOCAL" "$MERGED" "$REMOTE" "$BASE"]
+   } else {
+   set cmdline [list "$merge_tool_path" \
+   "$LOCAL" "$MERGED" "$REMOTE"]
+   }
+   }
ecmerge {
if {$base_stage ne {}} {
set cmdline [list "$merge_tool_path" "$BASE" "$LOCAL" 
"$REMOTE" --default --mode=merge3 --to="$MERGED"]
-- 
1.8.1.1.10.g9255f3f

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


  1   2   >