[PATCH v2 3/3] git.c: make sure we do not leak GIT_* to alias scripts

2015-12-19 Thread Nguyễn Thái Ngọc Duy
The unfortunate commit d95138e (setup: set env $GIT_WORK_TREE when
work tree is set, like $GIT_DIR - 2015-06-26) exposes another problem,
besides git-clone that's described in the previous commit. If
GIT_WORK_TREE (or even GIT_DIR) is exported to an alias script, it may
mislead git commands in the script where the repo is. Granted, most
scripts work on the repo where the alias is summoned from. But nowhere
do we forbid the script to visit another repository.

The revert of d95138e in the previous commit is sufficient as a
fix. However, to protect us from accidentally leaking GIT_*
environment variables again, we restore certain sensitive env before
calling the external script.

GIT_PREFIX is let through because there's another setup side effect
that we simply accepted so far: current working directory is
moved. Maybe in future we can introduce a new alias format that
guarantees no cwd move, then we can unexport GIT_PREFIX.

Reported-by: Gabriel Ganne 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 git.c   | 10 +++---
 t/t0001-init.sh | 17 +
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/git.c b/git.c
index 1a7d399..da278c3 100644
--- a/git.c
+++ b/git.c
@@ -41,13 +41,16 @@ static void save_env_before_alias(void)
}
 }
 
-static void restore_env(void)
+static void restore_env(int external_alias)
 {
int i;
-   if (orig_cwd && chdir(orig_cwd))
+   if (!external_alias && orig_cwd && chdir(orig_cwd))
die_errno("could not move to %s", orig_cwd);
free(orig_cwd);
for (i = 0; i < ARRAY_SIZE(env_names); i++) {
+   if (external_alias &&
+   !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
+   continue;
if (orig_env[i])
setenv(env_names[i], orig_env[i], 1);
else
@@ -243,6 +246,7 @@ static int handle_alias(int *argcp, const char ***argv)
int argc = *argcp, i;
 
commit_pager_choice();
+   restore_env(1);
 
/* build alias_argv */
alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 1));
@@ -291,7 +295,7 @@ static int handle_alias(int *argcp, const char ***argv)
ret = 1;
}
 
-   restore_env();
+   restore_env(0);
 
errno = saved_errno;
 
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index f91bbcf..295aa59 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -87,6 +87,23 @@ test_expect_success 'plain nested in bare through aliased 
command' '
check_config bare-ancestor-aliased.git/plain-nested/.git false unset
 '
 
+test_expect_success 'No extra GIT_* on alias scripts' '
+   (
+   env | sed -ne "/^GIT_/s/=.*//p" &&
+   echo GIT_PREFIX &&# setup.c
+   echo GIT_TEXTDOMAINDIR# wrapper-for-bin.sh
+   ) | sort | uniq >expected &&
+   cat <<-\EOF >script &&
+   #!/bin/sh
+   env | sed -ne "/^GIT_/s/=.*//p" | sort >actual
+   exit 0
+   EOF
+   chmod 755 script &&
+   git config alias.script \!./script &&
+   ( mkdir sub && cd sub && git script ) &&
+   test_cmp expected actual
+'
+
 test_expect_success 'plain with GIT_WORK_TREE' '
mkdir plain-wt &&
test_must_fail env GIT_WORK_TREE="$(pwd)/plain-wt" git init plain-wt
-- 
2.3.0.rc1.137.g477eb31

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


[PATCH v2 2/3] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..

2015-12-19 Thread Nguyễn Thái Ngọc Duy
Commit d95138e [1] fixes a .git file problem by setting GIT_WORK_TREE
whenever GIT_DIR is set. It sounded harmless because we handle
GIT_DIR and GIT_WORK_TREE side by side for most commands, with two
exceptions: git-init and git-clone.

"git clone" is not happy with d95138e. This command ignores GIT_DIR
but respects GIT_WORK_TREE [2] [3] which means it used to run fine
from a hook, where GIT_DIR was set but GIT_WORK_TREE was not (*). With
d95138e, GIT_WORK_TREE is set all the time and git-clone interprets
that as "I give you order to put the worktree here", usually against
the user's intention.

The solution in d95138e is reverted in this commit. Instead we reuse
the solution from c056261 [4]. c056261 fixes another setup-messed-
up-by-alias by saving and restoring env and spawning a new process,
but for git-clone and git-init only. Now I conclude that setup-messed-
up-by-alias is always evil. So the env restoration is done for _all_
commands, including external ones, whenever aliases are involved. It
fixes what d95138e tries to fix. And it does not upset git-clone-
inside-hooks.

The test from d95138e remains to verify it's not broken by this. A new
test is added to make sure git-clone-inside-hooks remains happy.

(*) GIT_WORK_TREE was not set _most of the time_. In some cases
GIT_WORK_TREE is set and git-clone will behave differently. The
use of GIT_WORK_TREE to direct git-clone to put work tree
elsewhere looks like a mistake because it causes surprises this
way. But that's a separate story.

[1] d95138e (setup: set env $GIT_WORK_TREE when work tree is set, like
 $GIT_DIR - 2015-06-26)
[2] 2beebd2 (clone: create intermediate directories of destination
 repo - 2008-06-25)
[3] 20ccef4 (make git-clone GIT_WORK_TREE aware - 2007-07-06)
[4] c056261 (git potty: restore environments after alias expansion -
 2014-06-08)

Reported-by: Anthony Sottile 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 environment.c|  2 --
 git.c| 23 ---
 t/t5601-clone.sh | 23 +++
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/environment.c b/environment.c
index 2da7fe2..1cc4aab 100644
--- a/environment.c
+++ b/environment.c
@@ -235,8 +235,6 @@ void set_git_work_tree(const char *new_work_tree)
}
git_work_tree_initialized = 1;
work_tree = xstrdup(real_path(new_work_tree));
-   if (setenv(GIT_WORK_TREE_ENVIRONMENT, work_tree, 1))
-   die("could not set GIT_WORK_TREE to '%s'", work_tree);
 }
 
 const char *get_git_work_tree(void)
diff --git a/git.c b/git.c
index 6ce7043..1a7d399 100644
--- a/git.c
+++ b/git.c
@@ -226,7 +226,6 @@ static int handle_options(const char ***argv, int *argc, 
int *envchanged)
 static int handle_alias(int *argcp, const char ***argv)
 {
int envchanged = 0, ret = 0, saved_errno = errno;
-   const char *subdir;
int count, option_count;
const char **new_argv;
const char *alias_command;
@@ -234,7 +233,7 @@ static int handle_alias(int *argcp, const char ***argv)
int unused_nongit;
 
save_env_before_alias();
-   subdir = setup_git_directory_gently(&unused_nongit);
+   setup_git_directory_gently(&unused_nongit);
 
alias_command = (*argv)[0];
alias_string = alias_lookup(alias_command);
@@ -292,8 +291,7 @@ static int handle_alias(int *argcp, const char ***argv)
ret = 1;
}
 
-   if (subdir && chdir(subdir))
-   die_errno("Cannot change to '%s'", subdir);
+   restore_env();
 
errno = saved_errno;
 
@@ -308,7 +306,6 @@ static int handle_alias(int *argcp, const char ***argv)
  * RUN_SETUP for reading from the configuration file.
  */
 #define NEED_WORK_TREE (1<<3)
-#define NO_SETUP   (1<<4)
 
 struct cmd_struct {
const char *cmd;
@@ -390,7 +387,7 @@ static struct cmd_struct commands[] = {
{ "cherry", cmd_cherry, RUN_SETUP },
{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
-   { "clone", cmd_clone, NO_SETUP },
+   { "clone", cmd_clone },
{ "column", cmd_column, RUN_SETUP_GENTLY },
{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
{ "commit-tree", cmd_commit_tree, RUN_SETUP },
@@ -416,8 +413,8 @@ static struct cmd_struct commands[] = {
{ "hash-object", cmd_hash_object },
{ "help", cmd_help },
{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
-   { "init", cmd_init_db, NO_SETUP },
-   { "init-db", cmd_init_db, NO_SETUP },
+   { "init", cmd_init_db },
+   { "init-db", cmd_init_db },
{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
{ "log", cmd_log, RUN_SETUP },
{ "ls-files", cmd_ls_files, RUN_SETUP },
@@ -531,9 +528,13 @@ static void handle_builtin(int argc, const char **argv)
 
builtin = get_builtin(c

[PATCH v2 1/3] git.c: make it clear save_env() is for alias handling only

2015-12-19 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 git.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/git.c b/git.c
index 6ed824c..6ce7043 100644
--- a/git.c
+++ b/git.c
@@ -25,14 +25,14 @@ static const char *env_names[] = {
GIT_PREFIX_ENVIRONMENT
 };
 static char *orig_env[4];
-static int saved_environment;
+static int saved_env_before_alias;
 
-static void save_env(void)
+static void save_env_before_alias(void)
 {
int i;
-   if (saved_environment)
+   if (saved_env_before_alias)
return;
-   saved_environment = 1;
+   saved_env_before_alias = 1;
orig_cwd = xgetcwd();
for (i = 0; i < ARRAY_SIZE(env_names); i++) {
orig_env[i] = getenv(env_names[i]);
@@ -233,6 +233,7 @@ static int handle_alias(int *argcp, const char ***argv)
char *alias_string;
int unused_nongit;
 
+   save_env_before_alias();
subdir = setup_git_directory_gently(&unused_nongit);
 
alias_command = (*argv)[0];
@@ -530,7 +531,7 @@ static void handle_builtin(int argc, const char **argv)
 
builtin = get_builtin(cmd);
if (builtin) {
-   if (saved_environment && (builtin->option & NO_SETUP))
+   if (saved_env_before_alias && (builtin->option & NO_SETUP))
restore_env();
else
exit(run_builtin(builtin, argc, argv));
@@ -590,7 +591,6 @@ static int run_argv(int *argcp, const char ***argv)
 */
if (done_alias)
break;
-   save_env();
if (!handle_alias(argcp, argv))
break;
done_alias = 1;
-- 
2.3.0.rc1.137.g477eb31

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


[PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

2015-12-19 Thread Nguyễn Thái Ngọc Duy
Changes since v1:

 - make sure we save/restore env for external commands in 2/3
 - fix t0001 test in 3/3

Interdiff:

diff --git b/git.c a/git.c
index 83b6c56..da278c3 100644
--- b/git.c
+++ a/git.c
@@ -229,7 +229,6 @@ static int handle_options(const char ***argv, int *argc, 
int *envchanged)
 static int handle_alias(int *argcp, const char ***argv)
 {
int envchanged = 0, ret = 0, saved_errno = errno;
-   const char *subdir;
int count, option_count;
const char **new_argv;
const char *alias_command;
@@ -237,7 +236,7 @@ static int handle_alias(int *argcp, const char ***argv)
int unused_nongit;
 
save_env_before_alias();
-   subdir = setup_git_directory_gently(&unused_nongit);
+   setup_git_directory_gently(&unused_nongit);
 
alias_command = (*argv)[0];
alias_string = alias_lookup(alias_command);
@@ -296,8 +295,7 @@ static int handle_alias(int *argcp, const char ***argv)
ret = 1;
}
 
-   if (subdir && chdir(subdir))
-   die_errno("Cannot change to '%s'", subdir);
+   restore_env(0);
 
errno = saved_errno;
 
@@ -534,9 +532,13 @@ static void handle_builtin(int argc, const char **argv)
 
builtin = get_builtin(cmd);
if (builtin) {
-   if (saved_env_before_alias)
-   restore_env(0);
-   else
+   /*
+* XXX: if we can figure out cases where it is _safe_
+* to do, we can avoid spawning a new process when
+* saved_env_before_alias is true
+* (i.e. setup_git_dir* has been run once)
+*/
+   if (!saved_env_before_alias)
exit(run_builtin(builtin, argc, argv));
}
 }
diff --git b/t/t0001-init.sh a/t/t0001-init.sh
index 19539fc..295aa59 100755
--- b/t/t0001-init.sh
+++ a/t/t0001-init.sh
@@ -88,24 +88,14 @@ test_expect_success 'plain nested in bare through aliased 
command' '
 '
 
 test_expect_success 'No extra GIT_* on alias scripts' '
-   cat <<-\EOF >expected &&
-   GIT_ATTR_NOSYSTEM
-   GIT_AUTHOR_EMAIL
-   GIT_AUTHOR_NAME
-   GIT_COMMITTER_EMAIL
-   GIT_COMMITTER_NAME
-   GIT_CONFIG_NOSYSTEM
-   GIT_EXEC_PATH
-   GIT_MERGE_AUTOEDIT
-   GIT_MERGE_VERBOSITY
-   GIT_PREFIX
-   GIT_TEMPLATE_DIR
-   GIT_TEXTDOMAINDIR
-   GIT_TRACE_BARE
-   EOF
+   (
+   env | sed -ne "/^GIT_/s/=.*//p" &&
+   echo GIT_PREFIX &&# setup.c
+   echo GIT_TEXTDOMAINDIR# wrapper-for-bin.sh
+   ) | sort | uniq >expected &&
cat <<-\EOF >script &&
#!/bin/sh
-   env | grep GIT_ | sed "s/=.*//" | sort >actual
+   env | sed -ne "/^GIT_/s/=.*//p" | sort >actual
exit 0
EOF
chmod 755 script &&

Nguyễn Thái Ngọc Duy (3):
  git.c: make it clear save_env() is for alias handling only
  setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..
  git.c: make sure we do not leak GIT_* to alias scripts

 environment.c|  2 --
 git.c| 41 +++--
 t/t0001-init.sh  | 17 +
 t/t5601-clone.sh | 23 +++
 4 files changed, 63 insertions(+), 20 deletions(-)

-- 
2.3.0.rc1.137.g477eb31

--
To unsubscribe from this list: send the line "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] commit: ensure correct permissions of the commit message

2015-12-19 Thread Jeff King
On Sat, Dec 19, 2015 at 07:21:59PM +0100, Johannes Schindelin wrote:

> It was pointed out by Yaroslav Halchenko that the file containing the
> commit message had the wrong permissions in a shared setting.
> 
> Let's fix that.
> 
> Signed-off-by: Johannes Schindelin 

I think this is probably a step forward, but I have to wonder how many
other files are in a similar situation (e.g., git-am state files, etc).
I think people generally haven't noticed because shared repositories are
generally about a shared bare rendezvous repo. So refs and objects are
important, but we don't expect people to commit.

So I don't have any real problem with this, but I suspect it's just the
tip of the iceberg. We might want something like:

  FILE *fopen_shared(const char *path, const char *mode)
  {
FILE *ret = fopen(path, mode);
if (!ret)
return NULL;
if (adjust_shared_perm(path)) {
fclose(ret);
return NULL;
}
return ret;
  }

but of course the hard part is auditing all of the existing fopen()
calls to see who needs to use it. :)

-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: Forcing git to pack objects

2015-12-19 Thread Jeff King
On Fri, Dec 18, 2015 at 07:03:39PM -0600, Edmundo Carmona Antoranz wrote:

> Recently I was running manually a git gc --prune command (wanted to
> shrink my 2.8G .git directory by getting rid of loose objects) and I
> ended up running out of space on my HD. After freaking out a little
> bit (didn't know if the repo would end up in a 'stable' state), I
> ended up freeing up some space and I again have a working repo...
> _but_ I noticed that basically _all_ objects on my repo are laying
> around in directories .git/objects/00 to ff (and taking a whole lot of
> space... like the .git directory is now like 5 GBs). After running git
> gc manually again it ended up taking a lot of time and the objects are
> still there. Also git svn sometimes gcs after fetching and it took to
> run cause of the gc execution (ended up killing it) and the files are
> still there. Is it possible to ask git to put all those objects in
> .pack files? Or did I mess something on my repo?

It sounds like these objects aren't reachable from any of your refs.
That would explain:

 - why they don't get packed; we only pack reachable objects.

   There's no easy way with "git gc" or "git repack" to pack unreachable
   objects. If you drop down to the "git pack-objects" level, you can
   manually convince it to do so.

   I have a patch to add "git repack -k", which packs even unreachable
   objects (we use this at GitHub, since our routine repository
   maintenance never deletes objects). If people are interested, I can
   clean it up and post it to the list.

 - why your repo grew during the gc; packed unreachable objects are
   exploded into loose objects during a gc, so they can age and expire
   on their own (we use filesystem mtimes to determine how old they
   are).

   Git is smart enough not to explode loose objects that it is just
   going to prune in a moment.  But the default expiration time is 2
   weeks, so there's a good chance it would want to keep some of them.

   If you use "git gc --prune=now", that should prune everything
   immediately (and this is safe, as long as you know there are no
   simultaneous users of your local repository).

-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


[PATCH 4/4] create_symref: use existing ref-lock code

2015-12-19 Thread Jeff King
The create_symref() function predates the existence of
"struct lock_file", let alone the more recent "struct
ref_lock". Instead, it just does its own manual dot-locking.
Besides being more code, this has a few downsides:

 - if git is interrupted while holding the lock, we don't
   clean up the lockfile

 - we don't do the usual directory/filename conflict check.
   So you can sometimes create a symref "refs/heads/foo/bar",
   even if "refs/heads/foo" exists (namely, if the refs are
   packed and we do not hit the d/f conflict in the
   filesystem).

This patch refactors create_symref() to use the "struct
ref_lock" interface, which handles both of these things.
There are a few bonus cleanups that come along with it:

 - we leaked ref_path in some error cases

 - the symref contents were stored in a fixed-size buffer,
   putting an artificial (albeit large) limitation on the
   length of the refname. We now write through fprintf, and
   handle refnames of any size.

 - we called adjust_shared_perm only after the file was
   renamed into place, creating a potential race with
   readers in a shared repository. Now we fix the
   permissions first, and commit only if that succeeded.
   This also makes the update atomic with respect to our
   exit code (whereas previously, we might report failure
   even though we updated the ref).

 - the legacy prefer_symlink_refs path did not do any
   locking at all. Admittedly, it is not atomic from a
   reader's perspective (and it cannot be; it has to unlink
   and then symlink, creating a race), but at least it
   cannot conflict with other writers now.

 - the result of this patch is hopefully more readable. It
   eliminates three goto labels. Two were for error checking
   that is now simplified, and the third was to reach shared
   code that has been pulled into its own function.

Signed-off-by: Jeff King 
---
 refs/files-backend.c| 113 +---
 t/t1401-symbolic-ref.sh |   8 
 2 files changed, 66 insertions(+), 55 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6bfa139..3d53c42 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2811,74 +2811,77 @@ static int commit_ref_update(struct ref_lock *lock,
return 0;
 }
 
-int create_symref(const char *ref, const char *target, const char *logmsg)
+static int create_ref_symlink(struct ref_lock *lock, const char *target)
 {
-   char *lockpath = NULL;
-   char buf[1000];
-   int fd, len, written;
-   char *ref_path = git_pathdup("%s", ref);
-   unsigned char old_sha1[20], new_sha1[20];
-   struct strbuf err = STRBUF_INIT;
-
-   if (logmsg && read_ref(ref, old_sha1))
-   hashclr(old_sha1);
-
-   if (safe_create_leading_directories(ref_path) < 0)
-   return error("unable to create directory for %s", ref_path);
-
+   int ret = -1;
 #ifndef NO_SYMLINK_HEAD
-   if (prefer_symlink_refs) {
-   unlink(ref_path);
-   if (!symlink(target, ref_path))
-   goto done;
+   char *ref_path = get_locked_file_path(lock->lk);
+   unlink(ref_path);
+   ret = symlink(target, ref_path);
+   free(ref_path);
+
+   if (ret)
fprintf(stderr, "no symlink - falling back to symbolic ref\n");
-   }
 #endif
+   return ret;
+}
 
-   len = snprintf(buf, sizeof(buf), "ref: %s\n", target);
-   if (sizeof(buf) <= len) {
-   error("refname too long: %s", target);
-   goto error_free_return;
-   }
-   lockpath = mkpathdup("%s.lock", ref_path);
-   fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
-   if (fd < 0) {
-   error("Unable to open %s for writing", lockpath);
-   goto error_free_return;
-   }
-   written = write_in_full(fd, buf, len);
-   if (close(fd) != 0 || written != len) {
-   error("Unable to write to %s", lockpath);
-   goto error_unlink_return;
-   }
-   if (rename(lockpath, ref_path) < 0) {
-   error("Unable to create %s", ref_path);
-   goto error_unlink_return;
-   }
-   if (adjust_shared_perm(ref_path)) {
-   error("Unable to fix permissions on %s", lockpath);
-   error_unlink_return:
-   unlink_or_warn(lockpath);
-   error_free_return:
-   free(lockpath);
-   free(ref_path);
-   return -1;
-   }
-   free(lockpath);
-
-#ifndef NO_SYMLINK_HEAD
-   done:
-#endif
+static void update_symref_reflog(struct ref_lock *lock, const char *ref,
+const char *target, const char *logmsg)
+{
+   struct strbuf err = STRBUF_INIT;
+   unsigned char new_sha1[20];
if (logmsg && !read_ref(target, new_sha1) &&
-   log_ref_write(ref, old_sha1, new_sha1, logmsg, 0, &err)) {
+   log_ref_write(ref, lock->old_oid.hash, new_sha1, logm

[PATCH 3/4] create_symref: modernize variable names

2015-12-19 Thread Jeff King
Once upon a time, create_symref() was used only to point
HEAD at a branch name, and the variable names reflect that
(e.g., calling the path git_HEAD). However, it is much more
generic these days (and has been for some time). Let's
update the variable names to make it easier to follow:

  - `ref_target` is now just `ref`, matching the declaration
in `cache.h` (and hopefully making it clear that it is
the symref itself, and not the target).

  - `git_HEAD` is now `ref_path`; the on-disk path
corresponding to `ref`.

  - `refs_heads_master` is now just `target`; i.e., what the
symref points at. This term also matches what is in
the symlink(2) manpage (at least on Linux).

  - the buffer to hold the symref file's contents was simply
called `ref`. It's now `buf` (admittedly also generic,
but at least not actively introducing confusion with the
other variable holding the refname).

Signed-off-by: Jeff King 
---
This could actually be squashed in with patch 4, as most of the changed
instances just go away. I did find the new names easier to work with,
though, so maybe they make the diff for that patch easier.

 refs.h   |  2 +-
 refs/files-backend.c | 41 -
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/refs.h b/refs.h
index 7a04077..96a25ad 100644
--- a/refs.h
+++ b/refs.h
@@ -292,7 +292,7 @@ extern char *shorten_unambiguous_ref(const char *refname, 
int strict);
 /** rename ref, return 0 on success **/
 extern int rename_ref(const char *oldref, const char *newref, const char 
*logmsg);
 
-extern int create_symref(const char *ref, const char *refs_heads_master, const 
char *logmsg);
+extern int create_symref(const char *ref, const char *target, const char 
*logmsg);
 
 enum action_on_err {
UPDATE_REFS_MSG_ON_ERR,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c648b5e..6bfa139 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2811,58 +2811,57 @@ static int commit_ref_update(struct ref_lock *lock,
return 0;
 }
 
-int create_symref(const char *ref_target, const char *refs_heads_master,
- const char *logmsg)
+int create_symref(const char *ref, const char *target, const char *logmsg)
 {
char *lockpath = NULL;
-   char ref[1000];
+   char buf[1000];
int fd, len, written;
-   char *git_HEAD = git_pathdup("%s", ref_target);
+   char *ref_path = git_pathdup("%s", ref);
unsigned char old_sha1[20], new_sha1[20];
struct strbuf err = STRBUF_INIT;
 
-   if (logmsg && read_ref(ref_target, old_sha1))
+   if (logmsg && read_ref(ref, old_sha1))
hashclr(old_sha1);
 
-   if (safe_create_leading_directories(git_HEAD) < 0)
-   return error("unable to create directory for %s", git_HEAD);
+   if (safe_create_leading_directories(ref_path) < 0)
+   return error("unable to create directory for %s", ref_path);
 
 #ifndef NO_SYMLINK_HEAD
if (prefer_symlink_refs) {
-   unlink(git_HEAD);
-   if (!symlink(refs_heads_master, git_HEAD))
+   unlink(ref_path);
+   if (!symlink(target, ref_path))
goto done;
fprintf(stderr, "no symlink - falling back to symbolic ref\n");
}
 #endif
 
-   len = snprintf(ref, sizeof(ref), "ref: %s\n", refs_heads_master);
-   if (sizeof(ref) <= len) {
-   error("refname too long: %s", refs_heads_master);
+   len = snprintf(buf, sizeof(buf), "ref: %s\n", target);
+   if (sizeof(buf) <= len) {
+   error("refname too long: %s", target);
goto error_free_return;
}
-   lockpath = mkpathdup("%s.lock", git_HEAD);
+   lockpath = mkpathdup("%s.lock", ref_path);
fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
if (fd < 0) {
error("Unable to open %s for writing", lockpath);
goto error_free_return;
}
-   written = write_in_full(fd, ref, len);
+   written = write_in_full(fd, buf, len);
if (close(fd) != 0 || written != len) {
error("Unable to write to %s", lockpath);
goto error_unlink_return;
}
-   if (rename(lockpath, git_HEAD) < 0) {
-   error("Unable to create %s", git_HEAD);
+   if (rename(lockpath, ref_path) < 0) {
+   error("Unable to create %s", ref_path);
goto error_unlink_return;
}
-   if (adjust_shared_perm(git_HEAD)) {
+   if (adjust_shared_perm(ref_path)) {
error("Unable to fix permissions on %s", lockpath);
error_unlink_return:
unlink_or_warn(lockpath);
error_free_return:
free(lockpath);
-   free(git_HEAD);
+   free(ref_path);
return -1;
}
free(lockpath);
@@ -2870,13 +2869,13 @@ int creat

[PATCH 2/4] t1401: test reflog creation for git-symbolic-ref

2015-12-19 Thread Jeff King
The current code writes a reflog entry whenever we update a
symbolic ref, but we never test that this is so. Let's add a
test to make sure upcoming refactoring doesn't cause a
regression.

Signed-off-by: Jeff King 
---
 t/t1401-symbolic-ref.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index fbb4835..1f0dff3 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -98,4 +98,20 @@ test_expect_success 'symbolic-ref reports failure in exit 
code' '
test_must_fail git symbolic-ref HEAD refs/heads/whatever
 '
 
+test_expect_success 'symbolic-ref writes reflog entry' '
+   git checkout -b log1 &&
+   test_commit one &&
+   git checkout -b log2  &&
+   test_commit two &&
+   git checkout --orphan orphan &&
+   git symbolic-ref -m create HEAD refs/heads/log1 &&
+   git symbolic-ref -m update HEAD refs/heads/log2 &&
+   cat >expect <<-\EOF &&
+   update
+   create
+   EOF
+   git log --format=%gs -g >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.7.0.rc1.350.g9acc0f4

--
To unsubscribe from this list: send the line "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/4] symbolic-ref: propagate error code from create_symref()

2015-12-19 Thread Jeff King
If create_symref() fails, git-symbolic-ref will still exit
with code 0, and our caller has no idea that the command did
nothing.

This appears to have been broken since the beginning of time
(e.g., it is not a regression where create_symref() stopped
calling die() or something similar).

Signed-off-by: Jeff King 
---
 builtin/symbolic-ref.c  | 2 +-
 t/t1401-symbolic-ref.sh | 6 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index ce0fde7..9c29a64 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -67,7 +67,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char 
*prefix)
if (!strcmp(argv[0], "HEAD") &&
!starts_with(argv[1], "refs/"))
die("Refusing to point HEAD outside of refs/");
-   create_symref(argv[0], argv[1], msg);
+   ret = !!create_symref(argv[0], argv[1], msg);
break;
default:
usage_with_options(git_symbolic_ref_usage, options);
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index 20b022a..fbb4835 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -92,4 +92,10 @@ test_expect_success LONG_REF 'we can parse long symbolic 
ref' '
test_cmp expect actual
 '
 
+test_expect_success 'symbolic-ref reports failure in exit code' '
+   test_when_finished "rm -f .git/HEAD.lock" &&
+   >.git/HEAD.lock &&
+   test_must_fail git symbolic-ref HEAD refs/heads/whatever
+'
+
 test_done
-- 
2.7.0.rc1.350.g9acc0f4

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


[PATCH 0/4] improve symbolic-ref robustness

2015-12-19 Thread Jeff King
I noticed that an interrupt "git symbolic-ref" will not clean up
"HEAD.lock". So I started this series as an attempt to convert
create_symref() to "struct lock_file" to get the usual tempfile cleanup.

But I found a few other points of interest. The biggest is that
git-symbolic-ref does not actually propagate errors in its exit code.
Whoops. That's fixed in the first patch, which could go separately to
"maint".

The rest of it is fairly dependent on master because of the
refs/files-backend.c code movement. I can backport it if we really want
it for maint.

  [1/4]: symbolic-ref: propagate error code from create_symref()
  [2/4]: t1401: test reflog creation for git-symbolic-ref
  [3/4]: create_symref: modernize variable names
  [4/4]: create_symref: use existing ref-lock code

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


Re: Forcing git to pack objects

2015-12-19 Thread Mikael Magnusson
On Sat, Dec 19, 2015 at 2:03 AM, Edmundo Carmona Antoranz
 wrote:
> Hi!
>
> Recently I was running manually a git gc --prune command (wanted to
> shrink my 2.8G .git directory by getting rid of loose objects) and I
> ended up running out of space on my HD. After freaking out a little
> bit (didn't know if the repo would end up in a 'stable' state), I
> ended up freeing up some space and I again have a working repo...
> _but_ I noticed that basically _all_ objects on my repo are laying
> around in directories .git/objects/00 to ff (and taking a whole lot of
> space... like the .git directory is now like 5 GBs). After running git
> gc manually again it ended up taking a lot of time and the objects are
> still there. Also git svn sometimes gcs after fetching and it took to
> run cause of the gc execution (ended up killing it) and the files are
> still there. Is it possible to ask git to put all those objects in
> .pack files? Or did I mess something on my repo?
>
> Just in case, that's a repo I use at work that's working on a windows
> box (git for windows 2.6.3).
>
> Thanks in advance.

git repack -d should do it.

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


Possible bug in "git mv"

2015-12-19 Thread David Hirsch
I think this might be a bug, albeit a minor one:

If you have a folder containing only files not tracked by git, and you try to 
use “git mv” to relocate it, then (at least on my machine, running git version 
2.6.3), I get this error message:

"fatal: source directory is empty, source=SRCDIR, destination=DESTDIR”

when a better message might be something like:

“no files in directory are under version control, source=SRCDIR, 
destination=DESTDIR”

David Hirsch
dhir...@me.com
http://davehirsch.com

-BEGIN PGP PUBLIC KEY BLOCK-
Comment: GPGTools - https://gpgtools.org

mQINBFZHkzsBEAC3tDADcJzSuCsnYLkegPmKgiOIq2gPFh1KmyZlfWFEDAZn5pkK
p4GB71kajYiBgVrqjpXBU15qPHh3ErYyp+dL2ouwsrC1ksX3TAj2DTIUMn8OdS4I
MxGhBWUnh7TSlKteW9NOKvlGPzRKiVyQHLuxXohEl/4grDZy/8dNy2A44oWZBEn1
DVBiluAo0ZFXWGfEOCIDqHH8PRUppWSZHfqildRdr9zXra4G7Urwy6CARkEwaRQV
d9PXBWFz/JCaQ59V8TQO9LhZ+79glV3tT8+wxdxmjabsVblBRzoYYJUULcdd+t7l
6wLCbbYehzCHc+5UMJYWjha9svdHZatJJw9PDkPB5XPmOJ1P+2gW2UqVyg6d9T9N
j+Kngn5EsxEniujIVdJ0nPe4+SNYkGTjy54TiMh73tsxjWOTNABL1IyUid9D38Hj
Gb5VNfWnM/5XvS3KoKGi/jmmQAtp5kWD5ANAyMsCLs5+u3fVdG8zKUBULlUdGEHp
EVw86VZJzCWOCMteQC9cjOKL6zTTjzj6SjK5k/uqbuvSO9Vw13QY2mJVIKJB8FxF
PEaRT2j9xQGluy3SogDS6w7iGgFeCV5YS9B6Fe9bUOBXy3XmXuE/5wvG8JdfdkK1
5wRVvzpQRlHLieIYrFDCKjlHxFL9zRSQPg0S9aEerLAKgPaPCU0ge1D4ywARAQAB
tB1EYXZpZCBIaXJzY2ggPGRoaXJzY2hAbWUuY29tPokCQAQTAQoAKgIbAwUJB4Yf
gAULCQgHAwUVCgkICwUWAgMBAAIeAQIXgAUCVnJsMwIZAQAKCRAlEfH3t4I6Reb8
D/9ejNbsjkba0b0Ue5eHKv39SI0+uGjan+yA8Y+NdS23nu91+7qMU6kOO5QtS37j
LzNZaR6vLDqVDIGkdvnIUaf4ot4MkKZE3YJlPoxq4vZvazJRacJFn+e6peV71vhb
S4aoUUtYeXBIYT2j7fB//e0eZwlRrtuXHDIAMpxAMV1X7JLVtvylzJgnzG/BuypW
GorPd3YX2tlj4HmLB1YxNvw1qxuU779O52es2Hp2a69Fi1OzxtPN706n/17W2WX/
9x8fWglxPeCubD1ozkKr32RUX9q70go6TDo/6KmPwBzVQJaMkPyC8/nugwku6p/E
GVDvNlZmSuaMK+BXnJrqn57/jSmkLIHtSBgcu1MwfyuAwmrYddAfFiB+S2o0RwNT
HuUyXAilHZZcySHXrI0bs9S+om1SxCtUUBGTfsbZwg7sK70RoCuhLmGGcRAKMKJB
zws/fENW1jAtSB2pqEbhhUhp1ym5m85A65rrApBxjIPFyDcq8XyooGGOXgGdbDpc
3E7CCCfhoIPfpRpY4eFb38XWUXonZxuSeDaptbydGFRLFuPVB5rE8f0o5anTKfXz
rhhyJzA8m5HPWSMOKtza1d0GdeV86IZYcqte4DSxlKVof2acWq3w+dn6mms3moqq
NTD+66x/pwmRUpmFwpSVVHAU2hGnr3/hVMO1NNgEuiQAr7QmRGF2aWQgSGlyc2No
IDxkYXZlLm0uaGlyc2NoQGdtYWlsLmNvbT6JAj0EEwEKACcFAlZybDICGwMFCQeG
H4AFCwkIBwMFFQoJCAsFFgIDAQACHgECF4AACgkQJRHx97eCOkXkaw/+KmOBKKFv
4cbnEAXnUasFmR4hv7uTrHRiFTLHAGtwTdaoefeoNG8BwtG/ChomKMU0/69Yyezk
W/9v8RciNpz05uKNAEa+Vc7kPEzHOl17lkIIkPAOVSSVSw/2cJz22TgG9TSsgR66
pwaan1GCHoO8ULmFJWbrKMc9ds2BmgYrdO+1aDbwTlD7FklKjQAI2aZk/tYpsEkM
Po3wFdSsazb2xfLGv7kbEXIRguI2lvhT9/rHxWm7MBN7iUOVorQsif5c1iuHDZZG
nuLZZGKdZUVCHG4ieoItIFhDzyiWKS8ZaQ2MsUWj8DC4KiPfacVi1BVkezTtk7ru
TAKjDN0uTpbfCaUWrU1oi2h36ua2QqwmQ9dH1ns2uipiwZ1ss/dGxpVxDGIVppBg
xPWCLNjhh+jwNAfX0NLPUyQIwDUXCNXNTAcsiWMnoyEgY3TvKsEURPFt6DLEa58r
sz0oLfBIDwgS8ig5EGCJ/hZL56FwmuBnpvTloz1UuBCDYXfpzzAtd3h6IQSHV9TN
nj4rzFc8OGB5c8bDXAuLWQwxfeIqywywpE6RThaxszjZC7iYH9KjIzzL0EOhKB4t
qloq34UYhEUJUrFi7qNBv6UevDpbGRFLOmiuC0mw5U4yM78DxCPbdS59hcRQCRI6
zQfeZ8zQjlRv9pQQB4hBT31HKUllyR/pTrC0HkRhdmlkIEhpcnNjaCA8ZGhpcnNj
aEBtYWMuY29tPokCPQQTAQoAJwUCVnJsQAIbAwUJB4YfgAULCQgHAwUVCgkICwUW
AgMBAAIeAQIXgAAKCRAlEfH3t4I6RYrxD/912kgauUIamAimoL8MGL2htGgcX1wA
rapuM/zA5VHIkcKy6DRLBfCH+As3tur8okgWszg4hC4zsOJ6Er+68aw6IruvjDoF
Hi65UPpm7yP6pMBLsOdme/U+oJDBU35mcrn8Dj4wcg9tsSYbbBmfe0lfSvCEY2+y
N++7EAhIgeo7rZOljdUzeZsa6xYxzWOFXiC+zkLhlSmeuy4/cr0WhCtzL7KKJTDm
XK7R+bPUzXUMdxhWqTxX97+D4sfdLU+XDsh1wSWBPLx960E7lovo1SzvAjZlv8MH
dnv2J8CVYZMKHporYXerS070T1pTh0OstAl76VspYKZpqSyMMa2Kfb0N1OxQ4FKX
FWGRcfEMsP8GQUp+eOgQYpEfbH6bDa440FW3yw7/BGT5Klqeg7DU3sOyf3lBMR3A
aVR+UDTwzW72X3OxUmves5sfxL+La4tzv9TapR6RmhuNRGAosfZb3xnlz3haq63i
WfauqmXadoSSi90g5dU072tQ2frarhZ1cypWO0FFpZZ4o4QHlihRUaxk0H53AbjG
lP1UdVu7+0GD0ELlN+ECJJF8BeS2aZENO1CjOmcR5UtZJ1fAcL+N0LfZRnAy0reK
ixJSD7lSfEUzc9iNag1AFEIamB0J0jAw8mLUXTENi6F8jA6fKONbLGcwfuVEfiil
Xj14qqUPbexyMLQhRGF2aWQgSGlyc2NoIDxkaGlyc2NoQGljbG91ZC5jb20+iQI9
BBMBCgAnBQJWcmxVAhsDBQkHhh+ABQsJCAcDBRUKCQgLBRYCAwEAAh4BAheAAAoJ
ECUR8fe3gjpFTFoQAInKZuqVchH6H+qHVbDlgwcPnnUFCX4vfsVBOc8JsFjV1+0p
VNYO6JEyX/2pjEp2krZl4dRTEKQC30XbZ2azmmvf/d128VLtLnr3b3QbSg6Xmfwp
AhtQO8lap0k4KEKbRLEcCFDvIhNqXWpowdzsw+BPZd+lizKOG+iLkheXVMCUU8VA
Labh1AIPA9XXBke9rwoxR2S7UYmiNoOjCmJdIGiRxz8QvSmhOT5C/Z7PBk1kik2j
RhVbjjaPybhd53v8oXeOy3kBKUoeXOrG8V4GTJw1jHNJNn7cCYqiVt1TXFwkbcVU
iom9bMoSwsJIM3Q9OqLFhF6CyZS5VFkW4J+uW+095+1O/g3chVTDsPpoFIaZlE3J
jAgwSwRPPNt2nkChH7asyDjfDu9J+wLATR6rjY1n7vHZBEeDKsHzZvnc6fC59XlE
1+pUjU9I0RvskHTPyh5ZSnJIvkbCk8cWPZoaCOTfdMS0dURV20vtIc+8kShUhEfF
7FC1PqIVDfvpxaSbhD7rlI/VmXyhlJFwdHu5FdFbL3AJDMiOuP2F6wZGmXR/Dnxk
ASH3eBXnNME/UeTe6oMIqNW1sUV74FZWNEwplhIpiOc4K8dMYs2klUst33G9aBK/
XHESgep560E6E5+mbEWupWUWuK7Us8+f5m2OkmIZKiSmgPP2BtjsgBZH76xpuQIN
BFZHkzsBEADWUb1ivnyi1M0FVx03meDraXpWlualKEgdg3/1tG7g8HB+6XP7avFD
9vIBlZ+poTUOsbhzXqpI2yBdyGGwwoku2I5X0WdqTCvBYYxyKz6/b4mFoiZub6tw
8peYDJjfknJhCrXrgPYVNxjFd7nvZ5eayq4uxS7lB9iRQiBPrnTh5UZ6HjmXgHvS
begbaDT2ijjbKLiRSCMp1xKCKTTKodqYnQN2mp5YbHP6cJx30GeKChzo+lFQfwy5
OIvYxP0ezKeZRYefopEemdIkhf112SFyInaQJptpmhEGFYxwFJCUoAlzXwT0sa8j
7Gy2IU9NNJdd7mMs6vIP79v+GYRg/4tRBqk1q+tQSkdF4C0820wTDwRaqPYIV/

Re: [RFC] Malicously tampering git metadata?

2015-12-19 Thread Theodore Ts'o
On Sat, Dec 19, 2015 at 12:30:18PM -0500, Santiago Torres wrote:
> > Now, the crazy behavior where github users randomly and promiscuously
> > do pushes and pull without doing any kind of verification may very
> > well be dangerous. 
> 
> Yes, we were mostly familiar with this workflow before starting this
> research. I can see how the "github generation" is open to many attacks
> of this nature. Would git be interested in integrating a defense that
> covers users of this nature (which seems to be a growing userbase)?

One of the interesting challenges is that git is a pretty low-level
tool, and so people have built all sorts of different workflows on top
of it.

For example, at $WORK, we use gerrit, which is a code review tool, so
all git commits that are to be merged into the "upstream" repository
0gets pushed to a gerrit server, where it goes through a code review
process where a second engineer can review the code, request changes,
make comments, or ask questions, and where the git commits can go
through multiple rounds of review / revision before they are finally
accepted (at least one reviewer must give a +2 review, and there must
be no -2 reviews; and there can be automated tools that do build or
regression tests that can give automated -1 or -2 reviews) --- and
where all of the information collected during the code review process
is saved as part of the audit trail for a Sarbanes-Oxley (SOX)
compliance regime.

Other people use github-style workflows, and others use signed tags
with e-mail code reviews, etc.  And I'm sure there must be many others.

So the challenge is that in order to accomodate many possible
workflows, some of which use third-party tools, changes to make git
more secure for one workflow must not get in the way of these other
workflows --- which means that enforcement of new controls for the
"github generation" probably will have to be optional.  But then
people belonging to the "github generation" can also easily turn off
these features.  And as the NSA learned the hard way in Vietnam, if
the tools cause any inconenience, or has the perception of
constraining legitmate users, security features can have a way of
getting turned off.[1]

[1] A History of US Communications Security, The David G. Boak
lectures, Volume II, "Nestor in Vietname".  pg 43-44.  (A declassified
version can be found at:
http://www.governmentattic.org/18docs/Hist_US_COMSEC_Boak_NSA_1973u.pdf)

> > But so is someone who saves a 80 patch series from
> > their inbox, and without reading or verifying all of the patches
> > applies them blindly to their tree using "git am" --- or if they were
> > using cvs or svn, bulk applied the patches without doing any
> > verification
> 
> Just out of curiosity, are there known cases of projects in which this
> has happened (I've noticed that both Git and Linux are quite stringent
> in their review/merge process so this wouldn't be the case).

I can't point at specific instances, but given that in the "github
generation", people are fine with blindly pulling someone else's
Docker images and running it on their production servers or
workstations, and where software installation gets done with "wget
http://example.org | bash" or the equivalent, that it's probably more
often than we might be comfortable.

I also suspect that a bad guy would probably find inserting a
man-in-the-middle server into one of these installation flows is
probably a much more practical attack in terms of real world
considerations.  :-)

Cheers,

- Ted
--
To unsubscribe from this list: send the line "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] gitk: Update Bulgarian translation (311t)

2015-12-19 Thread Alexander Shopov
Signed-off-by: Alexander Shopov 
---
 po/bg.po | 656 ---
 1 file changed, 336 insertions(+), 320 deletions(-)

diff --git a/po/bg.po b/po/bg.po
index 909a564..99aa77a 100644
--- a/po/bg.po
+++ b/po/bg.po
@@ -8,8 +8,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: gitk master\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-06-27 20:44+0300\n"
-"PO-Revision-Date: 2015-06-27 20:46+0300\n"
+"POT-Creation-Date: 2015-12-19 11:48+0200\n"
+"PO-Revision-Date: 2015-12-19 11:49+0200\n"
 "Last-Translator: Alexander Shopov \n"
 "Language-Team: Bulgarian \n"
 "Language: bg\n"
@@ -22,11 +22,11 @@ msgstr ""
 msgid "Couldn't get list of unmerged files:"
 msgstr "Списъкът с неслети файлове не може да бъде получен:"
 
-#: gitk:212 gitk:2381
+#: gitk:212 gitk:2399
 msgid "Color words"
 msgstr "Оцветяване на думите"
 
-#: gitk:217 gitk:2381 gitk:8220 gitk:8253
+#: gitk:217 gitk:2399 gitk:8239 gitk:8272
 msgid "Markup words"
 msgstr "Отбелязване на думите"
 
@@ -59,15 +59,15 @@ msgstr "Грешка при изпълнение на „git log“:"
 msgid "Reading"
 msgstr "Прочитане"
 
-#: gitk:496 gitk:4525
+#: gitk:496 gitk:4544
 msgid "Reading commits..."
 msgstr "Прочитане на подаванията…"
 
-#: gitk:499 gitk:1637 gitk:4528
+#: gitk:499 gitk:1637 gitk:4547
 msgid "No commits selected"
 msgstr "Не са избрани подавания"
 
-#: gitk:1445 gitk:4045 gitk:12432
+#: gitk:1445 gitk:4064 gitk:12469
 msgid "Command line"
 msgstr "Команден ред"
 
@@ -79,286 +79,294 @@ msgstr "Изходът от „git log“ не може да се анализи
 msgid "No commit information available"
 msgstr "Липсва информация за подавания"
 
-#: gitk:1903 gitk:1932 gitk:4315 gitk:9669 gitk:11241 gitk:11521
+#: gitk:1903 gitk:1932 gitk:4334 gitk:9702 gitk:11274 gitk:11554
 msgid "OK"
 msgstr "Добре"
 
-#: gitk:1934 gitk:4317 gitk:9196 gitk:9275 gitk:9391 gitk:9440 gitk:9671
-#: gitk:11242 gitk:11522
+#: gitk:1934 gitk:4336 gitk:9215 gitk:9294 gitk:9424 gitk:9473 gitk:9704
+#: gitk:11275 gitk:11555
 msgid "Cancel"
 msgstr "Отказ"
 
-#: gitk:2069
+#: gitk:2083
 msgid "&Update"
-msgstr "Обновяване"
+msgstr "&Обновяване"
 
-#: gitk:2070
+#: gitk:2084
 msgid "&Reload"
-msgstr "Презареждане"
+msgstr "&Презареждане"
 
-#: gitk:2071
+#: gitk:2085
 msgid "Reread re&ferences"
-msgstr "Наново прочитане на настройките"
+msgstr "&Наново прочитане на настройките"
 
-#: gitk:2072
+#: gitk:2086
 msgid "&List references"
-msgstr "Изброяване на указателите"
+msgstr "&Изброяване на указателите"
 
-#: gitk:2074
+#: gitk:2088
 msgid "Start git &gui"
-msgstr "Стартиране на „git gui“"
+msgstr "&Стартиране на „git gui“"
 
-#: gitk:2076
+#: gitk:2090
 msgid "&Quit"
-msgstr "Спиране на програмата"
+msgstr "&Спиране на програмата"
 
-#: gitk:2068
+#: gitk:2082
 msgid "&File"
-msgstr "Файл"
+msgstr "&Файл"
 
-#: gitk:2080
+#: gitk:2094
 msgid "&Preferences"
-msgstr "Настройки"
+msgstr "&Настройки"
 
-#: gitk:2079
+#: gitk:2093
 msgid "&Edit"
-msgstr "Редактиране"
+msgstr "&Редактиране"
 
-#: gitk:2084
+#: gitk:2098
 msgid "&New view..."
-msgstr "Нов изглед…"
+msgstr "&Нов изглед…"
 
-#: gitk:2085
+#: gitk:2099
 msgid "&Edit view..."
-msgstr "Редактиране на изгледа…"
+msgstr "&Редактиране на изгледа…"
 
-#: gitk:2086
+#: gitk:2100
 msgid "&Delete view"
-msgstr "Изтриване на изгледа"
+msgstr "&Изтриване на изгледа"
 
-#: gitk:2088 gitk:4043
+#: gitk:2102
 msgid "&All files"
-msgstr "Всички файлове"
+msgstr "&Всички файлове"
 
-#: gitk:2083 gitk:4067
+#: gitk:2097
 msgid "&View"
-msgstr "Изглед"
+msgstr "&Изглед"
 
-#: gitk:2093 gitk:2103 gitk:3012
+#: gitk:2107 gitk:2117
 msgid "&About gitk"
-msgstr "Относно gitk"
+msgstr "&Относно gitk"
 
-#: gitk:2094 gitk:2108
+#: gitk:2108 gitk:2122
 msgid "&Key bindings"
-msgstr "Клавишни комбинации"
+msgstr "&Клавишни комбинации"
 
-#: gitk:2092 gitk:2107
+#: gitk:2106 gitk:2121
 msgid "&Help"
-msgstr "Помощ"
+msgstr "Помо&щ"
 
-#: gitk:2185 gitk:8652
+#: gitk:2199 gitk:8671
 msgid "SHA1 ID:"
 msgstr "SHA1:"
 
-#: gitk:2229
+#: gitk:2243
 msgid "Row"
 msgstr "Ред"
 
-#: gitk:2267
+#: gitk:2281
 msgid "Find"
 msgstr "Търсене"
 
-#: gitk:2295
+#: gitk:2309
 msgid "commit"
 msgstr "подаване"
 
-#: gitk:2299 gitk:2301 gitk:4687 gitk:4710 gitk:4734 gitk:6755 gitk:6827
-#: gitk:6912
+#: gitk:2313 gitk:2315 gitk:4706 gitk:4729 gitk:4753 gitk:6774 gitk:6846
+#: gitk:6931
 msgid "containing:"
 msgstr "съдържащо:"
 
-#: gitk:2302 gitk:3526 gitk:3531 gitk:4763
+#: gitk:2316 gitk:3545 gitk:3550 gitk:4782
 msgid "touching paths:"
 msgstr "засягащо пътищата:"
 
-#: gitk:2303 gitk:4777
+#: gitk:2317 gitk:4796
 msgid "adding/removing string:"
 msgstr "добавящо/премахващо низ"
 
-#: gitk:2304 gitk:4779
+#: gitk:2318 gitk:4798
 msgid "changing lines matching:"
 msgstr "променящо редове напасващи:"
 
-#: gitk:2313 gitk:2315 gitk:4766
+#: gitk:2327 gitk:2329 gitk:4785
 msgid "Exact"
 msgstr "Точно"
 
-#: gitk:2315 gitk:4854 gitk:6723
+#: gitk:2329 gitk:4873 gitk:6742
 msgid "IgnCase"
 msgstr "Без регистър"
 
-#: gitk:2315 gitk:4736 gitk:4852 gitk:

Re: git leaves behind .git/COMMIT_EDITMSG non-shared in --shared non-bare repo

2015-12-19 Thread Johannes Schindelin
Hi Gábor,

On Sat, 19 Dec 2015, SZEDER Gábor wrote:

> > On Fri, 18 Dec 2015, Yaroslav Halchenko wrote:
> > 
> > > Not sure for what batch operations that file is actually useful,
> > 
> > None. This file is written when you commit interactively. It is deleted
> > afterwards, unless aborted in a fatal manner.
> 
> Is it?  I have a COMMIT_EDITMSG lying around in just about every git
> repository I have and couldn't find any unlink() in builtin/commit.c
> or elsewhere that would remove it.

Oops. My mistake. Patch sent.

Ciao,
Dscho

[PATCH] commit: ensure correct permissions of the commit message

2015-12-19 Thread Johannes Schindelin
It was pointed out by Yaroslav Halchenko that the file containing the
commit message had the wrong permissions in a shared setting.

Let's fix that.

Signed-off-by: Johannes Schindelin 
---
 builtin/commit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index d054f84..3bfd457 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -905,6 +905,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
strbuf_release(&committer_ident);
 
fclose(s->fp);
+   adjust_shared_perm(git_path(commit_editmsg));
 
/*
 * Reject an attempt to record a non-merge empty commit without
-- 
2.6.3.windows.1.300.g1c25e49
--
To unsubscribe from this list: send the line "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: [RFC] Malicously tampering git metadata?

2015-12-19 Thread Santiago Torres
> I assume you are assuming here that the "push to upstream" doesn't
> involve some kind of human verification?  If someone tried pushing
> something like this to Linus, he would be checking the git diff stats
> and git commit structure for things that might look like "developer
> negligence".  He's been known to complain to subsystem developers in
> his own inimitable when the git commit structure looks suspicious, so
> I'm pretty sure he would notice this.
> 
> For example, in my use case, I'm authorative over changes in fs/ext4.
> So when I pull from Linus's repo, I examine (using "gitk fs/ext4") all
> commits coming from upstream that modify fs/ext4.  So if someone tries
> introducing a change in fs/ext4 coming from "upstream", I will notice.
> Then when I request a pull request from Linus, the git pull request
> describes what commits are new in my tree that are not in his, and
> shows the diffstats from upstream.  When Linus verifies my pull, there
> are multiple opportunities where he will notice funny business:
> 
> a) He would notice that my origin commit is something that is not in
> his upstream tree.
> 
> b) His diffstat is different from my diffstat (since thanks to the
> man-in-the middle, the conception of what commits are new in the git
> pull request will be different from his).
> 
> c) His diffstat will show that files outside of fs/ext4 have been
> modified, which is a red flag that will merit more close examination.
> (And if the attacker had tried to introduce a change in fs/ext4, I
> would have noticed when I pulled from the man-in-the-middle git
> repo.)

Yes, we've been considering that these kind of attacks wouldn't be too
effective against, let's say, dictator-lieutenant workflows. 

> 
> Now, the crazy behavior where github users randomly and promiscuously
> do pushes and pull without doing any kind of verification may very
> well be dangerous. 

Yes, we were mostly familiar with this workflow before starting this
research. I can see how the "github generation" is open to many attacks
of this nature. Would git be interested in integrating a defense that
covers users of this nature (which seems to be a growing userbase)?

> But so is someone who saves a 80 patch series from
> their inbox, and without reading or verifying all of the patches
> applies them blindly to their tree using "git am" --- or if they were
> using cvs or svn, bulk applied the patches without doing any
> verification

Just out of curiosity, are there known cases of projects in which this
has happened (I've noticed that both Git and Linux are quite stringent
in their review/merge process so this wouldn't be the case).

> 
> Cheers,

Thanks for the insight!
-Santiago.
--
To unsubscribe from this list: send the line "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: Odd rebase behavior

2015-12-19 Thread John Keeping
On Fri, Dec 18, 2015 at 06:05:49PM +, John Keeping wrote:
> On Fri, Dec 18, 2015 at 11:43:16AM -0600, David A. Greene wrote:
> > John Keeping  writes:
> > 
> > > It seems that the problem is introduces by --preserve-merges (and
> > > -Xsubtree causes something interesting to happen as well).  I see the
> > > following behaviour:
> > 
> > Thanks for narrowing this down!  Is it possible this is actually a
> > cherry-pick problem since --preserve-merges forces rebase to use
> > cherry-pick?
> 
> I'm pretty sure this a result of the code in git-rebase--interactive.sh
> just below the comment "Watch for commits that have been dropped by
> cherry-pick", which filters out certain commits.  However, I'm not at
> all familiar with the --preserve-merges code in git-rebase so I could be
> completely wrong.

I've traced through git-rebase--interactive.sh and I can see what's
happening here now.  The problematic code is around the handling of the
"rewritten" directory.

In --preserve-merges mode, we write the SHA1 of the "onto" commit into
rewritten and then add any commits descended from it along the
first-parent path that we have identified as candidates for being
rebased.  This allows us to identify commits that have been merged in
and remove them from the rebase instruction list.

Because the right-hand commit in this case is disjoint from "onto", we
end up dropping everything at this point.  The --root option fixes this
because instead of preserving just "onto", it adds all of the commits
given by:

git merge-base --all $orig_head $upstream

Since the disjoint history causes the root commit to be rewritten, I
think requiring --root for this case to work is reasonable.  However,
I'm not sure we should add it automatically.  It may be better to detect
that there is no common history and die if --root has not been given.
--
To unsubscribe from this list: send the line "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 leaves behind .git/COMMIT_EDITMSG non-shared in --shared non-bare repo

2015-12-19 Thread Yaroslav Halchenko
fwiw that file is created not only by interactive tasks by with a regular 
commit -m msg as well.

On December 19, 2015 5:40:43 AM EST, Johannes Schindelin 
 wrote:
>Hi Yaroslav,
>
>On Fri, 18 Dec 2015, Yaroslav Halchenko wrote:
>
>> Not sure for what batch operations that file is actually useful,
>
>None. This file is written when you commit interactively. It is deleted
>afterwards, unless aborted in a fatal manner.
>
>So I would try to find out who the heck is working interactively
>without
>finishing their commit message in that shared directory.
>
>Ciao,
>Johannes

-- 
Sent from a phone which beats iPhone.
--
To unsubscribe from this list: send the line "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 leaves behind .git/COMMIT_EDITMSG non-shared in --shared non-bare repo

2015-12-19 Thread SZEDER Gábor
> On Fri, 18 Dec 2015, Yaroslav Halchenko wrote:
> 
> > Not sure for what batch operations that file is actually useful,
> 
> None. This file is written when you commit interactively. It is deleted
> afterwards, unless aborted in a fatal manner.

Is it?  I have a COMMIT_EDITMSG lying around in just about every git
repository I have and couldn't find any unlink() in builtin/commit.c
or elsewhere that would remove it.  In fact, not deleting it seems to
be the desired bahavior, because many tests in t7502-commit.sh depend
on this file still being present after a successful 'git commit'.

Gábor

--
To unsubscribe from this list: send the line "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 07/11] ref-filter: introduce align_atom_parser()

2015-12-19 Thread Karthik Nayak
On Fri, Dec 18, 2015 at 11:20 AM, Eric Sunshine  wrote:
> On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak  wrote:
>> Introduce align_atom_parser() which will parse an "align" atom and
>> store the required alignment position and width in the "use_atom"
>> structure for further usage in 'populate_value()'.
>>
>> Helped-by: Ramsay Jones 
>> Signed-off-by: Karthik Nayak 
>> ---
>> diff --git a/ref-filter.c b/ref-filter.c
>> @@ -63,6 +69,49 @@ static void color_atom_parser(struct used_atom *atom)
>> +static void align_atom_parser(struct used_atom *atom)
>> +{
>> +   [...]
>> +   match_atom_name(atom->str, "align", &buf);
>> +   if (!buf)
>> +   die(_("expected format: %%(align:,)"));
>> +   [...]
>> +}
>> @@ -880,35 +924,7 @@ static void populate_value(struct ref_array_item *ref)
>> v->s = " ";
>> continue;
>> } else if (match_atom_name(name, "align", &valp)) {
>
> Hmm, align_atom_parser() has already been called before we get here,
> right? And it has already invoked match_atom_name() and died if the
> argument to "align:" was missing, correct? If so, then this invocation
> of match_atom_name() in populate_value() is unnecessary and should be
> replaced with a simpler starts_with("align:").
>
> Plus, 'valp' is not used, aside from this unnecessary
> match_atom_name(), thus it can be removed as well.
>

WIll do. Thanks for noticing this. I missed it.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "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 07/11] ref-filter: introduce align_atom_parser()

2015-12-19 Thread Karthik Nayak
On Thu, Dec 17, 2015 at 3:24 AM, Eric Sunshine  wrote:
> On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak  wrote:
>> Introduce align_atom_parser() which will parse an "align" atom and
>> store the required alignment position and width in the "use_atom"
>> structure for further usage in 'populate_value()'.
>>
>> Helped-by: Ramsay Jones 
>> Signed-off-by: Karthik Nayak 
>> ---
>> diff --git a/ref-filter.c b/ref-filter.c
>> @@ -63,6 +69,49 @@ static void color_atom_parser(struct used_atom *atom)
>> +static align_type parse_align_position(const char *s)
>> +{
>> +   if (!strcmp(s, "right"))
>> +   return ALIGN_RIGHT;
>> +   else if (!strcmp(s, "middle"))
>> +   return ALIGN_MIDDLE;
>> +   else if (!strcmp(s, "left"))
>> +   return ALIGN_LEFT;
>> +   return -1;
>> +}
>> +
>> +static void align_atom_parser(struct used_atom *atom)
>> +{
>> +   struct align *align = &atom->u.align;
>> +   const char *buf = NULL;
>> +   struct strbuf **s, **to_free;
>> +   int width = -1;
>> +
>> +   match_atom_name(atom->str, "align", &buf);
>> +   if (!buf)
>> +   die(_("expected format: %%(align:,)"));
>> +   s = to_free = strbuf_split_str_omit_term(buf, ',', 0);
>> +
>> +   align->position = ALIGN_LEFT;
>> +
>> +   while (*s) {
>> +   int position;
>> +
>> +   if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)&width))
>
> This casting is pretty ugly. It probably would be cleaner to declare
> 'width' as 'unsigned int' and initialize it with ~0U than to do this
> ugly and potentially dangerous casting. Likewise, below where you
> check 'width < 0', you'd check instead for ~0U. However, such a change
> should not be part of the current patch, but rather as a separate
> preparatory patch.
>

Will do.

>> +   ;
>> +   else if ((position = parse_align_position(s[0]->buf)) > 0)
>
> Shouldn't this be '>=' rather than '>'?
>
> This likely would have been easier to spot if parse_align_position()
> had been factored out in its own patch, as suggested by my v1
> review[1], since the caller would have been trivially inspectable
> rather than having to keep track of both code movement and changes.
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/281916
>

Yes. it should. Will split this into three patches.

1. Creation of align_atom_parser().
2. Introduce parse_align_position().
3. make 'width' an unsigned int.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "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 leaves behind .git/COMMIT_EDITMSG non-shared in --shared non-bare repo

2015-12-19 Thread Johannes Schindelin
Hi Yaroslav,

On Fri, 18 Dec 2015, Yaroslav Halchenko wrote:

> Not sure for what batch operations that file is actually useful,

None. This file is written when you commit interactively. It is deleted
afterwards, unless aborted in a fatal manner.

So I would try to find out who the heck is working interactively without
finishing their commit message in that shared directory.

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


[PATCHv2 0/3] git-p4: fixing p4ChangesForPaths

2015-12-19 Thread Luke Diamand
James Farwell found a bug in p4ChangesForPaths() when handling
changes across multiple depot paths.

Sam Hocevar had already submitted a change to the same function
to get P4 to do queries across all of the depot paths, in order
to reduce server queries, which had the side effect of fixing
James' problem.

This followup just fixes Sam's original fix to restore the
behavior of p4ChangesForPaths() so that it returns a sorted list
of changes. That fixes a failing a testcase.

Luke Diamand (1):
  git-p4: failing test case for skipping changes with multiple depots

Sam Hocevar (2):
  git-p4: support multiple depot paths in p4 submit
  git-p4: reduce number of server queries for fetches

 git-p4.py   | 55 +++--
 t/t9818-git-p4-block.sh | 28 -
 2 files changed, 57 insertions(+), 26 deletions(-)

-- 
2.6.2.474.g3eb3291

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


[PATCHv2 1/3] git-p4: failing test case for skipping changes with multiple depots

2015-12-19 Thread Luke Diamand
James Farwell reported that with multiple depots git-p4 would
skip changes.

http://article.gmane.org/gmane.comp.version-control.git/282297

Add a failing test case demonstrating the problem.

Signed-off-by: Luke Diamand 
---
 t/t9818-git-p4-block.sh | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 3b3ae1f..64510b7 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -84,7 +84,7 @@ p4_add_file() {
(cd "$cli" &&
>$1 &&
p4 add $1 &&
-   p4 submit -d "Added a file" $1
+   p4 submit -d "Added file $1" $1
)
 }
 
@@ -112,6 +112,32 @@ test_expect_success 'Syncing files' '
)
 '
 
+# Handling of multiple depot paths:
+#git p4 clone //depot/pathA //depot/pathB
+#
+test_expect_success 'Create a repo with multiple depot paths' '
+   client_view "//depot/pathA/... //client/pathA/..." \
+   "//depot/pathB/... //client/pathB/..." &&
+   mkdir -p "$cli/pathA" "$cli/pathB" &&
+   for p in pathA pathB
+   do
+   for i in $(test_seq 1 10)
+   do
+   p4_add_file "$p/file$p$i"
+   done
+   done
+'
+
+test_expect_failure 'Clone repo with multiple depot paths' '
+   (
+   cd "$git" &&
+   git p4 clone --changes-block-size=4 //depot/pathA@all 
//depot/pathB@all \
+   --destination=dest &&
+   ls -1 dest >log &&
+   test_line_count = 20 log
+   )
+'
+
 test_expect_success 'kill p4d' '
kill_p4d
 '
-- 
2.6.2.474.g3eb3291

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


[PATCHv2 3/3] git-p4: reduce number of server queries for fetches

2015-12-19 Thread Luke Diamand
From: Sam Hocevar 

When fetching changes from a depot using a full client spec, there
is no need to perform as many queries as there are top-level paths
in the client spec.  Instead we query all changes in chronological
order, also getting rid of the need to sort the results and remove
duplicates.

Signed-off-by: Sam Hocevar 
Signed-off-by: Luke Diamand 
---
 git-p4.py   | 44 +---
 t/t9818-git-p4-block.sh |  2 +-
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 122aff2..c33dece 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -822,39 +822,37 @@ def p4ChangesForPaths(depotPaths, changeRange, 
requestedBlockSize):
 die("cannot use --changes-block-size with non-numeric 
revisions")
 block_size = None
 
-# Accumulate change numbers in a dictionary to avoid duplicates
-changes = {}
+changes = []
 
-for p in depotPaths:
-# Retrieve changes a block at a time, to prevent running
-# into a MaxResults/MaxScanRows error from the server.
+# Retrieve changes a block at a time, to prevent running
+# into a MaxResults/MaxScanRows error from the server.
 
-while True:
-cmd = ['changes']
+while True:
+cmd = ['changes']
 
-if block_size:
-end = min(changeEnd, changeStart + block_size)
-revisionRange = "%d,%d" % (changeStart, end)
-else:
-revisionRange = "%s,%s" % (changeStart, changeEnd)
+if block_size:
+end = min(changeEnd, changeStart + block_size)
+revisionRange = "%d,%d" % (changeStart, end)
+else:
+revisionRange = "%s,%s" % (changeStart, changeEnd)
 
+for p in depotPaths:
 cmd += ["%s...@%s" % (p, revisionRange)]
 
-for line in p4_read_pipe_lines(cmd):
-changeNum = int(line.split(" ")[1])
-changes[changeNum] = True
+# Insert changes in chronological order
+for line in reversed(p4_read_pipe_lines(cmd)):
+changes.append(int(line.split(" ")[1]))
 
-if not block_size:
-break
+if not block_size:
+break
 
-if end >= changeEnd:
-break
+if end >= changeEnd:
+break
 
-changeStart = end + 1
+changeStart = end + 1
 
-changelist = changes.keys()
-changelist.sort()
-return changelist
+changes = sorted(changes)
+return changes
 
 def p4PathStartsWith(path, prefix):
 # This method tries to remedy a potential mixed-case issue:
diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
index 64510b7..8840a18 100755
--- a/t/t9818-git-p4-block.sh
+++ b/t/t9818-git-p4-block.sh
@@ -128,7 +128,7 @@ test_expect_success 'Create a repo with multiple depot 
paths' '
done
 '
 
-test_expect_failure 'Clone repo with multiple depot paths' '
+test_expect_success 'Clone repo with multiple depot paths' '
(
cd "$git" &&
git p4 clone --changes-block-size=4 //depot/pathA@all 
//depot/pathB@all \
-- 
2.6.2.474.g3eb3291

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


[PATCHv2 2/3] git-p4: support multiple depot paths in p4 submit

2015-12-19 Thread Luke Diamand
From: Sam Hocevar 

When submitting from a repository that was cloned using a client spec,
use the full list of paths when ruling out files that are outside the
view.  This fixes a bug where only files pertaining to the first path
would be included in the p4 submit.

Signed-off-by: Sam Hocevar 
Signed-off-by: Luke Diamand 
---
 git-p4.py | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 7a9dd6a..122aff2 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1458,6 +1458,8 @@ class P4Submit(Command, P4UserMap):
Remove lines in the Files section that show changes to files
outside the depot path we're committing into."""
 
+[upstream, settings] = findUpstreamBranchPoint()
+
 template = ""
 inFilesSection = False
 for line in p4_read_pipe_lines(['change', '-o']):
@@ -1470,8 +1472,13 @@ class P4Submit(Command, P4UserMap):
 lastTab = path.rfind("\t")
 if lastTab != -1:
 path = path[:lastTab]
-if not p4PathStartsWith(path, self.depotPath):
-continue
+if settings.has_key('depot-paths'):
+if not [p for p in settings['depot-paths']
+if p4PathStartsWith(path, p)]:
+continue
+else:
+if not p4PathStartsWith(path, self.depotPath):
+continue
 else:
 inFilesSection = False
 else:
-- 
2.6.2.474.g3eb3291

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