[Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!

2013-08-08 Thread Luke San Antonio
Hi, my name's Luke!

Today, I had a problem merging a stash after immediately creating it.
This is exactly what I did!

git stash save --keep-index
git stash pop

And BAM! Merge conflict! This was especially weird because my file had
this in it (taken directly from my code!)

<<< Updated upstream
 *
 * It should be used and passed along to member objects by GameStates!
 *
===
 *
 * It should be used and passed along to member objects by GameStates!
 *
>>> Stashed changes

They are exactly the same!

Oh, by the way, I should mention that I did not edit any hunks to get
the index the way I wanted it, I have read that doing that causes
merge conflicts similar to this!

Then I got a hunch! I realized that git will refrain from applying a
hunk if it finds it already was applied exactly (that's correct
right?)... So I thought, maybe the patches are similar (represent the
same changes) but aren't *exactly* the same.

I was right!

After saving the stash I take a look at the diff: (git stash show -p)

 /*!
- * \brief The default font renderer, global to all who have a pointer to
- * the Game class.
+ * \brief The font renderer implementation, obtained from the config file.
+ *
+ * It should be used and passed along to member objects by GameStates!
  *
- * It need not be used at all!
+ * \note It can be cached, but not between GameStates, meaning it should be
+ * cached again every time a new GameState is constructed!
  */

After that, I take a look at the diff in my index: (git diff --staged)

 /*!
- * \brief The default font renderer, global to all who have a pointer to
- * the Game class.
+ * \brief The font renderer implementation, obtained from the config file.
  *
- * It need not be used at all!
+ * It should be used and passed along to member objects by GameStates!
+ *
+ * \note It can be cached, but not between GameStates, meaning it should be
+ * cached again every time a new GameState is constructed!
  */

Aha! A difference, a difference so tiny it went unnoticed by me, but not by git!

Now the housekeeping:

What I wanted to do:

Apply a stash on top of a 'kept' index.

What I did:

git stash save --keep-index
git stash pop

What I saw happen:

A merge conflict between the same changes (see above).

What I expected to see:

No merge conflict.

How are these different:

The conflict, which shouldn't happen since the changes introduced were the same!

-

It seems to me like the stash command is using a slightly different
diff algorithm...
Can anyone explain to me what's going on under the hood so I can
understand this subtle difference? Does anyone know?

Thanks in advance, I'm sure you all will be very helpful!
- Luke
--
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 exproll: steps to tackle gc aggression

2013-08-08 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> Junio C Hamano wrote:
>> Imagine we have a cheap way to enumerate the young objects without
>> the usual history traversal.
>
> Before we discuss the advantages, can you outline how we can possibly
> get this data without actually walking downwards from the roots
> (refs)? One way to do it is to pull data out of a log of ref updates
> (aka. reflog), but we both know how unreliable that can be.

My understanding of the topic is to come up with a way that is much
cheaper than the current "gc --auto" that involves recent history
walk to consolidate both loose objects and small young packs into
one, so that we can use that logic for "gc --auto".

The key phrase is "without the usual history traversal".  We are
talking about young objects, and they are likely to be reachable
from something (like reflog entries, if not refs).  We may include
unreachable cruft in the result in the "let's be quick and collect
them into a single young pack", and you will need to keep them while
reflog entries are alive, and you will need periodic sweeps with the
usual history walking to remove older crufts that recently have
become unreachable due to reflog expiry from packs anyway, so it is
not a problem for the pack that consolidates young objects into a
single pack to contain some unreachable crufts.

If you start from that assumption [*1*], the way to enumerate the
young objects without the usual history traversal should be fairly
obvious.

By definition, loose objects are all young because they were created
since the last "gc --auto".  Also pack .idx files know their own
creation timestamp to let you decide how old they are, you can see
how many objects there are in the corresponding .pack and how big it
is.

By doing an equivalent of "find .git/objects/[0-9a-f][0-9a-f]/", you
can enumerate the loose objects, and an equivalent of "show-ref"
will enumerate the objects in the pack that the .idx file you
determined to be small and young.

Note that *1* is an assumption. I do not know offhand if such a
"consolidate young objects quickly into one to keep the number of
packs small" strategy is an overall win.

--
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 PATCH] During a shallow fetch, prevent sending over unneeded objects

2013-08-08 Thread Duy Nguyen
On Thu, Aug 8, 2013 at 1:51 PM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> I think this applies to general case as well, not just shallow.
>> Imagine I have a disconnected commit that points to the latest tree
>> (i.e. it contains most of latest changes). Because it's disconnected,
>> it'll be ignored by the server side. But if the servide side does
>> mark_tree_interesting on this commit, a bunch of blobs might be
>> excluded from sending.
>
> I think you meant mark_tree_UNinteresting.

Yes, thanks for correcting.

>> ... So perhaps we could go over have_obj list
>> again, if it's not processed and is
>>
>>  - a tree-ish, mark_tree_uninteresting
>>  - a blob, just mark unintesting
>>
>> and this does regardless of shallow state or edges.
>
> As a general idea, I agree it may be worth trying out to see if your
> concern that the "have" list may be so big that this approach may be
> more costly than it is worth.
>
> If the recipient is known to have something, we do not have to send
> it.

OK. Mathijs, do you want make a patch for it?

> The things that we decide not to send are not necessarily what the
> recipient has, which introduces a twist you need to watch out for if
> we want to go that route.
>
> If the recipient is known to have something, a thin transfer can
> send a delta against it.  You do not want to send the commits before
> the shallow boundary (i.e. the parents of the commits listed in
> .git/shallow) because the recipient does not want them, and that
> means you may have to use a different mark to record that fact.  The
> recipient does not have them, we do not want to send them, and they
> cannot be used as a delta base for what we do send.  Which is quite
> different from the ordinary "uninteresting" objects, those we decide
> not to send because the recipient has them.

I fail to see the point here. There are two different things: what we
want to send, and what we can make deltas against. Shallow boundary
affects the former. What the recipient has affects latter. What is the
twist about?

As for considering objects before shallow boundary uninteresting, I
have a plan for it: kill upload-pack.c:do_rev_list(). The function is
created to make a cut at shallow boundary, but we already have a tool
for that: grafting. In my ongoing shallow series I will create a
temporary shallow file that contains new roots and pass the file to
pack-objects with --shallow-file. pack-objects will never see anything
outside what the recipient may want (i.e. commits before shallow
boundary) to receive and pack-objects' rev-list should do what
upload-pack.c:do_rev_list() currently does.
-- 
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] git exproll: steps to tackle gc aggression

2013-08-08 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> it is
> not a problem for the pack that consolidates young objects into a
> single pack to contain some unreachable crufts.

So far, we have never considered putting unreachable objects in packs.
Let me ask the obvious question first: what happens when I push? Do I
pack up all the loose objects quickly (without bothering about
reachability) and send unreachable cruft to the server? Who is
ultimately going to clean up this cruft, and when?

> Note that *1* is an assumption. I do not know offhand if such a
> "consolidate young objects quickly into one to keep the number of
> packs small" strategy is an overall win.

The way I see it, you're just delaying the reachability analysis
beyond the usual pack-time. The whole point of separating out loose
objects from packfiles was to not make the user wait everytime she
does common repository operations locally: delay the reachability
analysis and compression (aka. packing) until a network operation
kicks in.

To see if introducing another stage is an overall win, think about
what the bottlenecks are: in network operations, it's always the data
being sent over. If I understand correctly, during a network
transaction, there's very minimal computation where the server-client
just quickly tell each other where their refs are: therefore, it's
trivial to figure out what's missing and pack that up to send it over.
The strategy of including unreachable cruft in these packs might make
sense from the point of view of a local gc, but will ultimately slow
down network operations, no?
--
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] gc: reject if another gc is running, unless --force is given

2013-08-08 Thread Nguyễn Thái Ngọc Duy
This may happen when `git gc --auto` is run automatically, then the
user, to avoid wait time, switches to a new terminal, keeps working
and `git gc --auto` is started again because the first gc instance has
not clean up the repository.

This patch tries to avoid multiple gc running, especially in --auto
mode. In the worst case, gc may be delayed 12 hours if a daemon reuses
the pid stored in gc.pid.

kill(pid, 0) support is added to MinGW port so it should work on
Windows too.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 this patch is getting cooler:

 - uname() is dropped in favor of gethostbyname(), which is supported
   by MinGW port.
 - host name is stored in gc.pid as junio suggested so...
 - now we can say "gc is already running on _this host_ with _this pid_..."

 Documentation/git-gc.txt |  6 -
 builtin/gc.c | 67 
 compat/mingw.c   |  6 +
 3 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2402ed6..e158a3b 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -9,7 +9,7 @@ git-gc - Cleanup unnecessary files and optimize the local 
repository
 SYNOPSIS
 
 [verse]
-'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune]
+'git gc' [--aggressive] [--auto] [--quiet] [--prune= | --no-prune] 
[--force]
 
 DESCRIPTION
 ---
@@ -72,6 +72,10 @@ automatic consolidation of packs.
 --quiet::
Suppress all progress reports.
 
+--force::
+   Force `git gc` to run even if there may be another `git gc`
+   instance running on this repository.
+
 Configuration
 -
 
diff --git a/builtin/gc.c b/builtin/gc.c
index 6be6c8d..99682f0 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -167,11 +167,69 @@ static int need_to_gc(void)
return 1;
 }
 
+/* return NULL on success, else hostname running the gc */
+static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
+{
+   static struct lock_file lock;
+   static char locking_host[128];
+   char my_host[128];
+   struct strbuf sb = STRBUF_INIT;
+   struct stat st;
+   uintmax_t pid;
+   FILE *fp;
+   int fd, should_exit;
+
+   if (gethostname(my_host, sizeof(my_host)))
+   strcpy(my_host, "unknown");
+
+   fd = hold_lock_file_for_update(&lock, git_path("gc.pid"),
+  LOCK_DIE_ON_ERROR);
+   if (!force) {
+   fp = fopen(git_path("gc.pid"), "r");
+   memset(locking_host, 0, sizeof(locking_host));
+   should_exit =
+   fp != NULL &&
+   !fstat(fileno(fp), &st) &&
+   /*
+* 12 hour limit is very generous as gc should
+* never take that long. On the other hand we
+* don't really need a strict limit here,
+* running gc --auto one day late is not a big
+* problem. --force can be used in manual gc
+* after the user verifies that no gc is
+* running.
+*/
+   time(NULL) - st.st_mtime <= 12 * 3600 &&
+   fscanf(fp, "%"PRIuMAX" %127c", &pid, locking_host) == 2 
&&
+   !strcmp(locking_host, my_host) &&
+   !kill(pid, 0);
+   if (fp != NULL)
+   fclose(fp);
+   if (should_exit) {
+   if (fd >= 0)
+   rollback_lock_file(&lock);
+   *ret_pid = pid;
+   return locking_host;
+   }
+   }
+
+   strbuf_addf(&sb, "%"PRIuMAX" %s",
+   (uintmax_t) getpid(), my_host);
+   write_in_full(fd, sb.buf, sb.len);
+   strbuf_release(&sb);
+   commit_lock_file(&lock);
+
+   return NULL;
+}
+
 int cmd_gc(int argc, const char **argv, const char *prefix)
 {
int aggressive = 0;
int auto_gc = 0;
int quiet = 0;
+   int force = 0;
+   const char *name;
+   pid_t pid;
 
struct option builtin_gc_options[] = {
OPT__QUIET(&quiet, N_("suppress progress reporting")),
@@ -180,6 +238,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
OPT_BOOLEAN(0, "aggressive", &aggressive, N_("be more thorough 
(increased runtime)")),
OPT_BOOLEAN(0, "auto", &auto_gc, N_("enable auto-gc mode")),
+   OPT_BOOL(0, "force", &force, N_("force running gc even if there 
may be another gc running")),
OPT_END()
};
 
@@ -225,6 +284,14 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
} else
add_repack_all_option();
 
+   name = lock_repo_for_gc(forc

Reproducible, corrupt packfile after fresh

2013-08-08 Thread gitml . jexpert
I'm a heavy user of git-svn and experience an issue with one specific
(git-svn) repository: 'git fsck' reports a corrupt packfile after every
checkout.

Now I'm totally puzzled about the cause and what do about it.
This is what I do:

git svn init -s http://svn.foo.com/myproject myproject.git
cd myproject.git
git svn fetch # Much more reliable than 'git-svn clone'

This checks out ~2100 commits and executes 2 git-gc during checkout. The
final .git repo size is about 940MB. Then I run

git fsck

➜ myproject.git git:(master) git fsck
Checking object directories: 100% (256/256), done.
error: packed 0f5f33639bfc1a781fe080c31a1f076d9a25c1d3 from
.git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack is
corrupt
*** glibc detected *** git: free(): invalid pointer: 0x7f46a09e9010 ***
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f46d9ebab96]
git[0x4ddf46]
git[0x4b4123]
git[0x431524]
git[0x405ce8]
git[0x4050e2]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7f46d9e5d76d]
git[0x405529]
=== Memory map: 
0040-0055f000 r-xp  fc:01 12452043
 /usr/bin/git
0075e000-0075f000 r--p 0015e000 fc:01 12452043
 /usr/bin/git




This only affects this very particular git-svn repo (and I have dozens
of them). The error happens reproducible on every fresh checkout as
described above. The backtrace does not appear always. The object is a
very large blob

git show 0f5f33639bfc1a781fe080c31a1f076d9a25c1d3 | wc -c
39524691


Any hints what to do?

Thanks
- Ben

--
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: Reproducible, corrupt packfile after fresh

2013-08-08 Thread Thomas Rast
gitml.jexp...@recursor.net writes:

> ➜ myproject.git git:(master) git fsck
> Checking object directories: 100% (256/256), done.
> error: packed 0f5f33639bfc1a781fe080c31a1f076d9a25c1d3 from
> .git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack is
> corrupt
> *** glibc detected *** git: free(): invalid pointer: 0x7f46a09e9010 ***
> === Backtrace: =
> /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f46d9ebab96]
> git[0x4ddf46]
> git[0x4b4123]
> git[0x431524]
> git[0x405ce8]
> git[0x4050e2]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7f46d9e5d76d]
> git[0x405529]
> === Memory map: 
> 0040-0055f000 r-xp  fc:01 12452043
>  /usr/bin/git
> 0075e000-0075f000 r--p 0015e000 fc:01 12452043
>  /usr/bin/git
[...]
> Any hints what to do?

Regardless of any possible fault in git-svn, there's an obvious bug here
with git-fsck.  Can you share the pack (if the project is public) or
compile a git-fsck without optimization and with debugging, and run it
under valgrind, to hopefully get us a backtrace of where the memory
management goes off the rails?

-- 
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: Reproducible, corrupt packfile after fresh

2013-08-08 Thread Stefan Beller
On 08/08/2013 01:56 PM, gitml.jexp...@recursor.net wrote:
> I'm a heavy user of git-svn and experience an issue with one specific
> (git-svn) repository: 'git fsck' reports a corrupt packfile after every
> checkout.
> 
> Now I'm totally puzzled about the cause and what do about it.
> This is what I do:
> 
> git svn init -s http://svn.foo.com/myproject myproject.git
> cd myproject.git
> git svn fetch # Much more reliable than 'git-svn clone'
> 
> This checks out ~2100 commits and executes 2 git-gc during checkout. The
> final .git repo size is about 940MB. Then I run
> 
> git fsck
> 
> ➜ myproject.git git:(master) git fsck
> Checking object directories: 100% (256/256), done.
> error: packed 0f5f33639bfc1a781fe080c31a1f076d9a25c1d3 from
> .git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack is
> corrupt
> *** glibc detected *** git: free(): invalid pointer: 0x7f46a09e9010 ***
> === Backtrace: =
> /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f46d9ebab96]
> git[0x4ddf46]
> git[0x4b4123]
> git[0x431524]
> git[0x405ce8]
> git[0x4050e2]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7f46d9e5d76d]
> git[0x405529]
> === Memory map: 
> 0040-0055f000 r-xp  fc:01 12452043
>  /usr/bin/git
> 0075e000-0075f000 r--p 0015e000 fc:01 12452043
>  /usr/bin/git
> 
> 
> 
> 
> This only affects this very particular git-svn repo (and I have dozens
> of them). The error happens reproducible on every fresh checkout as
> described above. The backtrace does not appear always. The object is a
> very large blob
> 
> git show 0f5f33639bfc1a781fe080c31a1f076d9a25c1d3 | wc -c
> 39524691
> 
> 
> Any hints what to do?
> 
> Thanks
> - Ben
> 
> --
> 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
> 

Hi,

Regarding the backtrace, would it be possible to install debug symbols,
so the backtrace is a little more meaningful?
If you distribution doesn't provide debug symbols, compiling git 
yourself is rather easy (git clone && make && make install 
[defaults to ~/bin, so not root required for installing apart 
from missing dependencies])

Stefan




signature.asc
Description: OpenPGP digital signature


Re: Reproducible, corrupt packfile after fresh git-svn checkout

2013-08-08 Thread gitml . jexpert
> Regardless of any possible fault in git-svn, there's an obvious bug here
> with git-fsck.  Can you share the pack (if the project is public) or
> compile a git-fsck without optimization and with debugging, and run it
> under valgrind, to hopefully get us a backtrace of where the memory
> management goes off the rails?

Unfortunately I'm unable to share the pack.

As Java Developer I'm note very savy, but I'd try.
Do you have me any pointers on ".. without optimization and with
debugging" and "run it under valgrind"?

Currently I used
   deb http://ppa.launchpad.net/git-core/ppa/ubuntu quantal main
as source.

--
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: Reproducible, corrupt packfile after fresh git-svn checkout

2013-08-08 Thread Thomas Rast
gitml.jexp...@recursor.net writes:

>> Regardless of any possible fault in git-svn, there's an obvious bug here
>> with git-fsck.  Can you share the pack (if the project is public) or
>> compile a git-fsck without optimization and with debugging, and run it
>> under valgrind, to hopefully get us a backtrace of where the memory
>> management goes off the rails?
>
> Unfortunately I'm unable to share the pack.
>
> As Java Developer I'm note very savy, but I'd try.
> Do you have me any pointers on ".. without optimization and with
> debugging" and "run it under valgrind"?
>
> Currently I used
>deb http://ppa.launchpad.net/git-core/ppa/ubuntu quantal main
> as source.

Try something like

  # The package names might be wrong and/or you may need additional -dev
  # packages, I don't know the specifics for ubuntu
  apt-get install gcc make valgrind libcurl-dev zlib-dev
  cd
  git clone git://git.kernel.org/pub/scm/git/git.git
  cd git
  echo 'CFLAGS = -O0 -g' >>config.mak
  make
  cd /path/to/repo
  valgrind --track-origins=yes ~/git/git-fsck

It'll be very slow, at least 20x the normal runtime, so don't be
surprised if it doesn't seem to get anywhere at first.

-- 
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: Reproducible, corrupt packfile after fresh git-svn checkout

2013-08-08 Thread Matthieu Moy
gitml.jexp...@recursor.net writes:

>> Regardless of any possible fault in git-svn, there's an obvious bug here
>> with git-fsck.  Can you share the pack (if the project is public) or
>> compile a git-fsck without optimization and with debugging, and run it
>> under valgrind, to hopefully get us a backtrace of where the memory
>> management goes off the rails?
>
> Unfortunately I'm unable to share the pack.
>
> As Java Developer I'm note very savy, but I'd try.
> Do you have me any pointers on ".. without optimization and with
> debugging" and "run it under valgrind"?

To fetch sources and compile:

  git clone https://github.com/git/git.git
  cd git/
  make CFLAGS='-g -O0'

To run gdb:

  gdb --args ./git fsck

Then type "run" at the gdb prompt. When the program crashes, you should
get the gdb prompt back. Then, typing "bt" gives you a backtrace, and
"bt full" gives you the same with more information (be careful, there
may be private information in it).

To use valgrind, just do:

  valgrind ./git fsck

(essentially, it runs the program adding a bunch of runtime checks hence
may provide better diagnosis)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Reproducible, corrupt packfile after fresh git-svn checkout

2013-08-08 Thread Stefan Beller
On 08/08/2013 02:23 PM, gitml.jexp...@recursor.net wrote:
>> Regardless of any possible fault in git-svn, there's an obvious bug here
>> with git-fsck.  Can you share the pack (if the project is public) or
>> compile a git-fsck without optimization and with debugging, and run it
>> under valgrind, to hopefully get us a backtrace of where the memory
>> management goes off the rails?
> 
> Unfortunately I'm unable to share the pack.
> 
> As Java Developer I'm note very savy, but I'd try.
> Do you have me any pointers on ".. without optimization and with
> debugging" and "run it under valgrind"?
> 
> Currently I used
>deb http://ppa.launchpad.net/git-core/ppa/ubuntu quantal main
> as source.

The version from the ppa seems to be 1.7.0.4-1ubuntu0.2 but the version
of git being in the native ubuntu quantal repository 
(I assume you're running Ubuntu?) is already 1.7.10.4-1ubuntu1 
(http://packages.ubuntu.com/quantal/git)


However to get an unoptimized version with debug symbols you could do this:

# get the source:
git clone git://git.kernel.org/pub/scm/git/git.git/
cd git

# read INSTALL, the section starting at 
# > Git is reasonably self-sufficient, but does depend on a few external
# > programs and libraries
# this is a guess, maybe you need more or less packages, but you'd need 
the -dev packages for the C-headers
sudo apt-get install libz-dev libopenssl-dev libcurl-dev libexpat-dev 

# Now disable optimizing by overwriting default compile flags:
echo "CFLAGS=-g" > config.mak

# compile and install   
make 
make install # installs to ~/bin

# open a new shell or relogin, and see if you're using the version you 
just build:
git --version
# git version 1.8.4.rc1

Stefan



signature.asc
Description: OpenPGP digital signature


[PATCH/RFC v2] read-cache: save index entry updates in ILOG index extension

2013-08-08 Thread Nguyễn Thái Ngọc Duy
If you have something different from both worktree and HEAD in index,
then accidentally do "git add foo", you may find it hard to recover
the previous version of foo in index. This is especially true when you
do "git add -p" with manual patch editing.

This patch makes sure that every operation that modifies the index
from worktree or stdin is recorded as list of (path, SHA-1) in index
with command+arguments of the operation.

When you make such a mistake, you can look at ILOG extension with
(unimplemented) "git ls-files --generation=X [ -- ]" where X is
from 1 (the most recent operation) to N (the least recent) . "X" could
even be "all" to list all generations.

SHA-1 syntax is also going to be extended to support :-N:path syntax
to get an entry from generation N, for convenience.

Old operation's updates are removed as new ones are added to keep the
size under 1 MB. ILOG keeps minimum 10 operations regardless of its
size. These contansts should be configurable later one. ILOG content
will be compressed later on so that it leaves minimum
footprint. Because it's only needed at index writing time, read-only
operations won't pay the cost for decompressing and compressing ILOG.

ILOG may also be used in a different way to implement "git add
--undo". Before updating entries, git-add could save _old_ entries to
ILOG (and mark to-be-added entries as "deleted/something" for
example). It then can use this information to revert the
operation. Similar candidates are "git commit -a" (destroying current
index) or "git merge" and of course "git mv" and "git rm"

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/add.c  |   1 +
 builtin/apply.c|   1 +
 builtin/update-index.c |   1 +
 cache.h|   3 ++
 read-cache.c   | 121 +
 5 files changed, 127 insertions(+)

diff --git a/builtin/add.c b/builtin/add.c
index 8266a9c..d000f8a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -456,6 +456,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
struct update_callback_data update_data;
 
git_config(add_config, NULL);
+   log_index_changes(prefix, argv);
 
argc = parse_options(argc, argv, prefix, builtin_add_options,
  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
diff --git a/builtin/apply.c b/builtin/apply.c
index 50912c9..fc43ea8 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4423,6 +4423,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
prefix = prefix_;
prefix_length = prefix ? strlen(prefix) : 0;
git_config(git_apply_config, NULL);
+   log_index_changes(prefix, argv);
if (apply_default_whitespace)
parse_whitespace_option(apply_default_whitespace);
if (apply_default_ignorewhitespace)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index c317981..aa757cb 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -799,6 +799,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
usage_with_options(update_index_usage, options);
 
git_config(git_default_config, NULL);
+   log_index_changes(prefix, argv);
 
/* We can't free this memory, it becomes part of a linked list parsed 
atexit() */
lock_file = xcalloc(1, sizeof(struct lock_file));
diff --git a/cache.h b/cache.h
index 85b544f..a2156bf 100644
--- a/cache.h
+++ b/cache.h
@@ -168,6 +168,7 @@ struct cache_entry {
 
 /* used to temporarily mark paths matched by pathspecs */
 #define CE_MATCHED   (1 << 26)
+#define CE_BASE  (1 << 27)
 
 /*
  * Extended on-disk flags
@@ -277,6 +278,7 @@ struct index_state {
 initialized : 1;
struct hash_table name_hash;
struct hash_table dir_hash;
+   struct strbuf *index_log;
 };
 
 extern struct index_state the_index;
@@ -481,6 +483,7 @@ extern struct cache_entry *make_cache_entry(unsigned int 
mode, const unsigned ch
 extern int ce_same_name(const struct cache_entry *a, const struct cache_entry 
*b);
 extern int index_name_is_other(const struct index_state *, const char *, int);
 extern void *read_blob_data_from_index(struct index_state *, const char *, 
unsigned long *);
+extern void log_index_changes(const char *prefix, const char **argv);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID  01
diff --git a/read-cache.c b/read-cache.c
index c3d5e35..4021667 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -14,6 +14,7 @@
 #include "resolve-undo.h"
 #include "strbuf.h"
 #include "varint.h"
+#include "quote.h"
 
 static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int 
really);
 
@@ -33,8 +34,10 @@ static struct cache_entry *refresh_cache_entry(struct 
cache_entry *ce, int reall
 #define CACHE_EXT(s) ( (s[0]<<24)|(s[1]<<16)|(s[2]<<8)|(s[3]) )
 #define CACHE_EXT_TREE 0x54524545  /* "TREE" */
 #define CACHE_EXT_RES

Re: Reproducible, corrupt packfile after fresh git-svn checkout message 3 of 20)

2013-08-08 Thread gitml . jexpert
Great! Thank you all guys for your extensive instructions!

@Thomas: I only had to add libexpat1-dev to the list.

I checked out Git v1.8.3.4 as this was my used verion and built as
instructed. The error is still reproducible using the "CFLAGS = -O0 -g"
build.

So - now the puzzling thing: With valgrind it seems to work! 
If I run it plain, it doesn't:

/tmp/project.git $ valgrind --track-origins=yes  ~/projects/git.git/git-fsck
==3431== Memcheck, a memory error detector
==3431== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==3431== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==3431== Command: /home/ben/projects/git.git/git-fsck
==3431== 
==3431== Warning: set address range perms: large range [0x70b5000, 0x2b0ff000) 
(defined)
Checking object directories: 100% (256/256), done.
==3431== Warning: set address range perms: large range [0x3959d000, 0x49d53000) 
(defined)
Checking objects: 100% (63588/63588), done.
Checking connectivity: 68696, done.
==3431== 
==3431== HEAP SUMMARY:
==3431== in use at exit: 7,574,911 bytes in 15,428 blocks
==3431==   total heap usage: 422,498 allocs, 407,070 frees, 4,396,939,465 bytes 
allocated
==3431== 
==3431== LEAK SUMMARY:
==3431==definitely lost: 0 bytes in 0 blocks
==3431==indirectly lost: 0 bytes in 0 blocks
==3431==  possibly lost: 0 bytes in 0 blocks
==3431==still reachable: 7,574,911 bytes in 15,428 blocks
==3431== suppressed: 0 bytes in 0 blocks
==3431== Rerun with --leak-check=full to see details of leaked memory
==3431== 
==3431== For counts of detected and suppressed errors, rerun with: -v
==3431== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
/tmp/project.git $ ~/projects/git.git/git-fsck
Checking object directories: 100% (256/256), done.
error: packed 49cdd0b21a351f3366008615d2cf8d03ca943978 from 
.git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack is corrupt
*** glibc detected *** : free(): invalid pointer: 0x7f8576682010 
***
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f85ad765b96]
[0x4e727c]


I'm using a i5-3340M on Linux Mint 14 with a offical Linux 3.9.9
gcc (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2

Any further hints?


> Try something like
>
>   # The package names might be wrong and/or you may need additional -dev
>   # packages, I don't know the specifics for ubuntu
>   apt-get install gcc make valgrind libcurl-dev zlib-dev
>   cd
>   git clone git://git.kernel.org/pub/scm/git/git.git
>   cd git
>   echo 'CFLAGS = -O0 -g' >>config.mak
>   make
>   cd /path/to/repo
>   valgrind --track-origins=yes ~/git/git-fsck
>
> It'll be very slow, at least 20x the normal runtime, so don't be
> surprised if it doesn't seem to get anywhere at first.
>

--
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-p4: Fix occasional truncation of symlink contents.

2013-08-08 Thread Alexandru Juncu
Symlink contents in p4 print sometimes have a trailing
new line character, but sometimes it doesn't. git-p4
should only remove the last character if that character
is '\n'.

Signed-off-by: Alex Juncu 
Signed-off-by: Alex Badea 
---
 git-p4.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 31e71ff..a53a6dc 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2180,9 +2180,13 @@ class P4Sync(Command, P4UserMap):
 git_mode = "100755"
 if type_base == "symlink":
 git_mode = "12"
-# p4 print on a symlink contains "target\n"; remove the newline
+# p4 print on a symlink sometimes contains "target\n";
+# if it does, remove the newline
 data = ''.join(contents)
-contents = [data[:-1]]
+if data[-1] == '\n':
+contents = [data[:-1]]
+else:
+contents = [data]
 
 if type_base == "utf16":
 # p4 delivers different text in the python output to -G
-- 
1.8.4.rc0.1.g8f6a3e5

--
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: Reproducible, corrupt packfile after fresh git-svn checkout message 3 of 20)

2013-08-08 Thread Matthieu Moy
gitml.jexp...@recursor.net writes:

> So - now the puzzling thing: With valgrind it seems to work! 

Weird, indeed. What about GDB ?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Reproducible, corrupt packfile after fresh git-svn checkout message 3 of 20)

2013-08-08 Thread Thomas Rast
gitml.jexp...@recursor.net writes:

> So - now the puzzling thing: With valgrind it seems to work! 
> If I run it plain, it doesn't:
>
> /tmp/project.git $ valgrind --track-origins=yes  ~/projects/git.git/git-fsck
[...]
> ==3431== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
> /tmp/project.git $ ~/projects/git.git/git-fsck
> Checking object directories: 100% (256/256), done.
> error: packed 49cdd0b21a351f3366008615d2cf8d03ca943978 from
> .git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack
> is corrupt
> *** glibc detected *** : free(): invalid pointer: 0x7f8576682010 
> ***
> === Backtrace: =
> /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f85ad765b96]
> [0x4e727c]
[...]
> Any further hints?

Hrm.  Can you try getting a backtrace from

  $ gdb ~/projects/git.git/git-fsck
  (gdb) run
  ... wait until it crashes ...
  (gdb) backtrace full

I would have been more interested in error output from valgrind, because
memory corruption invariably happens long before glibc finally figures
out that something is amiss.  But as things stand, the backtrace is
probably the only thing we have...

-- 
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: Reproducible, corrupt packfile after fresh git-svn checkout message

2013-08-08 Thread gitml . jexpert
Am 08.08.2013 15:18, schrieb Matthieu Moy - matthieu@grenoble-inp.fr:> 
gitml.jexp...@recursor.net writes:
>> So - now the puzzling thing: With valgrind it seems to work! 
> Weird, indeed. What about GDB ?


Ah - come on. Is this another Heisenberg bug ?
Still trying to reproduce it using gdb and/or valgrind...



ben@n179 /tmp/project.git $ gdb --args ~/projects/git.git/git-fsck
GNU gdb (GDB) 7.5-ubuntu
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
...
Reading symbols from /home/ben/projects/git.git/git-fsck...done.
(gdb) run
Starting program: /home/ben/projects/git.git/git-fsck 
warning: no loadable sections found in added symbol-file system-supplied DSO at 
0x77ffa000
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Checking object directories: 100% (256/256), done.
Checking objects: 100% (63588/63588), done.
[Inferior 1 (process 3795) exited normally]
(gdb) quit

ben@n179 /tmp/project.git $ ~/projects/git.git/git-fsck
Checking object directories: 100% (256/256), done.
error: packed 49cdd0b21a351f3366008615d2cf8d03ca943978 from 
.git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack is corrupt
Checking objects: 100% (63588/63588), done.

ben@n179 /tmp/project.git $ ~/projects/git.git/git-fsck
Checking object directories: 100% (256/256), done.
error: packed 49cdd0b21a351f3366008615d2cf8d03ca943978 from 
.git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack is corrupt
Checking objects: 100% (63588/63588), done.

ben@n179 /tmp/project.git $ ~/projects/git.git/git-fsck
Checking object directories: 100% (256/256), done.
Checking objects: 100% (63588/63588), done.

ben@n179 /tmp/project.git $ ~/projects/git.git/git-fsck
Checking object directories: 100% (256/256), done.
error: packed 0f5f33639bfc1a781fe080c31a1f076d9a25c1d3 from 
.git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack is corrupt
*** glibc detected *** : free(): invalid pointer: 0x7fe129669010 
***
=== Backtrace: =
/lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7fe162d3eb96]
[0x51e401]
[0x51e53c]
[0x51ecc3]
[0x4e707b]
[0x4e7485]
[0x43d433]
[0x405158]
[0x4052ee]
[0x4054ba]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7fe162ce176d]
[0x404429]
=== Memory map: 
0040-005bb000 r-xp  fc:01 3939047
/home/ben/projects/git.git/git-fsck
007ba000-007bb000 r--p 001ba000 fc:01 3939047
/home/ben/projects/git.git/git-fsck
007bb000-007c2000 rw-p 001bb000 fc:01 3939047
/home/ben/projects/git.git/git-fsck
007c2000-00822000 rw-p  00:00 0 
0277-05809000 rw-p  00:00 0  [heap]
7fe129669000-7fe12bc38000 rw-p  00:00 0 
7fe12dff5000-7fe12e00a000 r-xp  fc:01 7081526
/lib/x86_64-linux-gnu/libgcc_s.so.1
7fe12e00a000-7fe12e209000 ---p 00015000 fc:01 7081526
/lib/x86_64-linux-gnu/libgcc_s.so.1
7fe12e209000-7fe12e20a000 r--p 00014000 fc:01 7081526
/lib/x86_64-linux-gnu/libgcc_s.so.1
7fe12e20a000-7fe12e20b000 rw-p 00015000 fc:01 7081526
/lib/x86_64-linux-gnu/libgcc_s.so.1
7fe12e20b000-7fe13e9c1000 r--p  fc:01 10879034   
/tmp/project.git/.git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.pack
7fe13e9c1000-7fe162a0b000 r--p  fc:01 10881032   
/tmp/project.git/.git/objects/pack/pack-5b132454d5acb542969a939fe55186a65d9dd697.pack
7fe162a0b000-7fe162abc000 r--p  fc:01 10902665   
/tmp/project.git/.git/objects/pack/pack-6a6f5355584a5d71215d5fc867ce09602ceab533.idx
7fe162abc000-7fe162abe000 r-xp  fc:01 7085848
/lib/x86_64-linux-gnu/libdl-2.15.so
7fe162abe000-7fe162cbe000 ---p 2000 fc:01 7085848
/lib/x86_64-linux-gnu/libdl-2.15.so
7fe162cbe000-7fe162cbf000 r--p 2000 fc:01 7085848
/lib/x86_64-linux-gnu/libdl-2.15.so
7fe162cbf000-7fe162cc rw-p 3000 fc:01 7085848
/lib/x86_64-linux-gnu/libdl-2.15.so
7fe162cc-7fe162e75000 r-xp  fc:01 7085850
/lib/x86_64-linux-gnu/libc-2.15.so
7fe162e75000-7fe163074000 ---p 001b5000 fc:01 7085850
/lib/x86_64-linux-gnu/libc-2.15.so
7fe163074000-7fe163078000 r--p 001b4000 fc:01 7085850
/lib/x86_64-linux-gnu/libc-2.15.so
7fe163078000-7fe16307a000 rw-p 001b8000 fc:01 7085850
/lib/x86_64-linux-gnu/libc-2.15.so

Re: [PATCH v2 12/19] read-cache: read index-v5

2013-08-08 Thread Thomas Gummerer
Duy Nguyen  writes:

> On Wed, Jul 17, 2013 at 3:11 PM, Thomas Gummerer  wrote:
>> Duy Nguyen  writes:
>>
>> [..]
>>
 +static int read_entries(struct index_state *istate, struct 
 directory_entry **de,
 +   unsigned int *entry_offset, void **mmap,
 +   unsigned long mmap_size, unsigned int *nr,
 +   unsigned int *foffsetblock)
 +{
 +   struct cache_entry *head = NULL, *tail = NULL;
 +   struct conflict_entry *conflict_queue;
 +   struct cache_entry *ce;
 +   int i;
 +
 +   conflict_queue = NULL;
 +   if (read_conflicts(&conflict_queue, *de, mmap, mmap_size) < 0)
 +   return -1;
 +   for (i = 0; i < (*de)->de_nfiles; i++) {
 +   if (read_entry(&ce,
 +  *de,
 +  entry_offset,
 +  mmap,
 +  mmap_size,
 +  foffsetblock) < 0)
 +   return -1;
 +   ce_queue_push(&head, &tail, ce);
 +   *foffsetblock += 4;
 +
 +   /*
 +* Add the conflicted entries at the end of the index file
 +* to the in memory format
 +*/
 +   if (conflict_queue &&
 +   (conflict_queue->entries->flags & CONFLICT_CONFLICTED) 
 != 0 &&
 +   !cache_name_compare(conflict_queue->name, 
 conflict_queue->namelen,
 +   ce->name, ce_namelen(ce))) {
 +   struct conflict_part *cp;
 +   cp = conflict_queue->entries;
 +   cp = cp->next;
 +   while (cp) {
 +   ce = convert_conflict_part(cp,
 +   conflict_queue->name,
 +   conflict_queue->namelen);
 +   ce_queue_push(&head, &tail, ce);
 +   conflict_part_head_remove(&cp);
 +   }
 +   conflict_entry_head_remove(&conflict_queue);
 +   }
>>>
>>> I start to wonder if separating staged entries is a good idea. It
>>> seems to make the code more complicated. The good point about conflict
>>> section at the end of the file is you can just truncate() it out.
>>> Another way is putting staged entries in fileentries, sorted
>>> alphabetically then by stage number, and a flag indicating if the
>>> entry is valid. When you remove resolve an entry, just set the flag to
>>> invalid (partial write), so that read code will skip it.
>>>
>>> I think this approach is reasonably cheap (unless there are a lot of
>>> conflicts) and it simplifies this piece of code. truncate() may be
>>> overrated anyway. In my experience, I "git add " as soon as I
>>> resolve  (so that "git diff" shrinks). One entry at a time, one
>>> index write at a time. I don't think I ever resolve everything then
>>> "git add -u .", which is where truncate() shines because staged
>>> entries are removed all at once. We should optimize for one file
>>> resolution at a time, imo.
>>
>> Thanks for your comments.  I'll address the other ones once we decided
>> to do with the conflicts.
>>
>> It does make the code quite a bit more complicated, but also has one
>> advantage that you overlooked.
>
> I did overlook, although my goal is to keep the code simpler, not more
> comlicated. The thinking is if we can find everything in fileentries
> table, the code here is simplified, so..
>
>> We wouldn't truncate() when resolving
>> the conflicts.  The resolve undo data is stored with the conflicts and
>> therefore we could just flip a bit and set the stage of the cache-entry
>> in the main index to 0 (always once we got partial writing).  This can
>> be fast both in case we resolve one entry at a time and when we resolve
>> a lot of entries.  The advantage is even bigger when we resolve one
>> entry at a time, when we otherwise would have to re-write the index for
>> each conflict resolution.
>
> If I understand it correctly, filentries can only contain stage-0 or
> stage-1 entries, "stage > 0" entries are stored in conflict data. Once
> a conflict is solved, you update the stage-1 entry in fileentries,
> turning it to stage-0 and recalculate the entry checksum. Conflict
> data remains there to function as the old REUC extension. Correct?

Correct.

> First of all, if that's true, we only need 1 bit for stage in fileentries 
> table.
>
> Secondly, you may get away with looking up to conflict data in this
> function by storing all stages in fileentries (now we need 2-bit
> stage), replicated in conflict data for reuc function. When you
> resolve conflict, you flip

Re: Reproducible, corrupt packfile after fresh git-svn checkout message

2013-08-08 Thread Matthieu Moy
gitml.jexp...@recursor.net writes:

> /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7fe162d3eb96]
> [0x51e401]
> [0x51e53c]
> [0x51ecc3]
> [0x4e707b]
> [0x4e7485]
> [0x43d433]
> [0x405158]
> [0x4052ee]
> [0x4054ba]

Using

  addr2line -e ~/projects/git.git/git-fsck

on these addresses may help a little, but not sure it's going to be
sufficient :-(.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Reproducible, corrupt packfile after fresh git-svn checkout message (gitml: message 5 of 20)

2013-08-08 Thread gitml . jexpert
> Using
> 
>   addr2line -e ~/projects/git.git/git-fsck
> 
> on these addresses may help a little, but not sure it's going to be
> sufficient :-(.


I'm still trying to reproduce this issue using gdb.
Also I'm trying to reproduce this issue with my git repo on another machine.

ben@n179 /tmp $ addr2line -e ~/projects/git.git/git-fsck
0x51e401
0x51e53c
0x51ecc3
0x4e707b
0x4e7485
0x43d433
0x405158
0x4052ee
0x4054ba
/home/ben/projects/git.git/sha1_file.c:1901
/home/ben/projects/git.git/sha1_file.c:1928
/home/ben/projects/git.git/sha1_file.c:2096
/home/ben/projects/git.git/pack-check.c:119
/home/ben/projects/git.git/pack-check.c:177
/home/ben/projects/git.git/builtin/fsck.c:678
/home/ben/projects/git.git/git.c:291
/home/ben/projects/git.git/git.c:453
/home/ben/projects/git.git/git.c:543

--
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: Reproducible, corrupt packfile after fresh git-svn checkout message (gitml: message 5 of 20)

2013-08-08 Thread Thomas Rast
gitml.jexp...@recursor.net writes:

>> Using
>> 
>>   addr2line -e ~/projects/git.git/git-fsck
>> 
>> on these addresses may help a little, but not sure it's going to be
>> sufficient :-(.
>
>
> I'm still trying to reproduce this issue using gdb.
> Also I'm trying to reproduce this issue with my git repo on another machine.
>
> ben@n179 /tmp $ addr2line -e ~/projects/git.git/git-fsck
> 0x51e401
> 0x51e53c
> 0x51ecc3
> 0x4e707b
> 0x4e7485
> 0x43d433
> 0x405158
> 0x4052ee
> 0x4054ba
> /home/ben/projects/git.git/sha1_file.c:1901
> /home/ben/projects/git.git/sha1_file.c:1928
> /home/ben/projects/git.git/sha1_file.c:2096
> /home/ben/projects/git.git/pack-check.c:119
> /home/ben/projects/git.git/pack-check.c:177
> /home/ben/projects/git.git/builtin/fsck.c:678
> /home/ben/projects/git.git/git.c:291
> /home/ben/projects/git.git/git.c:453
> /home/ben/projects/git.git/git.c:543

Can you try to reproduce with a version older than v1.8.3?
E.g. v1.8.2.3.

I'm asking because the above points at packed_object_info(), which I
recently rewrote to be nonrecursive.


Also, can you please stop losing the Cc list?  The etiquette on this
list is to Cc everyone who was involved so far, usually meaning everyone
who was already a recipient of the mail you are replying to.

-- 
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


[PATCH v3] status: always show tracking branch even no change

2013-08-08 Thread Jiang Xin
Changes since v2:

 * The return value of function stat_tracking_info() is changed.
   When the current branch and its remote tracking branch point
   to the same commit, will return 1, instead of 0. Because we
   want to report the tracking info for such case.

 * Remove duplicated codes in builtin/branch.c, and make it simpler.

Jiang Xin (1):
  status: always show tracking branch even no change

 builtin/branch.c | 18 +---
 remote.c | 18 +++-
 t/t6040-tracking-info.sh | 54 
 wt-status.c  |  5 +
 4 files changed, 73 insertions(+), 22 deletions(-)

-- 
1.8.4.rc1.430.g417e2f3

--
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] status: always show tracking branch even no change

2013-08-08 Thread Jiang Xin
If the current branch has an upstream branch, and there are changes
between the current branch and its upstream, some commands (such as
"git status", "git status -bs", and "git checkout") will report their
relationship. E.g.

$ git status
# On branch master
# Your branch is ahead of 'origin/master' by 1 commit.
#   (use "git push" to publish your local commits)
#
...

$ git status -bs
## master...origin/master [ahead 1]
...

$ git checkout master
Already on 'master'
Your branch is ahead of 'origin/master' by 1 commit.
  (use "git push" to publish your local commits)

But if there is no difference between the current branch and its
upstream, the relationship will not be reported, and it's hard to
tell whether the current branch has a tracking branch or not. And
what's worse, when the 'push.default' config variable is set to
`matching`, it's hard to tell whether the current branch has already
been pushed out or not at all [1].

So always show the remote tracking branch in the output of "git status"
and other commands will help users to see where the current branch
will push to and pull from. E.g.

$ git status
# On branch master
# Your branch is identical to 'origin/master'.
#
...

$ git status -bs
## master...origin/master
...

$ git checkout master
Already on 'master'
Your branch is identical to 'origin/master'.

This patch changes the return value of function stat_tracking_info().
When the current branch and its remote tracking branch point to the
same commit, will return 1, instead of 0. Because we want to report
the tracking info for such case. Also add some test cases in t6040.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/198703

Signed-off-by: Jiang Xin 
---
 builtin/branch.c | 18 +---
 remote.c | 18 +++-
 t/t6040-tracking-info.sh | 54 
 wt-status.c  |  5 +
 4 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0836890..359e75d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -424,19 +424,8 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
struct branch *branch = branch_get(branch_name);
struct strbuf fancy = STRBUF_INIT;
 
-   if (!stat_tracking_info(branch, &ours, &theirs)) {
-   if (branch && branch->merge && branch->merge[0]->dst &&
-   show_upstream_ref) {
-   ref = shorten_unambiguous_ref(branch->merge[0]->dst, 0);
-   if (want_color(branch_use_color))
-   strbuf_addf(stat, "[%s%s%s] ",
-   
branch_get_color(BRANCH_COLOR_UPSTREAM),
-   ref, 
branch_get_color(BRANCH_COLOR_RESET));
-   else
-   strbuf_addf(stat, "[%s] ", ref);
-   }
+   if (!stat_tracking_info(branch, &ours, &theirs))
return;
-   }
 
if (show_upstream_ref) {
ref = shorten_unambiguous_ref(branch->merge[0]->dst, 0);
@@ -448,7 +437,10 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
strbuf_addstr(&fancy, ref);
}
 
-   if (!ours) {
+   if (!ours && !theirs) {
+   if (ref)
+   strbuf_addf(stat, _("[%s]"), fancy.buf);
+   } else if (!ours) {
if (ref)
strbuf_addf(stat, _("[%s: behind %d]"), fancy.buf, 
theirs);
else
diff --git a/remote.c b/remote.c
index 2433467..79766df 100644
--- a/remote.c
+++ b/remote.c
@@ -1740,6 +1740,10 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs)
const char *rev_argv[10], *base;
int rev_argc;
 
+   /* Set both num_theirs and num_ours as undetermined. */
+   *num_theirs = -1;
+   *num_ours = -1;
+
/*
 * Nothing to report unless we are marked to build on top of
 * somebody else.
@@ -1758,16 +1762,18 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs)
theirs = lookup_commit_reference(sha1);
if (!theirs)
return 0;
+   *num_theirs = 0;
 
if (read_ref(branch->refname, sha1))
return 0;
ours = lookup_commit_reference(sha1);
if (!ours)
return 0;
+   *num_ours = 0;
 
-   /* are we the same? */
+   /* are we the same? both num_theirs and num_ours have been set to 0. */
if (theirs == ours)
-   return 0;
+   return 1;
 
/* Run "rev-list --left-right ours...theirs" internally... */
rev_argc = 0;
@@ -1786,8 +1792,6 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *nu

Re: Reproducible, corrupt packfile after fresh git-svn checkout message (gitml: message 5 of 20) (gitml: message 6 of 20)

2013-08-08 Thread Ben Tebulin

Am 08.08.2013 16:20, schrieb Thomas Rast - tr...@inf.ethz.ch:
> Can you try to reproduce with a version older than v1.8.3?
> E.g. v1.8.2.3.
> I'm asking because the above points at packed_object_info(), which I
> recently rewrote to be nonrecursive.

It seems to run 'much better' 
  v1.8.2.3 : 3/10 runs do fail
  fb56570  : 9/10 runs do fail

They always fail on a big blob (39MB) as I wrote in my first e-mail:

ben@n179 /tmp/project.git $ ~/projects/git.git/git-show 
49cdd0b21a351f3366008615d2cf8d03ca943978 | wc -c
error: sha1 mismatch 49cdd0b21a351f3366008615d2cf8d03ca943978
fatal: bad object 49cdd0b21a351f3366008615d2cf8d03ca943978
0
ben@n179 /tmp/project.git $ ~/projects/git.git/git-show 
49cdd0b21a351f3366008615d2cf8d03ca943978 | wc -c
39517156


> Also, can you please stop losing the Cc list?  
I'm _very_ sorry for this. I was trying to hide my e-mail 
address from spam robots using spamgourmet.com as remailer.
Unfortunately it breaks CC.  Switching e-mail now.
--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Matthieu Moy
Ramkumar Ramachandra  writes:

> Junio C Hamano wrote:
>> The only thing it does is to scratch an irrelevant itch by people
>> who peek the codebase and find an old command whose existence does
>> not even hurt them.  They may have too much time on their hand, but
>> that is not an excuse to waste other people's time by adding extra
>> makework on our plate, or changing the behaviour for people who use
>> the command without explanation.
>
> It's a maintenance burden 

I'm not really worried about the maintainance of a 20-lines long
function ...

> that confuses users.

... but I do agree that the doc is really confusing. It would be much
better if the doc could be reduced to:

"This is a synonym for linkgit:git-log[1] --raw --some --other ---options.
Please refer to the documentation of that command."

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Feature Request: Support logic or shell execution to control values in .gitconfig

2013-08-08 Thread Morgan McClure
I sync all my dot files (including .gitconfig) among several machines
and it's currently not possible to put conditional logic in many
fields (any that aren't considered strings to be executed as shell
commands ie aliases, editor, etc).

My specific use case is the email address. Normally I want my email
address to read:
mcclurem@$HOSTNAME  

I use this to track which machine I'm committing from etc.

I propose using something reminiscent of bash syntax, either:
value = $(SOMETEXTTOEXECUTE)

or

value = `SOMETEXTTOEXECUTE`

Is this a feature others could get behind?

-Morgan McClure
--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Ramkumar Ramachandra
Matthieu Moy wrote:
> ... but I do agree that the doc is really confusing. It would be much
> better if the doc could be reduced to:
>
> "This is a synonym for linkgit:git-log[1] --raw --some --other ---options.
> Please refer to the documentation of that command."

I don't think there's an exact correspondence with any combination of
command-line options. Perhaps we can describe it in words, and ask
people to use log instead?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Feature Request: Support logic or shell execution to control values in .gitconfig

2013-08-08 Thread Matthieu Moy
Morgan McClure  writes:

> I propose using something reminiscent of bash syntax, either:
> value = $(SOMETEXTTOEXECUTE)
>
> or
>
> value = `SOMETEXTTOEXECUTE`

That would mean executing SOMETEXTTOEXECUTE each time the config file is
read. This raises two issues:

* A security issue, as SOMETEXTTOEXECUTE could also be something
  dangerous. It would not be much worse than the current situation (if
  your config file is not trusted, then an attacker could put malicious
  code in core.editor for example), but still increase the security
  risk (as any command reading the config may trigger execution).

* A performance issue with the current git implementation, as the config
  file may be read many time for a single git execution.

> Is this a feature others could get behind?

I think it's unlikely that this ever be implemented. What I suggest
instead is to edit/track/share template configuration files like

~/.gitconfig.in
email = me@HOSTNAME@

and then script something like sed -e "s/@HOSTNAME@/$(hostname)/" <
~/.gitconfig.in > ~/.gitconfig.

You may also use the include.path functionality to share most of your
configuration, and include a small file which is different on each host.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Remove old forgotten command: whatchanged

2013-08-08 Thread Matthieu Moy
Ramkumar Ramachandra  writes:

> Matthieu Moy wrote:
>> ... but I do agree that the doc is really confusing. It would be much
>> better if the doc could be reduced to:
>>
>> "This is a synonym for linkgit:git-log[1] --raw --some --other ---options.
>> Please refer to the documentation of that command."
>
> I don't think there's an exact correspondence with any combination of
> command-line options. Perhaps we can describe it in words, and ask
> people to use log instead?

I'd say either this, or add the missing features to "git log" to make my
suggestion possible (after all, if some people like "git whatchanged",
then maybe the feature would be of interest to "git log" users ?).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Reproducible, corrupt packfile after fresh git-svn checkout message (gitml: message 5 of 20) (gitml: message 6 of 20)

2013-08-08 Thread Thomas Rast
Ben Tebulin  writes:

> Am 08.08.2013 16:20, schrieb Thomas Rast - tr...@inf.ethz.ch:
>> Can you try to reproduce with a version older than v1.8.3?
>> E.g. v1.8.2.3.
>> I'm asking because the above points at packed_object_info(), which I
>> recently rewrote to be nonrecursive.
>
> It seems to run 'much better' 
>   v1.8.2.3 : 3/10 runs do fail
>   fb56570  : 9/10 runs do fail

The good news is that this shifts the blame away from my commit ;-) as
the problem clearly existed even before that.

The bad news, of course, is that this is another hunch that turned out
to be wrong.  I'm running out of ideas.

> They always fail on a big blob (39MB) as I wrote in my first e-mail:
>
> ben@n179 /tmp/project.git $ ~/projects/git.git/git-show 
> 49cdd0b21a351f3366008615d2cf8d03ca943978 | wc -c
> error: sha1 mismatch 49cdd0b21a351f3366008615d2cf8d03ca943978
> fatal: bad object 49cdd0b21a351f3366008615d2cf8d03ca943978
> 0
> ben@n179 /tmp/project.git $ ~/projects/git.git/git-show 
> 49cdd0b21a351f3366008615d2cf8d03ca943978 | wc -c
> 39517156

Hrmm.  I wonder about the significance of those 39MB.  What is your
core.packedGitWindowSize?  (Judging from the pastes you seem to be on
64bit, so the default would be 1GB.)

-- 
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: Feature Request: Support logic or shell execution to control values in .gitconfig

2013-08-08 Thread Greg Troxel

Matthieu Moy  writes:

> What I suggest instead is to edit/track/share template configuration
> files like
>
> ~/.gitconfig.in
> email = me@HOSTNAME@
>
> and then script something like sed -e "s/@HOSTNAME@/$(hostname)/" <
> ~/.gitconfig.in > ~/.gitconfig.
>
> You may also use the include.path functionality to share most of your
> configuration, and include a small file which is different on each host.

For what it's worth, I

  keep dotfiles checked in as m4 sources, which is more or less
  equivalent to Matthieu's suggestion of sed

  don't try to keep .gitconfig under source control, but instead have a
  "git-config" alias that executes a lot of "git config --global"
  commands.

The downsides to the git-config shell function approach are:

  unwanted configurations are not removed

  one has to source (e.g.) .bash_aliases and then rerun git-config after
  updating the dotfile repo,


I suspect what's really needed is some sort of two-way macro
preprocessor that can take changes in an output file an back them into
the source.


pgpHDjNi8b3hD.pgp
Description: PGP signature


Re: Reproducible, corrupt packfile after fresh git-svn checkout message

2013-08-08 Thread Ben Tebulin
I was unable to reproduce the error with the same repo and same Git
version on a different machine (Debian Squeeze x64 on a AMD Phenom x6
1045T).


> I'm running out of ideas.
Me, too. Based on out current observations I'd assume one of:

a) a rare, timing-sensitive bug in Git
b) a compiler/distribution/environment sensitive issue
c) or defect/buggy Hardware (CPU, Memory)

> Hrmm.  I wonder about the significance of those 39MB.  What is your
> core.packedGitWindowSize?  (Judging from the pastes you seem to be on
> 64bit, so the default would be 1GB.)
The default. I have 12GB RAM.

So I'll try to rule out b) and c) as far as I can and report in if I
have any new findings.

Nevertheless thank you very much for your extensive assistance!

- Ben
--
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: Reproducible, corrupt packfile after fresh git-svn checkout message

2013-08-08 Thread Matthieu Moy
Ben Tebulin  writes:

> c) or defect/buggy Hardware (CPU, Memory)

You may want to try a memcheck86 on your machine for a few hours, in
case ... (if your RAM is broken, you do want to know it!).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-08 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> Junio C Hamano wrote:
>> it is
>> not a problem for the pack that consolidates young objects into a
>> single pack to contain some unreachable crufts.
>
> So far, we have never considered putting unreachable objects in packs.
> Let me ask the obvious question first: what happens when I push? Do I
> pack up all the loose objects quickly (without bothering about
> reachability) and send unreachable cruft to the server?

No.

I thought the discussion was about making the local gc cheaper, and
the "Imagine we have a cheap way" was to address it by assuming that
the daily "pack young objects into a single pack" can be sped up if
we did not have to traverse history.  More permanent packs (the
older ones in "set of packs staggered by age" Martin proposes) in
the repository should go through the normal history traversal route.

And of course we do not transfer objects that are not asked for from
or to a repository over pack tranfer.

Most importantly, it is not about butchering the pack machinery in
such a way that we can create _only_ such "non history traversal"
packs.

So I do not see how that question is "obvious".  The question
obviously pointless and misses the mark by wide margin?  The
question makes it obvious that whoever asks it does not understand
how Git works?

--
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 PATCH] During a shallow fetch, prevent sending over unneeded objects

2013-08-08 Thread Junio C Hamano
Duy Nguyen  writes:

> I fail to see the point here. There are two different things: what we
> want to send, and what we can make deltas against. Shallow boundary
> affects the former. What the recipient has affects latter. What is the
> twist about?

do_rev_list() --> mark_edges_uninteresting() --> show_edge() callchain
that eventually does this:

static void show_edge(struct commit *commit)
{
fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
}

was what I had in mind.

For a non-shallow transfer, feeding "-" is done for
commits that we do not send (we do not do so for all of them) and
those that we know the recipient does have.  Two different things
used to be the same, but with your suggestion they are not.  Which
is a good thing but we need to be careful to make sure existing
codepaths do not conflate them and untangle ones that do if there
are any, that's all.

> As for considering objects before shallow boundary uninteresting, I
> have a plan for it: kill upload-pack.c:do_rev_list(). The function is
> created to make a cut at shallow boundary,...

Hmph, that function is not primarily about shallow boundary but does
all packing in general.

The edge hinting in there is for thin transfer where the sender
sends deltas against base objects that are known to be present in
the receiving repository, without sending the base objects.

--
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 5/5] rm: delete .gitmodules entry of submodules removed from the work tree

2013-08-08 Thread Jens Lehmann
Am 07.08.2013 20:28, schrieb Fredrik Gustafsson:
> On Tue, Aug 06, 2013 at 02:11:56PM -0700, Junio C Hamano wrote:
>> Thanks, will replace the top two commits and queue.  Looks like we
>> are getting ready for 'next'?
> 
> I'm a bit curious about if we should move towards a reentrent libgit
> (which would for example make multithreading easier) or not.

I'm not aware of such an effort in core Git (I always thought that
libgit2 is the project doing what you seem to aim for).

> If so, I suggest that this patch only use die() in builtin/. However I
> know that there's a lot of die() all over libgit today, I'm curious
> about what direction we're heading.

The die() calls are just one part. Global variables are another issue,
we have memory which is implicitly freed on exit ... so unless we
commit ourselves to fix all those issues I see no point in moving the
die() calls into builtin/ in my 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: Remove old forgotten command: whatchanged

2013-08-08 Thread Junio C Hamano
Matthieu Moy  writes:

> I'd say either this, or add the missing features to "git log" to make my
> suggestion possible (after all, if some people like "git whatchanged",
> then maybe the feature would be of interest to "git log" users ?).

There is no _missing feature_ per se.  whatchanged by default (1) gives
diffs, (2) does not show empty commits, and (3) uses raw format for
its diff output.

I am fuzzy about where the name of the command and its default
behaviour came from (I suspect it may have been modelled after
another SCM kernel folks were familiar with), but I do not mind if
somebody does an archaeology (read: ask Linus) and reduce the
document to something like this:

git-whatchanged(1)
==

NAME

git-whatchanged - Show logs with difference each commit introduces


SYNOPSIS

[verse]
'git whatchanged' ...

DESCRIPTION
---

Shows commit logs and diff output each commit introduces.  This is
essentially the same as linkgit:git-log[1] run with different
defaults.

The command name and behaviour were originally borrowed from 
and the command is kept primarily for historical reasons (fingers of
many people who learned Git by reading Linux kernel mailing list are
trained to type it before `git log` was invented).

New users are encouraged to use linkgit:git-log[1] instead.


Examples

`git whatchanged -p v2.6.12.. include/scsi drivers/scsi`::

Show as patches the commits since version 'v2.6.12' that changed
any file in the include/scsi or drivers/scsi subdirectories

`git whatchanged --since="2 weeks ago" -- gitk`::

Show the changes during the last two weeks to the file 'gitk'.
The "--" is necessary to avoid confusion with the *branch* named
'gitk'

GIT
---
Part of the linkgit:git[1] suite
--
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 exproll: steps to tackle gc aggression

2013-08-08 Thread Martin Fick
On Thursday, August 08, 2013 10:56:38 am Junio C Hamano 
wrote:
> I thought the discussion was about making the local gc
> cheaper, and the "Imagine we have a cheap way" was to
> address it by assuming that the daily "pack young
> objects into a single pack" can be sped up if we did not
> have to traverse history.  More permanent packs (the
> older ones in "set of packs staggered by age" Martin
> proposes) in the repository should go through the normal
> history traversal route.

Assuming I understand what you are suggesting, would these 
"young object" likely still get "deduped" in an efficient 
way without doing history traversal (it sounds like they 
would)?  In other words, if I understand correctly, it would 
save time by not pruning unreferenced objects, but it would 
still be deduping things and delta compressing also, so you 
would still likely get a great benefit from creating these 
young object packs?  In other words, is there still a good 
chance that my 317 new pack files which included a 33M pack 
file will still get consolidated down to something near 8M?  

If so, then yeah this might be nice, especially if the 
history traversal is what would speed this up.  Because 
today, my solution mostly saves IO and not time.  I think it 
still saves time, I believe I have seen up to a 50% savings, 
but that is nothing compared to massive, several orders of 
magnitude IO savings.  But if what you suggest could also 
give massive time (orders of magnitude) savings along with 
the IO improvements I am seeing, then suddenly repacking 
regularly would become very cheap even on large repos.  

The only time consuming piece would be pruning then?  Could 
bitmaps eventually help out there?

-Martin


-- 
The Qualcomm Innovation Center, Inc. is a member of Code 
Aurora Forum, hosted by The Linux Foundation
 
--
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 exproll: steps to tackle gc aggression

2013-08-08 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> So I do not see how that question is "obvious".  The question
> obviously pointless and misses the mark by wide margin?  The
> question makes it obvious that whoever asks it does not understand
> how Git works?

Shall we all sit and mourn over the fact that I don't understand how
Git works, or are you willing to explain it to me?

> And of course we do not transfer objects that are not asked for from
> or to a repository over pack tranfer.
>
> Most importantly, it is not about butchering the pack machinery in
> such a way that we can create _only_ such "non history traversal"
> packs.

I asked you a very simple question: what happens when I do "git push"?
Instead of answering the question, you butchered the pack machinery to
"only" create packs with garbage in them (aka. stripped out the
reachability analysis code completely), and blamed me for doing it.

Explain it to me in plain English without getting agitated:

1. I'm on my terminal doing various repository operations: constantly
creating new objects and moving my refs around to create unreachable
objects. I have lots of loose objects.

2. I say "git push". What happens? A reachability analysis is
performed on my loose objects, and the ones reachable by the ref I'm
sending are packed up and sent over the network. Now, I no longer have
any loose objects.

3. After a few days of working, the gc heuristics figure out that I
have too much garbage and too many packs; a cleanup is required. The
gc --auto which doesn't tolerate fragmentation: it tries to put
everything into one large pack.

Loop.

We're talking about tackling the gc aggression problem in 3. And you
propose putting the young objects in a pack without performing
reachability analysis: I'm asking how this is going to benefit me;
when I say "git push" (or when gc decides to repack), won't I need to
explode the young pack into loose objects, do a reachability analysis,
and repack anyway?
--
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: Reproducible, corrupt packfile after fresh git-svn checkout message (gitml: message 5 of 20) (gitml: message 6 of 20)

2013-08-08 Thread Junio C Hamano
Thomas Rast  writes:

> Ben Tebulin  writes:
>
>> Am 08.08.2013 16:20, schrieb Thomas Rast - tr...@inf.ethz.ch:
>>> Can you try to reproduce with a version older than v1.8.3?
>>> E.g. v1.8.2.3.
>>> I'm asking because the above points at packed_object_info(), which I
>>> recently rewrote to be nonrecursive.
>>
>> It seems to run 'much better' 
>>   v1.8.2.3 : 3/10 runs do fail
>>   fb56570  : 9/10 runs do fail
>
> The good news is that this shifts the blame away from my commit ;-) as
> the problem clearly existed even before that.
>
> The bad news, of course, is that this is another hunch that turned out
> to be wrong.  I'm running out of ideas.
>
>> They always fail on a big blob (39MB) as I wrote in my first e-mail:
>>
>> ben@n179 /tmp/project.git $ ~/projects/git.git/git-show 
>> 49cdd0b21a351f3366008615d2cf8d03ca943978 | wc -c
>> error: sha1 mismatch 49cdd0b21a351f3366008615d2cf8d03ca943978
>> fatal: bad object 49cdd0b21a351f3366008615d2cf8d03ca943978
>> 0
>> ben@n179 /tmp/project.git $ ~/projects/git.git/git-show 
>> 49cdd0b21a351f3366008615d2cf8d03ca943978 | wc -c
>> 39517156

Hmm, from this, and a later one ...

> Ben Tebulin  writes:
>
> I was unable to reproduce the error with the same repo and same Git
> version on a different machine (Debian Squeeze x64 on a AMD Phenom x6
> 1045T).

... I am reading that (1) the packfile and repository is basically
OK, (2) reading that object sometimes fails, and (3) the symptom is
not limited to fsck but anything that reads the object with
parse_object().  And that symptom exists only on that single machine
(I am assuming that the repository was bit-for-bit copied, not
"cloned", for the purpose of testing it on the other machine).  That
makes me suspect something outside the control of Git (e.g. faulty
memory or disk controller cable).

Are there other big blobs in the repository, and would "show | wc" fail
if you attempt to read it on that machine?

Thanks all for helping to track this, by the way.
--
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


[RFC] allow git pull to preserve merges

2013-08-08 Thread Stephen Haberman
Hey,

Following up on an old thread (2008):

http://git.661346.n2.nabble.com/pull-preserve-merges-td1471688.html

I'd like to finally add a config parameter/setting to allow git pull to preserve
merges when it's rebasing. This addresses a somewhat common boundary case of a
locally merged feature branch getting flattened into master, as described here:

http://notes.envato.com/developers/rebasing-merge-commits-in-git/

This current patch adds a new `pull.preserve-merges` boolean config setting, but
we could also change the existing `pull.rebase` to be tri-state (so
`pull.rebase` can be true, false, or preserve-merges), or add a more generic
`pull.rebaseoptions` that is just a string of flags to pass to rebase.

Any of these would be fine with me--what would be preferred?

This patch doesn't update the docs, but I wanted to get an initial sanity check
on the preferred config setting before doing that.

Thanks!

- Stephen


Stephen Haberman (1):
  pull: Allow pull to preserve merges when rebasing.

 git-pull.sh | 11 +--
 t/t5520-pull.sh | 15 +++
 2 files changed, 24 insertions(+), 2 deletions(-)

-- 
1.8.1.2

--
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] pull: Allow pull to preserve merges when rebasing.

2013-08-08 Thread Stephen Haberman
If a user is working on master, and has merged in their feature branch, but now
has to "git pull" because master moved, with pull.rebase their feature branch
will be flattened into master.

This is because "git pull" currently does not know about rebase's preserve
merges flag, which would this behavior, and instead replay on the merge commit
of the feature branch onto the new master, and not the entire feature branch
itself.

Add a -p/--preserve-merges, to pass along git rebase if --rebase is in affect.

Also add a new pull.preserve-merges config setting, to enable this behavior as
the default.

Signed-off-by: Stephen Haberman 
---
 git-pull.sh | 11 +--
 t/t5520-pull.sh | 15 +++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index f0df41c..61d1efb 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -40,7 +40,7 @@ test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity= progress= recurse_submodules= verify_signatures=
-merge_args= edit=
+merge_args= edit= rebase_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
@@ -48,6 +48,10 @@ if test -z "$rebase"
 then
rebase=$(git config --bool pull.rebase)
 fi
+if [ $(git config --bool pull.preserve-merges) = "true" ] ;
+then
+   rebase_args=--preserve-merges
+fi
 dry_run=
 while :
 do
@@ -116,6 +120,9 @@ do
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
rebase=false
;;
+   -p|--preserve-merges)
+   rebase_args=--preserve-merges
+   ;;
--recurse-submodules)
recurse_submodules=--recurse-submodules
;;
@@ -292,7 +299,7 @@ fi
 merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
-   eval="git-rebase $diffstat $strategy_args $merge_args $verbosity"
+   eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args 
$verbosity"
eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
;;
 *)
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index ed4d9c8..2a2ee97 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -148,6 +148,21 @@ test_expect_success 'branch.to-rebase.rebase should 
override pull.rebase' '
test new = $(git show HEAD:file2)
 '
 
+test_expect_success 'preserve merges' '
+   git reset --hard before-rebase &&
+   test_config pull.rebase true &&
+   test_config pull.preserve-merges true &&
+   git checkout -b keep-merge second^ &&
+   echo new > file3 &&
+   git add file3 &&
+   git commit -m "new file3" &&
+   git checkout to-rebase &&
+   git merge keep-merge &&
+   git pull . copy &&
+   test $(git rev-parse HEAD^^) = $(git rev-parse copy) &&
+   test $(git rev-parse HEAD^2) = $(git rev-parse keep-merge)
+'
+
 test_expect_success '--rebase with rebased upstream' '
 
git remote add -f me . &&
-- 
1.8.1.2

--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Damien Robert
Matthieu Moy  wrote in message :
>> that confuses users.
> 
> ... but I do agree that the doc is really confusing. It would be much
> better if the doc could be reduced to:
> 
> "This is a synonym for linkgit:git-log[1] --raw --some --other ---options.
> Please refer to the documentation of that command."

If I may chime in as a user: what really confused me about git whatchanged
is this part of man gitcore-tutorial:

   To see the whole history of our pitiful little git-tutorial project,
   you can do

   $ git log


   which shows just the log messages, or if we want to see the log
   together with the associated patches use the more complex (and much
   more powerful)

   $ git whatchanged -p


   and you will see exactly what has changed in the repository over its
   short history.

I had to go look at the source code to realize that nowadays git log and
git whatchanged are basically the same functions with different defaults. A
pointer to that in the man page of git whatchanged would definitively be
helpful.

--
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] rm: remove unneeded null pointer check

2013-08-08 Thread Stefan Beller
As of 7612a1efdb (2006-06-09 git-rm: honor -n flag.) the variable
'pathspec' seems to be assumed to be never NULL after calling get_pathspec
There was a NULL pointer check after the seen = NULL assignment, which
was removed by that commit. So if pathspec would be NULL now, we'd segfault
in the line accessing the pathspec:
for (i = 0; pathspec[i] ; i++)

A few lines later, 'pathspec' still cannot be NULL, but that check was
overlooked, hence removing it now.

As the null pointer check was removed, it makes no sense to assign NULL
to seen and 3 lines later another value as there are no conditions in
between.

Signed-off-by: Stefan Beller 
---
 builtin/rm.c | 38 +-
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 0df0b4d..d00eaf8 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -277,8 +277,8 @@ static struct option builtin_rm_options[] = {
 
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
-   int i, newfd;
-   const char **pathspec;
+   int i, newfd, seen_any;
+   const char **pathspec, *match;
char *seen;
 
git_config(git_default_config, NULL);
@@ -314,7 +314,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
pathspec = get_pathspec(prefix, argv);
refresh_index(&the_index, REFRESH_QUIET, pathspec, NULL, NULL);
 
-   seen = NULL;
for (i = 0; pathspec[i] ; i++)
/* nothing */;
seen = xcalloc(i, 1);
@@ -328,27 +327,24 @@ int cmd_rm(int argc, const char **argv, const char 
*prefix)
list.entry[list.nr++].is_submodule = S_ISGITLINK(ce->ce_mode);
}
 
-   if (pathspec) {
-   const char *match;
-   int seen_any = 0;
-   for (i = 0; (match = pathspec[i]) != NULL ; i++) {
-   if (!seen[i]) {
-   if (!ignore_unmatch) {
-   die(_("pathspec '%s' did not match any 
files"),
-   match);
-   }
-   }
-   else {
-   seen_any = 1;
+
+   seen_any = 0;
+   for (i = 0; (match = pathspec[i]) != NULL ; i++) {
+   if (!seen[i]) {
+   if (!ignore_unmatch) {
+   die(_("pathspec '%s' did not match any files"),
+   match);
}
-   if (!recursive && seen[i] == MATCHED_RECURSIVELY)
-   die(_("not removing '%s' recursively without 
-r"),
-   *match ? match : ".");
}
-
-   if (! seen_any)
-   exit(0);
+   else {
+   seen_any = 1;
+   }
+   if (!recursive && seen[i] == MATCHED_RECURSIVELY)
+   die(_("not removing '%s' recursively without -r"),
+   *match ? match : ".");
}
+   if (!seen_any)
+   exit(0);
 
/*
 * If not forced, the file, the index and the HEAD (if exists)
-- 
1.8.4.rc1.25.gd121ba2

--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Matthieu Moy
Damien Robert  writes:

> Matthieu Moy  wrote in message :
>>> that confuses users.
>> 
>> ... but I do agree that the doc is really confusing. It would be much
>> better if the doc could be reduced to:
>> 
>> "This is a synonym for linkgit:git-log[1] --raw --some --other ---options.
>> Please refer to the documentation of that command."
>
> If I may chime in as a user: what really confused me about git whatchanged
> is this part of man gitcore-tutorial:

Indeed.

How about applying this, to reduce the number of references to
whatchanged in the docs?

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 3bdd56e..486a58b 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -818,7 +818,7 @@ for further details.
 'GIT_FLUSH'::
If this environment variable is set to "1", then commands such
as 'git blame' (in incremental mode), 'git rev-list', 'git log',
-   'git check-attr', 'git check-ignore', and 'git whatchanged' will
+   'git check-attr', and 'git check-ignore' will
force a flush of the output stream after each record have been
flushed. If this
variable is set to "0", the output of these commands will be done
diff --git a/Documentation/gitcore-tutorial.txt 
b/Documentation/gitcore-tutorial.txt
index f538a87..c6a1677 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -532,12 +532,7 @@ commit, and you can tell it to show a whole series of 
diffs.
 Alternatively, you can tell it to be "silent", and not show the diffs at
 all, but just show the actual commit message.
 
-In fact, together with the 'git rev-list' program (which generates a
-list of revisions), 'git diff-tree' ends up being a veritable fount of
-changes. A trivial (but very useful) script called 'git whatchanged' is
-included with Git which does exactly this, and shows a log of recent
-activities.
-
+'git log' can also be used to display changes introduced by some commits.
 To see the whole history of our pitiful little git-tutorial project, you
 can do
 
@@ -550,7 +545,7 @@ with the associated patches use the more complex (and much 
more
 powerful)
 
 
-$ git whatchanged -p
+$ git log --raw -p
 
 
 and you will see exactly what has changed in the repository over its


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Remove old forgotten command: whatchanged

2013-08-08 Thread Ramkumar Ramachandra
Damien Robert wrote:
> If I may chime in as a user

By all means. Do not feel inhibited to state your problems because you
are a "user": we are all users; we eventually became contributors
because we found certain things that needed fixing, and fixed them
little by little.
--
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] gc: reject if another gc is running, unless --force is given

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

> diff --git a/builtin/gc.c b/builtin/gc.c
> index 6be6c8d..99682f0 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -167,11 +167,69 @@ static int need_to_gc(void)
> + ...
> + fd = hold_lock_file_for_update(&lock, git_path("gc.pid"),
> +LOCK_DIE_ON_ERROR);
> + if (!force) {
> + fp = fopen(git_path("gc.pid"), "r");
> + memset(locking_host, 0, sizeof(locking_host));
> + should_exit =
> + fp != NULL &&
> + !fstat(fileno(fp), &st) &&
> + /*
> +  * 12 hour limit is very generous as gc should
> +  * never take that long. On the other hand we
> +  * don't really need a strict limit here,
> +  * running gc --auto one day late is not a big
> +  * problem. --force can be used in manual gc
> +  * after the user verifies that no gc is
> +  * running.
> +  */
> + time(NULL) - st.st_mtime <= 12 * 3600 &&
> + fscanf(fp, "%"PRIuMAX" %127c", &pid, locking_host) == 2 
> &&
> + !strcmp(locking_host, my_host) &&
> + !kill(pid, 0);

If there is a lockfile we can read, and if we can positively
determine that the process named in the lockfile is still running,
then we definitely do not want to do another "gc".  That part is
good.

If the lock is very stale, 12-hour test will kick in, and we do not
read who locked it, nor check if the locker is still alive.  By
doing so, we avoid misidentifying a new process that is unrelated to
the locker who died and left the lockfile behind, which is a good
thing.  The logic to ignore such lockfile as "unknown" applies
equally to a remote locker on a lockfile that is old.  So the logic
for an old lockfile is good, too.

When we see a recent lockfile created by a "gc" running elsewhere,
we do not set "should_exit".  Is that a good thing?  I am wondering
if the last two lines should be:

-   !strcmp(locking_host, my_host) &&
-   !kill(pid, 0);
+   (strcmp(locking_host, my_host) || !kill(pid, 0));

instead.

Thanks, looking good.

--
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] diff: remove ternary operator evaluating always to true

2013-08-08 Thread Stefan Beller
The line being changed is deep inside the function builtin_diff.
The variable name_b, which is used to evaluate the ternary expression
must evaluate to true at that position, hence the replacement with
just name_b.

The name_b variable only occurs a few times in that lengthy function:
As a parameter to the function itself:
static void builtin_diff(const char *name_a,
 const char *name_b,
...
The next occurrences are at:
/* Never use a non-valid filename anywhere if at all possible */
name_a = DIFF_FILE_VALID(one) ? name_a : name_b;
name_b = DIFF_FILE_VALID(two) ? name_b : name_a;

a_one = quote_two(a_prefix, name_a + (*name_a == '/'));
b_two = quote_two(b_prefix, name_b + (*name_b == '/'));

In the last line of this block 'name_b' is dereferenced and compared
to '/'. This would crash if name_b was NULL. Hence in the following code
we can assume name_b being non-null.

The next occurrence is just as a function argument, which doesn't change
the memory, which name_b points to, so the assumption name_b being not
null still holds:
emit_rewrite_diff(name_a, name_b, one, two,
textconv_one, textconv_two, o);

The next occurrence would be the line of this patch. As name_b still must
be not null, we can remove the ternary operator.


Inside the emit_rewrite_diff function there is a also a line
ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
which was also simplified as there is also a dereference before the
ternary operator.

Signed-off-by: Stefan Beller 
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 266112c..80f8439 100644
--- a/diff.c
+++ b/diff.c
@@ -669,7 +669,7 @@ static void emit_rewrite_diff(const char *name_a,
memset(&ecbdata, 0, sizeof(ecbdata));
ecbdata.color_diff = want_color(o->use_color);
ecbdata.found_changesp = &o->found_changes;
-   ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+   ecbdata.ws_rule = whitespace_rule(name_b);
ecbdata.opt = o;
if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
mmfile_t mf1, mf2;
@@ -2372,7 +2372,7 @@ static void builtin_diff(const char *name_a,
ecbdata.label_path = lbl;
ecbdata.color_diff = want_color(o->use_color);
ecbdata.found_changesp = &o->found_changes;
-   ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+   ecbdata.ws_rule = whitespace_rule(name_b);
if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
check_blank_at_eof(&mf1, &mf2, &ecbdata);
ecbdata.opt = o;
-- 
1.8.4.rc1.25.gd121ba2

--
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 v2] read-cache: save index entry updates in ILOG index extension

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

> Old operation's updates are removed as new ones are added to keep the
> size under 1 MB. ILOG keeps minimum 10 operations regardless of its
> size. These contansts should be configurable later one. ILOG content
> will be compressed later on so that it leaves minimum
> footprint.

List of  tuples would not compress well, I suspect.

> Because it's only needed at index writing time, read-only
> operations won't pay the cost for decompressing and compressing ILOG.

Another idea is to lazily read existing ILOG by (1) keeping just an
offset in the originating index file at read_index() time and (2)
reading it on demand when ":-4:Makefile" was asked for.

> diff --git a/cache.h b/cache.h
> index 85b544f..a2156bf 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -168,6 +168,7 @@ struct cache_entry {
>  
>  /* used to temporarily mark paths matched by pathspecs */
>  #define CE_MATCHED   (1 << 26)
> +#define CE_BASE  (1 << 27)

As this is not about pathspec match, please have its own comment
line (or a blank line, if this goes without comment) above this new
line.

> @@ -277,6 +278,7 @@ struct index_state {
>initialized : 1;
>   struct hash_table name_hash;
>   struct hash_table dir_hash;
> + struct strbuf *index_log;
>  };

Sane to have this as a per-index_state variable.

> +extern void log_index_changes(const char *prefix, const char **argv);

Not sane to name this function _index_anything and not to have index_state
as its first parameter, breaking the naming convention.

> diff --git a/read-cache.c b/read-cache.c
> index c3d5e35..4021667 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -14,6 +14,7 @@
>  #include "resolve-undo.h"
>  #include "strbuf.h"
>  #include "varint.h"
> +#include "quote.h"
>  
>  static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int 
> really);
>  
> @@ -33,8 +34,10 @@ static struct cache_entry *refresh_cache_entry(struct 
> cache_entry *ce, int reall
>  #define CACHE_EXT(s) ( (s[0]<<24)|(s[1]<<16)|(s[2]<<8)|(s[3]) )
>  #define CACHE_EXT_TREE 0x54524545/* "TREE" */
>  #define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* "REUC" */
> +#define CACHE_EXT_INDEX_LOG 0x494C4F47 /* "ILOG" */
>  
>  struct index_state the_index;
> +static struct strbuf log_message = STRBUF_INIT;

Not sane to have a single global here.  Give the callers a helper
function to prepare a ilog message into a strbuf they prepare before
they muck with argc/argv with parse_options(), and have them pass
that strbuf to

log_index_changes(struct index_state *, struct strbuf *);

and supply the usual

#define log_cache_changes(logmsg) log_index_changes(&the_index, 
(logmsg))

macro, inside "#ifndef NO_THE_INDEX_COMPATIBILITY_MACROS" section.

> @@ -1509,6 +1520,14 @@ int read_index_from(struct index_state *istate, const 
> char *path)
>   src_offset += extsize;
>   }
>   munmap(mmap, mmap_size);
> + if (istate == &the_index) {

Yuck.  Why can't the callers operate on their own copies of the
in-core index and get the same benefit?

> + for (i = 0; i < istate->cache_nr; i++) {
> + struct cache_entry *ce = istate->cache[i];
> + if (ce_stage(ce))
> + continue;
> + ce->ce_flags |= CE_BASE;
> + }
> + }
>   return istate->cache_nr;

> +static void get_updated_entries(struct index_state *istate,
> + struct cache_entry ***cache_out,
> + unsigned int *cache_nr_out)
> +{
> + struct cache_entry **cache;
> + unsigned int i, nr, cache_nr = 0;
> +
> + *cache_nr_out = 0;
> + *cache_out = NULL;
> + for (i = 0; i < istate->cache_nr; i++) {
> + if (istate->cache[i]->ce_flags & CE_BASE)
> + continue;
> + cache_nr++;
> + }
> + if (!cache_nr)
> + return;
> + cache = xmalloc(cache_nr * sizeof(*istate->cache));
> + for (i = nr = 0; i < istate->cache_nr; i++) {
> + struct cache_entry *ce = istate->cache[i];
> + if (ce->ce_flags & CE_BASE)
> + continue;
> + cache[nr++] = ce;
> + }
> + *cache_out = cache;
> + *cache_nr_out = cache_nr;
> +}

I can read what the function does, but I do not understand the
assumption this code makes.

Is this assuming that any newly created cache_entry that goes to
the_index via add_index_entry() will not have CE_BASE bit set?  Some
codepaths try to preserve the flags bit they do not care and/or
understand (e.g. rename_index_entry_at() creates a new ce with a new
name, and keeps most of the flags bit except for the name-hash state
bits), so CE_BASE will be propagated from the original to the new
one, I think.

You seem to be recording the state _after_ the change---that can be
read without the extension, can't it?  I thought we care more about
the state tha

Re: [PATCH v4 5/5] rm: delete .gitmodules entry of submodules removed from the work tree

2013-08-08 Thread Fredrik Gustafsson
On Thu, Aug 08, 2013 at 07:11:04PM +0200, Jens Lehmann wrote:
> Am 07.08.2013 20:28, schrieb Fredrik Gustafsson:
> > On Tue, Aug 06, 2013 at 02:11:56PM -0700, Junio C Hamano wrote:
> >> Thanks, will replace the top two commits and queue.  Looks like we
> >> are getting ready for 'next'?
> > 
> > I'm a bit curious about if we should move towards a reentrent libgit
> > (which would for example make multithreading easier) or not.
> 
> I'm not aware of such an effort in core Git (I always thought that
> libgit2 is the project doing what you seem to aim for).
> 
> > If so, I suggest that this patch only use die() in builtin/. However I
> > know that there's a lot of die() all over libgit today, I'm curious
> > about what direction we're heading.
> 
> The die() calls are just one part. Global variables are another issue,
> we have memory which is implicitly freed on exit ... so unless we
> commit ourselves to fix all those issues I see no point in moving the
> die() calls into builtin/ in my series.

Okay, thanks for your answer.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.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] git exproll: steps to tackle gc aggression

2013-08-08 Thread Junio C Hamano
Martin Fick  writes:

> Assuming I understand what you are suggesting, would these 
> "young object" likely still get "deduped" in an efficient 
> way without doing history traversal (it sounds like they 
> would)?

Yes.

The very first thing pack-object machinery does is to get the list
of object names and sort them in a certain order to help producing
good deltas, and this initial input preprocessing will dedup them.

> If so, then yeah this might be nice, especially if the history
> traversal is what would speed this up.

That was the assumption behind the "it might help" suggestion.  If
that helps or not is not known yet, and since Ram started this
subthread telling me not to talk about performance improvements, my
time on this thread is _not_ spent on that (yet).


--
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] diff: remove another ternary expression always evaluating to true

2013-08-08 Thread Stefan Beller
The condition before the changed line dereferences 'one' to query the mode,
so if the condition evaluates to true, the variable one must not be null.
Therefore we do not need the ternary operator depending on one, giving
either one->path or two->path. This always evaluates to one->path, so
we can remove the ternary operator.

The condition and the usage of the ternary operator have been introduced
by the same commit (752c0c24, 2009-10-19, Add the --submodule option to
the diff option family). As that commit message refers to a GitTogether
I'd assume that patch was crafted in a hurry, so maybe overlooking the
need for a ternary operator there.

Signed-off-by: Stefan Beller 
---
 diff.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 80f8439..f30b7e4 100644
--- a/diff.c
+++ b/diff.c
@@ -2252,8 +2252,7 @@ static void builtin_diff(const char *name_a,
(!two->mode || S_ISGITLINK(two->mode))) {
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
-   show_submodule_summary(o->file, one ? one->path : two->path,
-   line_prefix,
+   show_submodule_summary(o->file, one->path, line_prefix,
one->sha1, two->sha1, two->dirty_submodule,
meta, del, add, reset);
return;
-- 
1.8.4.rc1.25.gd121ba2

--
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] pull: Allow pull to preserve merges when rebasing.

2013-08-08 Thread Stephen Haberman

> This is because "git pull" currently does not know about rebase's
> preserve merges flag, which would this behavior, and instead replay
> on the merge commit of the feature branch onto the new master, and
> not the entire feature branch itself.

Ack, sorry, I was doing this too late last night--should say:

This is because "git pull" currently does not know about rebase's
preserve merges flag, which would avoid this behavior by replaying the
merge commit of the feature branch onto the new master, and not
replaying each individual commit in the feature branch.

- Stephen
--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread John Keeping
On Thu, Aug 08, 2013 at 08:06:09PM +0200, Matthieu Moy wrote:
> --- a/Documentation/gitcore-tutorial.txt
> +++ b/Documentation/gitcore-tutorial.txt
> @@ -532,12 +532,7 @@ commit, and you can tell it to show a whole series of 
> diffs.
>  Alternatively, you can tell it to be "silent", and not show the diffs at
>  all, but just show the actual commit message.
>  
> -In fact, together with the 'git rev-list' program (which generates a
> -list of revisions), 'git diff-tree' ends up being a veritable fount of
> -changes. A trivial (but very useful) script called 'git whatchanged' is
> -included with Git which does exactly this, and shows a log of recent
> -activities.
> -
> +'git log' can also be used to display changes introduced by some commits.
>  To see the whole history of our pitiful little git-tutorial project, you
>  can do
>  
> @@ -550,7 +545,7 @@ with the associated patches use the more complex (and 
> much more
>  powerful)

Isn't this paragraph slightly misleading now?  We're not using a
different command any more so this could say:

which shows just the log messages, or if we want to see the log together
with the associated patches use the `--patch` option


>  
> -$ git whatchanged -p
> +$ git log --raw -p
>  

Then this can be "git log --patch", since it seems that the surrounding
text doesn't actually care that whatchanged prints the raw diffstat.
--
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 exproll: steps to tackle gc aggression

2013-08-08 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
> Martin Fick  writes:
>> Assuming I understand what you are suggesting, would these
>> "young object" likely still get "deduped" in an efficient
>> way without doing history traversal (it sounds like they
>> would)?
>
> Yes.
>
> The very first thing pack-object machinery does is to get the list
> of object names and sort them in a certain order to help producing
> good deltas, and this initial input preprocessing will dedup them.

So, the proposal is to create an index of young objects without doing
reachability analysis (I still didn't get the point of packing them;
as I pointed out, it seems to be rather counter-productive) to help
the actual packing? From what I vaguely understood:

1. Index all the young objects to save a history traversal (?)

2. Perform the reachability analysis using the index in step 1, and
then generate the pack.

I'm not yet clear about what information 1 contains to help 2. Is it
the rough ordering? (The big important objects come near the top of
the pack, and the deltas are generated against them). I say "rough"
because the ordering might change after the unreachable objects are
pruned.

*scratches head*
--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Junio C Hamano
Damien Robert  writes:

> Matthieu Moy  wrote in message :
>>> that confuses users.
>> 
>> ... but I do agree that the doc is really confusing. It would be much
>> better if the doc could be reduced to:
>> 
>> "This is a synonym for linkgit:git-log[1] --raw --some --other ---options.
>> Please refer to the documentation of that command."
>
> If I may chime in as a user: what really confused me about git whatchanged
> is this part of man gitcore-tutorial:
>
>To see the whole history of our pitiful little git-tutorial project,
>you can do
>
>$ git log
>
>
>which shows just the log messages, or if we want to see the log
>together with the associated patches use the more complex (and much
>more powerful)
>
>$ git whatchanged -p
>
>
>and you will see exactly what has changed in the repository over its
>short history.
>
> I had to go look at the source code to realize that nowadays git log and
> git whatchanged are basically the same functions with different defaults. A
> pointer to that in the man page of git whatchanged would definitively be
> helpful.

The "tutorial" was written in fairly early days of Git's history, in
order to primarily help those who want to use the plumbing command
to script their own Porcelain commands.  As it says at the very
beginning, the end-user tutorial to use Git's Porcelain is
gittutorial.txt and the user manual, not this document.

The above section primarily explains the use of diff-tree and it was
appropriate back when git-whatchanged was a script.  The intent of
the whole document, not just this section, was to tickle the
curiousity of the users and encourage them to see how the above
"much more powerful" whatchanged was implemented by going to the
source.

Which no longer is easy to do, as we have many Porcelains
reimplemented in C.

I think a good way forward is to rewrite the above to:

To see the whole history of our pitiful little git-tutorial
project, you can do

$ git log

which shows just the log messages, or if we want to see the
log together with the associated patches, use

$ git log -p

and you will see exactly what has changed in the repository
over its short history.

and show a skeleton scripted implementation of "git log" using the
plumbing commands which would be essentially "git rev-list" piped to
"git diff-tree --stdin".  Either have it as an example under
contrib/examples, or as part of the text in this section of this
document.
--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Junio C Hamano
Matthieu Moy  writes:

> Damien Robert  writes:
>
>> Matthieu Moy  wrote in message :
 that confuses users.
>>> 
>>> ... but I do agree that the doc is really confusing. It would be much
>>> better if the doc could be reduced to:
>>> 
>>> "This is a synonym for linkgit:git-log[1] --raw --some --other ---options.
>>> Please refer to the documentation of that command."
>>
>> If I may chime in as a user: what really confused me about git whatchanged
>> is this part of man gitcore-tutorial:
>
> Indeed.
>
> How about applying this, to reduce the number of references to
> whatchanged in the docs?

I fully agree with the change to git.txt

I am slightly negative about the rest.

gitcore-tutorial is about teaching people who want to script around
plumbing to do their own Porcelain.  It is *not* about teaching them
"if you want to see the history, use 'git log'".

It is meant to teach them "if you want to do your own 'git log', you
can do so with 'rev-list' piped to 'diff-tree --stdin'".  Changing
'whatchanged' to 'log' in the latter statement is an improvement,
but dropping 'can be done by combining rev-list and diff-tree' goes
against the objective of the whole document.

If we were to be spending more braincycles to update the latter, I
think we should enrich it by finding ways to resurrect good examples
that were lost over time due to "rewrite in C and make them builtin"
process.

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 3bdd56e..486a58b 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -818,7 +818,7 @@ for further details.
>  'GIT_FLUSH'::
> If this environment variable is set to "1", then commands such
> as 'git blame' (in incremental mode), 'git rev-list', 'git log',
> -   'git check-attr', 'git check-ignore', and 'git whatchanged' will
> +   'git check-attr', and 'git check-ignore' will
> force a flush of the output stream after each record have been
> flushed. If this
> variable is set to "0", the output of these commands will be done
> diff --git a/Documentation/gitcore-tutorial.txt 
> b/Documentation/gitcore-tutorial.txt
> index f538a87..c6a1677 100644
> --- a/Documentation/gitcore-tutorial.txt
> +++ b/Documentation/gitcore-tutorial.txt
> @@ -532,12 +532,7 @@ commit, and you can tell it to show a whole series of 
> diffs.
>  Alternatively, you can tell it to be "silent", and not show the diffs at
>  all, but just show the actual commit message.
>  
> -In fact, together with the 'git rev-list' program (which generates a
> -list of revisions), 'git diff-tree' ends up being a veritable fount of
> -changes. A trivial (but very useful) script called 'git whatchanged' is
> -included with Git which does exactly this, and shows a log of recent
> -activities.
> -
> +'git log' can also be used to display changes introduced by some commits.
>  To see the whole history of our pitiful little git-tutorial project, you
>  can do
>  
> @@ -550,7 +545,7 @@ with the associated patches use the more complex (and 
> much more
>  powerful)
>  
>  
> -$ git whatchanged -p
> +$ git log --raw -p
>  
>  
>  and you will see exactly what has changed in the repository over its
--
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 exproll: steps to tackle gc aggression

2013-08-08 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> I asked you a very simple question: what happens when I do "git push"?

You asked "does push send unreachable cruft?" and I said No.  Does
that answer your question?

> 3. After a few days of working, the gc heuristics figure out that I
> have too much garbage and too many packs; a cleanup is required. The
> gc --auto which doesn't tolerate fragmentation: it tries to put
> everything into one large pack.

Today's "gc --auto" skims the history shallowly and packs loose
objects into a single _new_ pack.  If you start from a single pack
and enough loose objects and run "gc --auto", you will have two
packs, one intact from the original, one new.

> We're talking about tackling the gc aggression problem in 3.

> when I say "git push" (or when gc decides to repack), won't I need
> to explode the young pack into loose objects, do a reachability
> analysis, and repack anyway?

You do not explode anything.  "push" will read objects from
anywhere, either from loose or packed, and send a newly created pack
over the wire.

And I see from "(or when ...)" part that you do not know what you
are talking about.  "push" will not stream existing pack to the
other end without computation, and it never will.  It will reuse
existing individual pieces when it can, and having data in a pack
(even without deltifying, or as a suboptimal delta) is much better
for push performance than having the same data in a loose object if
only for this reason, as you do not have to recompress and reencode
it in a different way (loose objects and undelitifed object in pack
are encoded differently).

> ... are you willing to explain it to me?

Hope that the above clarifies.

I've treated this thread as a design discussion, not an education
session, but the above ended up having more education material than
anything that would advance the design.
--
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 exproll: steps to tackle gc aggression

2013-08-08 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
>> 3. After a few days of working, the gc heuristics figure out that I
>> have too much garbage and too many packs; a cleanup is required. The
>> gc --auto which doesn't tolerate fragmentation: it tries to put
>> everything into one large pack.
>
> Today's "gc --auto" skims the history shallowly and packs loose
> objects into a single _new_ pack.  If you start from a single pack
> and enough loose objects and run "gc --auto", you will have two
> packs, one intact from the original, one new.

That is when I have lots of loose objects and few packs. We are
discussing gc aggression when there are too many packs (how did we get
there in the first place if new packs aren't created?): in which case
it doesn't tolerate fragmentation, and tries to put everything in one
pack. That is both IO and CPU intensive, and we've been trying to
tackle that since the start of the thread.

> And I see from "(or when ...)" part that you do not know what you
> are talking about.  "push" will not stream existing pack to the
> other end without computation, and it never will.  It will reuse
> existing individual pieces when it can, and having data in a pack
> (even without deltifying, or as a suboptimal delta) is much better
> for push performance than having the same data in a loose object if
> only for this reason, as you do not have to recompress and reencode
> it in a different way (loose objects and undelitifed object in pack
> are encoded differently).

Certainly. A push will never use an existing pack as-is, as it's very
highly unlikely that the server requested exactly what gc --auto
packed for us locally.

Sure, undeltified objects in the pack are probably better for push
performance, but I'm talking about the majority: deltified objects.
Don't you need to apply the deltas (ie. explode the pack), before you
can recompute the deltas for the information you're sending across for
the push?

> I've treated this thread as a design discussion, not an education
> session, but the above ended up having more education material than
> anything that would advance the design.

You will need to educate your contributors and potential contributors
if you want these problems to be fixed.
--
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 status resets permissions on index file -- Ubuntu 12.04 64b

2013-08-08 Thread Justin Collum
I've run into a strange situation with git lately. It seems that
anything I do involving git will alter the permissions on my index
file to the point that I can't do anything until I re-add the
permissions on the file.

Looks like a bug to me, is it? It does seem like this has started
happening since I moved over to 64b Ubuntu.

$ ll .git
total 156K
...
drwxrwxrwx   2 dev dev 4.0K Jul 23 09:30 hooks
-rwxrwxrwx   1 dev dev  17K Aug  8 13:12 index
drwxrwxrwx   2 dev dev 4.0K Jul 19 09:31 info
...

$ gs
# On branch build-0.3
# Your branch is ahead of 'staging/build-0.3' by 5 commits.
#   (use "git push" to publish your local commits)
#
# Untracked files:
#   (use "git add ..." to include in what will be committed)
#
# scripts/loadMongo.coffee
nothing added to commit but untracked files present (use "git add" to track)

$ ll .git
total 156K
...
drwxrwxrwx   2 dev dev 4.0K Jul 23 09:30 hooks
-rw-rw-r--   1 dev dev  17K Aug  8 13:16 index   # <---
this line  <---
drwxrwxrwx   2 dev dev 4.0K Jul 19 09:31 info
...

$ git --version
git version 1.8.3.4

Ubuntu:
Distributor ID: Ubuntu
Description: Ubuntu 12.04.2 LTS
Release: 12.04
Codename: precise
--
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] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!

2013-08-08 Thread Phil Hord
On Thu, Aug 8, 2013 at 3:07 AM, Luke San Antonio
 wrote:
>
> Hi, my name's Luke!
>
> Today, I had a problem merging a stash after immediately creating it.
> This is exactly what I did!
>
> git stash save --keep-index
> git stash pop
>
> And BAM! Merge conflict! This was especially weird because my file had
> this in it (taken directly from my code!)
>

Luke,

I think the issue is that your working directory receives your cached
file when you say 'git stash --keep-index'.  When you restore the
stash, your previous working directory now conflicts with your new
working directory, but neither is the same as HEAD.

Here's a test script to demonstrate the issue, I think.  Did I get
this right, Luke?

 # cd /tmp && rm -rf foo
 git init foo && cd foo
 echo "foo" > bar &&  git add bar && git commit -mfoo
 echo "bar" > bar &&  git add bar
 echo "baz" > bar
 echo "Before stash  bar: $(cat bar)"
 git stash --keep-index
 echo "After stash  bar: $(cat bar)"
 git stash apply


The output looks like this:

$  git init foo && cd foo
Initialized empty Git repository in /tmp/foo/.git/
$ git commit --allow-empty -mInitialCommit
[master (root-commit) b5ecc7e] InitialCommit
$ echo "Bar" > bar &&  git add bar && git commit -mBar
[master 16d708b] Bar
 1 file changed, 1 insertion(+)
 create mode 100644 bar
$ echo "bar" > bar &&  git add bar
$  echo "baz" > bar
$  echo "Before stash  bar: $(cat bar)"
Before stash  bar: baz
$  git stash --keep-index
Saved working directory and index state WIP on master: 16d708b Bar
HEAD is now at 16d708b Bar
$  echo "After stash  bar: $(cat bar)"
After stash  bar: bar
$  git stash apply
Auto-merging bar
CONFLICT (content): Merge conflict in bar
Recorded preimage for 'bar'
$ cat bar
<<< Updated upstream
bar
===
baz
>>> Stashed changes



Phil
--
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] diff: remove another ternary expression always evaluating to true

2013-08-08 Thread Johannes Schindelin
Hi Stefan,

On Thu, 8 Aug 2013, Stefan Beller wrote:

> The condition before the changed line dereferences 'one' to query the mode,
> so if the condition evaluates to true, the variable one must not be null.

To show this better, please use -U10 (or some other appropriate context
option) in the future.

> Therefore we do not need the ternary operator depending on one, giving
> either one->path or two->path. This always evaluates to one->path, so we
> can remove the ternary operator.
> 
> The condition and the usage of the ternary operator have been introduced
> by the same commit (752c0c24, 2009-10-19, Add the --submodule option to
> the diff option family). As that commit message refers to a GitTogether
> I'd assume that patch was crafted in a hurry, so maybe overlooking the
> need for a ternary operator there.

If this is my code, I do not need a GitTogether to excuse my sloppiness.
In this particular case, I imagine the appropriate fix is to test for
one->path instead of removing the conditional, though.

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


Re: [PATCH] git exproll: steps to tackle gc aggression

2013-08-08 Thread Martin Langhoff
On Thu, Aug 8, 2013 at 4:04 PM, Ramkumar Ramachandra  wrote:
> You will need to educate your contributors and potential contributors
> if you want these problems to be fixed.

TBH, I think Junio is exceedingly kind and generous with his time.

IME (and there's quite a few years of it :-) ), good contributors
learn a little bit asking, and ton on their own.

Your tone is quite demanding, which I personally find... rather out of place.



m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
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] replace: forbid replacing an object with one of a different type

2013-08-08 Thread Philip Oakley

From: "Christian Couder" 
Sent: Wednesday, August 07, 2013 5:42 AM

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

The doc will be updated in a later patch.

Signed-off-by: Christian Couder 
---
If this patch is considered useful, I will update the doc and
maybe add tests.


Acked-by: Philip Oakley 
Improved documentation would always be useful.



builtin/replace.c | 9 +
1 file changed, 9 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..0246ab3 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref,
const char *replace_ref,
   int force)
{
 unsigned char object[20], prev[20], repl[20];
+ enum object_type obj_type, repl_type;
 char ref[PATH_MAX];
 struct ref_lock *lock;

@@ -100,6 +101,14 @@ static int replace_object(const char *object_ref,
const char *replace_ref,
 if (check_refname_format(ref, 0))
 die("'%s' is not a valid ref name.", ref);

+ obj_type = sha1_object_info(object, NULL);
+ repl_type = sha1_object_info(repl, NULL);


Do (very) large blobs have any issues here? As I understand it, such
blobs, as with other smaller objects, need to be expanded to determine
the type. Is there a heuristic (and is it worth it) to do a 'large
object == blob' initial check to avoid such an expansion? Small objects
shouldn't be an overhead.

Just a thought.


+ if (obj_type != repl_type)
+ die("Object ref '%s' is of type '%s'\n"
+ "while replace ref '%s' is of type '%s'.",
+ object_ref, typename(obj_type),
+ replace_ref, typename(repl_type));


Perhaps start with "Objects must be of the same type.\n"


+
 if (read_ref(ref, prev))
 hashclr(prev);
 else if (!force)
--
1.8.4.rc1.22.g132b1a0


--
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: Repo with only one file

2013-08-08 Thread Phil Hord
On Wed, Aug 7, 2013 at 5:07 PM, shawn wilson  wrote:
> On Wed, Aug 7, 2013 at 6:43 AM, Johannes Sixt  wrote:
>> Am 8/7/2013 8:24, schrieb shawn wilson:> ... create a repo for one of
>>> these scripts and I'd like to keep the commit history.
>>>
>>> Ok, so:
>>> % find -type f ! -iname "webban.pl" | while read f; do git
>>> filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f"
>>> HEAD ; done
>>>
>>> Which basically did it. But, I've got this one commit that seems to be
>>> orphaned - it doesn't change any files.
>>
>> Try this:
>>
>>   git filter-branch HEAD -- webban.pl
>>
>
>  % git filter-branch HEAD -- webban.pl
> Cannot create a new backup.
> A previous backup already exists in refs/original/
> Force overwriting the backup with -f
>  % git filter-branch -f HEAD -- webban.pl
> Rewrite 1e04b18c256c996312f167be808733bcc755f1e3 (9/9)
> WARNING: Ref 'refs/heads/master' is unchanged

I think you can ignore the warning.  Maybe you want to create a new
branch which only has this file in it now.

   $ git checkout -b webban

Phil
--
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] pull: Allow pull to preserve merges when rebasing.

2013-08-08 Thread Johannes Schindelin
Hi Stephen,

On Thu, 8 Aug 2013, Stephen Haberman wrote:

> If a user is working on master, and has merged in their feature branch,
> but now has to "git pull" because master moved, with pull.rebase their
> feature branch will be flattened into master.
> 
> This is because "git pull" currently does not know about rebase's
> preserve merges flag, which would this behavior, and instead replay on
> the merge commit of the feature branch onto the new master, and not the
> entire feature branch itself.
> 
> Add a -p/--preserve-merges, to pass along git rebase if --rebase is in affect.

ACK!

> Also add a new pull.preserve-merges config setting, to enable this
> behavior as the default.

This should probably be added to config.txt and
Documentation/git-pull.txt, too, right?

FYI I started to rewrite the complete --preserve-merges support for the
interactive rebase some time ago, based on the experience of the merging
rebases:

https://github.com/msysgit/git/commit/b733454b

(part of the rebase-i-p branch). The idea is to use the 'exec' command of
the interactive rebase to do a much better job, and to allow reordering
(and in particular fixup commits) even when trying to preserve merges.

Unfortunately, the resulting branches look slightly differently now,
breaking the (horribly complicated -- my fault!) unit tests, and due to an
utter lack of time I had to stall that project.

Feel free to play with it if you want!

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


Re: [PATCH] diff: remove another ternary expression always evaluating to true

2013-08-08 Thread Stefan Beller
On 08/08/2013 11:01 PM, Johannes Schindelin wrote:
> Hi Stefan,
> 
> On Thu, 8 Aug 2013, Stefan Beller wrote:
> 
>> The condition before the changed line dereferences 'one' to query the mode,
>> so if the condition evaluates to true, the variable one must not be null.
> 
> To show this better, please use -U10 (or some other appropriate context
> option) in the future.
> 
>> Therefore we do not need the ternary operator depending on one, giving
>> either one->path or two->path. This always evaluates to one->path, so we
>> can remove the ternary operator.
>>
>> The condition and the usage of the ternary operator have been introduced
>> by the same commit (752c0c24, 2009-10-19, Add the --submodule option to
>> the diff option family). As that commit message refers to a GitTogether
>> I'd assume that patch was crafted in a hurry, so maybe overlooking the
>> need for a ternary operator there.
> 
> If this is my code, I do not need a GitTogether to excuse my sloppiness.
> In this particular case, I imagine the appropriate fix is to test for
> one->path instead of removing the conditional, though.
> 
> Ciao,
> Johannes
> 

Ok, here is a resend with -U10
I forgot about the -U10 as gitk shows a different number of lines 
of context around. Is there a way to configure gitk to show less lines
of code or a default -U10 for send-email/format-patch?

So you rather propose to have 
-   show_submodule_summary(o->file, one ? one->path : two->path,
+   show_submodule_summary(o->file, one->path ? one->path : 
two->path,

instead of the patch below?
(The test suite run without any problem using that patch)

Stefan

--8<--
From 3a580c51f0bf70745f26eb5045d646dfead2afd3 Mon Sep 17 00:00:00 2001
From: Stefan Beller 
Date: Thu, 8 Aug 2013 20:54:24 +0200
Subject: [PATCH] diff: remove another ternary expression always evaluating to
 true

The condition before the changed line dereferences 'one' to query the mode,
so if the condition evaluates to true, the variable one must not be null.
Therefore we do not need the ternary operator depending on one, giving
either one->path or two->path. This always evaluates to one->path, so
we can remove the ternary operator.

The condition and the usage of the ternary operator have been introduced
by the same commit (752c0c24, 2009-10-19, Add the --submodule option to
the diff option family). As that commit message refers to a GitTogether
I'd assume that patch was crafted in a hurry, so maybe overlooking the
need for a ternary operator there.

Signed-off-by: Stefan Beller 
---
 diff.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 80f8439..f30b7e4 100644
--- a/diff.c
+++ b/diff.c
@@ -2245,22 +2245,21 @@ static void builtin_diff(const char *name_a,
struct userdiff_driver *textconv_one = NULL;
struct userdiff_driver *textconv_two = NULL;
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
 
if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
(!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
-   show_submodule_summary(o->file, one ? one->path : two->path,
-   line_prefix,
+   show_submodule_summary(o->file, one->path, line_prefix,
one->sha1, two->sha1, two->dirty_submodule,
meta, del, add, reset);
return;
}
 
if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
textconv_one = get_textconv(one);
textconv_two = get_textconv(two);
}
 
-- 
1.8.4.rc1.25.gd121ba2




signature.asc
Description: OpenPGP digital signature


Re: git status resets permissions on index file -- Ubuntu 12.04 64b

2013-08-08 Thread Stefan Beller
On 08/08/2013 10:27 PM, Justin Collum wrote:
> I've run into a strange situation with git lately. It seems that
> anything I do involving git will alter the permissions on my index
> file to the point that I can't do anything until I re-add the
> permissions on the file.
> 
> Looks like a bug to me, is it? It does seem like this has started
> happening since I moved over to 64b Ubuntu.
> 
> $ ll .git
> total 156K
> ...
> drwxrwxrwx   2 dev dev 4.0K Jul 23 09:30 hooks
> -rwxrwxrwx   1 dev dev  17K Aug  8 13:12 index
> drwxrwxrwx   2 dev dev 4.0K Jul 19 09:31 info
> ...
> 
> $ gs
> # On branch build-0.3
> # Your branch is ahead of 'staging/build-0.3' by 5 commits.
> #   (use "git push" to publish your local commits)
> #
> # Untracked files:
> #   (use "git add ..." to include in what will be committed)
> #
> # scripts/loadMongo.coffee
> nothing added to commit but untracked files present (use "git add" to track)
> 
> $ ll .git
> total 156K
> ...
> drwxrwxrwx   2 dev dev 4.0K Jul 23 09:30 hooks
> -rw-rw-r--   1 dev dev  17K Aug  8 13:16 index   # <---
> this line  <---
> drwxrwxrwx   2 dev dev 4.0K Jul 19 09:31 info
> ...
> 
> $ git --version
> git version 1.8.3.4
> 
> Ubuntu:
> Distributor ID: Ubuntu
> Description: Ubuntu 12.04.2 LTS
> Release: 12.04
> Codename: precise

The permissions are set to reading for all and writing for you(r user)
and your group. This should be no problem with standard git commands.
Before you had the index file executable, why would you need that?

What kind of filesystem do you have? (The output of 'mount' would be 
interesting) 

Also what exactly breaks? ("can't do anything") 
What commands do not behave as you'd expect? 

Stefan



signature.asc
Description: OpenPGP digital signature


Re: [PATCH] pull: Allow pull to preserve merges when rebasing.

2013-08-08 Thread Stephen Haberman
Hi Johannes,

> This should probably be added to config.txt and
> Documentation/git-pull.txt, too, right?

Yep, I meant to note that I'd do that after getting an initial
confirmation that the pull.preserve-merges was the preferred approach.

(I was being lazy and didn't want to write up docs only to switch to
overloading pull.rebase or what not.)

But I'll go ahead and do that.

>   https://github.com/msysgit/git/commit/b733454b

Interesting!

> Feel free to play with it if you want!

I'll poke around out of curiosity, but no promises, as, yes, this is a
tricky bit of functionality that can quickly lead to a lot of lost
sleep. :-)

- Stephen


--
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] diff: remove another ternary expression always evaluating to true

2013-08-08 Thread Philip Oakley

From: "Stefan Beller" 
Sent: Thursday, August 08, 2013 7:55 PM
Subject: [PATCH] diff: remove another ternary expression always 
evaluating to true


Have these issues (and the earlier expression simplifications patches 
$gmane/231916, $gmane/231912 ) been discovered with the "STACK" tool I'd 
noted in $gmane/230542 which you were also interested in (I've not had 
chance to run the tool).


If so, it's probably worth referencing the tool and the paper which 
explains the broader issues.


Philip


The condition before the changed line dereferences 'one' to query the 
mode,
so if the condition evaluates to true, the variable one must not be 
null.

Therefore we do not need the ternary operator depending on one, giving
either one->path or two->path. This always evaluates to one->path, so
we can remove the ternary operator.

The condition and the usage of the ternary operator have been 
introduced
by the same commit (752c0c24, 2009-10-19, Add the --submodule option 
to
the diff option family). As that commit message refers to a 
GitTogether

I'd assume that patch was crafted in a hurry, so maybe overlooking the
need for a ternary operator there.

Signed-off-by: Stefan Beller 
---
diff.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 80f8439..f30b7e4 100644
--- a/diff.c
+++ b/diff.c
@@ -2252,8 +2252,7 @@ static void builtin_diff(const char *name_a,
 (!two->mode || S_ISGITLINK(two->mode))) {
 const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
 const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
- show_submodule_summary(o->file, one ? one->path : two->path,
- line_prefix,
+ show_submodule_summary(o->file, one->path, line_prefix,
 one->sha1, two->sha1, two->dirty_submodule,
 meta, del, add, reset);
 return;
--
1.8.4.rc1.25.gd121ba2

--


--
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] diff: remove another ternary expression always evaluating to true

2013-08-08 Thread Johannes Schindelin
Hi Stefan,

On Thu, 8 Aug 2013, Stefan Beller wrote:

> So you rather propose to have 
> - show_submodule_summary(o->file, one ? one->path : two->path,
> + show_submodule_summary(o->file, one->path ? one->path : 
> two->path,

I do. The reason is that one->path could be NULL (but not both one->path
and two->path) and the summary would not be as helpful as possible if it
wrote "(null)" instead of the path of the submodule.

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


Re: [PATCH] replace: forbid replacing an object with one of a different type

2013-08-08 Thread Junio C Hamano
"Philip Oakley"  writes:

> From: "Christian Couder" 
> Sent: Wednesday, August 07, 2013 5:42 AM
>> Users replacing an object with one of a different type were not
>> prevented to do so, even if it was obvious, and stated in the doc,
>> that bad things would result from doing that.
>>
>> To avoid mistakes, it is better to just forbid that though.
>>
>> The doc will be updated in a later patch.
>>
>> Signed-off-by: Christian Couder 
>> ---
>> If this patch is considered useful, I will update the doc and
>> maybe add tests.
>
> Acked-by: Philip Oakley 
> Improved documentation would always be useful.

Will wait for a reroll with tests and doc updates, then.

>> @@ -100,6 +101,14 @@ static int replace_object(const char *object_ref,
>> const char *replace_ref,
>>  if (check_refname_format(ref, 0))
>>  die("'%s' is not a valid ref name.", ref);
>>
>> + obj_type = sha1_object_info(object, NULL);
>> + repl_type = sha1_object_info(repl, NULL);
>
> Do (very) large blobs have any issues here?  As I understand it, such
> blobs, as with other smaller objects, need to be expanded to determine
> the type.

sha1_object_info() is coded to peek into only the early part of a
loose object or the object header part of a packed object.  Loose
ones we do mmap(), but we only inflate a tiny bit (the first 32
bytes).

>> + if (obj_type != repl_type)
>> + die("Object ref '%s' is of type '%s'\n"
>> + "while replace ref '%s' is of type '%s'.",
>> + object_ref, typename(obj_type),
>> + replace_ref, typename(repl_type));
>
> Perhaps start with "Objects must be of the same type.\n"

Yes.
--
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] pull: Allow pull to preserve merges when rebasing.

2013-08-08 Thread Philip Oakley

From: "Stephen Haberman" 

Hi Johannes,


This should probably be added to config.txt and
Documentation/git-pull.txt, too, right?


Yep, I meant to note that I'd do that after getting an initial
confirmation that the pull.preserve-merges was the preferred approach.

(I was being lazy and didn't want to write up docs only to switch to
overloading pull.rebase or what not.)

But I'll go ahead and do that.


https://github.com/msysgit/git/commit/b733454b


Interesting!


Feel free to play with it if you want!


I'll poke around out of curiosity, but no promises, as, yes, this is a
tricky bit of functionality that can quickly lead to a lot of lost
sleep. :-)

- Stephen


Johannes also kindly explained his merging-rebase script to me on the 
msysgit list a few days ago 
https://groups.google.com/forum/?hl=en_US?hl%3Den#!topic/msysgit/LiPa2T_K4C4 
which shows how msysgit and git both keep parallel lines of development 
with fast forwarding and rebasing at the same time.


The technique should also help those case for keeping 
private/independent lines of development that are discussed often.


Philip

--
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] pull: Allow pull to preserve merges when rebasing.

2013-08-08 Thread Junio C Hamano
Stephen Haberman  writes:

> Hi Johannes,
>
>> This should probably be added to config.txt and
>> Documentation/git-pull.txt, too, right?
>
> Yep, I meant to note that I'd do that after getting an initial
> confirmation that the pull.preserve-merges was the preferred approach.

If you were to go that route, no dashes in the last component of
configuration variable names, please.

> (I was being lazy and didn't want to write up docs only to switch to
> overloading pull.rebase or what not.)

I think we have a recent update that allows you to say

[pull]
rebase = false

to mean "I want 'git pull' to use merge".  Currently the other
choice is:

[pull]
rebase = true

to say "I want to run 'git pull --rebase'".  I do not think it is
unreasonable to extend it further so that

[pull]
rebase = preserve

is understood.
--
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] diff: remove another ternary expression always evaluating to true

2013-08-08 Thread Junio C Hamano
Johannes Schindelin  writes:

> Hi Stefan,
>
> On Thu, 8 Aug 2013, Stefan Beller wrote:
>
>> So you rather propose to have 
>> -show_submodule_summary(o->file, one ? one->path : two->path,
>> +show_submodule_summary(o->file, one->path ? one->path : 
>> two->path,
>
> I do. The reason is that one->path could be NULL (but not both one->path
> and two->path) and the summary would not be as helpful as possible if it
> wrote "(null)" instead of the path of the submodule.

Good.

Also some C libraries would choke when asked to %s format a NULL,
instead of giving "(null)" to avoid segfaulting (which I think is a
quirk in glibc).
--
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] diff: fix a possible null pointer dereference

2013-08-08 Thread Stefan Beller
The condition in the ternary operator was wrong, hence the wrong char
pointer could be used as the parameter for show_submodule_summary.
one->path may be null, but we definitely need a non null path given
to the function.

Signed-off-by: Stefan Beller 
Acked-By: Johannes Schindelin 
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 80f8439..061694b 100644
--- a/diff.c
+++ b/diff.c
@@ -2252,7 +2252,7 @@ static void builtin_diff(const char *name_a,
(!two->mode || S_ISGITLINK(two->mode))) {
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
-   show_submodule_summary(o->file, one ? one->path : two->path,
+   show_submodule_summary(o->file, one->path ? one->path : 
two->path,
line_prefix,
one->sha1, two->sha1, two->dirty_submodule,
meta, del, add, reset);
-- 
1.8.4.rc1.25.gd121ba2

--
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] diff: remove another ternary expression always evaluating to true

2013-08-08 Thread Stefan Beller
On 08/08/2013 11:36 PM, Philip Oakley wrote:
> From: "Stefan Beller" 
> Sent: Thursday, August 08, 2013 7:55 PM
> Subject: [PATCH] diff: remove another ternary expression always
> evaluating to true
> 
> Have these issues (and the earlier expression simplifications patches
> $gmane/231916, $gmane/231912 ) been discovered with the "STACK" tool I'd
> noted in $gmane/230542 which you were also interested in (I've not had
> chance to run the tool).
> 
> If so, it's probably worth referencing the tool and the paper which
> explains the broader issues.
> 
> Philip
> 

Yes, those 3 issues have been discovered using the STACK tool
The paper regarding that tool can be found at
http://pdos.csail.mit.edu/~xi/papers/stack-sosp13.pdf

It's definitely an interesting read! (At least for me it was ;)
The authors intend to make that tool available to broader public
as open source in August this year, iirc.

However I do not know if their repository address was
already announced publicly, so I did not announce to have used it.
(Last time I announced using static code analysis tools for
my patches the review-process was much harder as well :D)

Stefan



signature.asc
Description: OpenPGP digital signature


Re: git status resets permissions on index file -- Ubuntu 12.04 64b

2013-08-08 Thread Andrew Ruder
On Thu, Aug 08, 2013 at 11:35:35PM +0200, Stefan Beller wrote:
> On 08/08/2013 10:27 PM, Justin Collum wrote:
> > [...]
> > -rwxrwxrwx   1 dev dev  17K Aug  8 13:12 index
> > [...]
> > -rw-rw-r--   1 dev dev  17K Aug  8 13:16 index   # <---
> 
> The permissions are set to reading for all and writing for you(r user)
> and your group. This should be no problem with standard git commands.
> Before you had the index file executable, why would you need that?

I'm about 90% sure the issue he's having is that the write bit for
other/world goes away and he is neither the user dev or the group dev
and the reason for all the executable bits is that he is regularly
running

chmod -R 777 .

Justin, if this is true, I will tell you that git respects your umask
but I just can't bring myself to really suggest someone type umask 000
ever. :(

- Andy

--
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 status resets permissions on index file -- Ubuntu 12.04 64b

2013-08-08 Thread Justin Collum
On Thu, Aug 8, 2013 at 3:18 PM, Andrew Ruder  wrote:
> he is neither the user dev or the group dev

I am both. There's only one user on this machine and he is me.

> he is regularly running  chmod -R 777

Yes, true. I have a program that I use to edit some of these files
(not the git files) that likes to screw up permissions. It's a long
story. Anyway, I've been running that chmod regularly since well
before this git issue started. I'll start using 644 for the chmod and
see if that helps.
--
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 status resets permissions on index file -- Ubuntu 12.04 64b

2013-08-08 Thread Kyle J. McKay

On Aug 8, 2013, at 15:18, Andrew Ruder wrote:

On Thu, Aug 08, 2013 at 11:35:35PM +0200, Stefan Beller wrote:

On 08/08/2013 10:27 PM, Justin Collum wrote:

[...]
-rwxrwxrwx   1 dev dev  17K Aug  8 13:12 index
[...]
-rw-rw-r--   1 dev dev  17K Aug  8 13:16 index   # <---


The permissions are set to reading for all and writing for you(r  
user)

and your group. This should be no problem with standard git commands.
Before you had the index file executable, why would you need that?


I'm about 90% sure the issue he's having is that the write bit for
other/world goes away and he is neither the user dev or the group dev
and the reason for all the executable bits is that he is regularly
running

   chmod -R 777 .

Justin, if this is true, I will tell you that git respects your umask
but I just can't bring myself to really suggest someone type umask 000
ever. :(



Justin,

If you really want a repository that's writable by everyone, why not  
just do "git config core.sharedRepository 0666" ?


If you just want them to be group-writable you may be happier with  
"git config core.sharedRepository true" or possibly "git config  
core.sharedRepository all".  The setting is described fully in "git  
help config".

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


[ANNOUNCE] Git v1.8.4-rc2

2013-08-08 Thread Junio C Hamano
A release candidate Git v1.8.4-rc2 is now available for testing
at the usual places.

There are only a handful of small documentation and test updates
since -rc1, except one notable change for Cygwin users.  We no
longer use a custom "fast but cheating" lstat(2) emulation and
instead use the platform one.

The release tarballs are found at:

http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

1477b3def09d3b10a94441881b1aa19eb7f6586f  git-1.8.4.rc2.tar.gz
554ae8d0125c93bfdfa508bbe35a2375c82c545a  git-htmldocs-1.8.4.rc2.tar.gz
92a36c67f0def3544bcbd09bced2779142433ba5  git-manpages-1.8.4.rc2.tar.gz

The following public repositories all have a copy of the v1.8.4-rc2
tag and the master branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.8.4 Release Notes (draft)


Backward compatibility notes (for Git 2.0)
--

When "git push [$there]" does not say what to push, we have used the
traditional "matching" semantics so far (all your branches were sent
to the remote as long as there already are branches of the same name
over there).  In Git 2.0, the default will change to the "simple"
semantics that pushes:

 - only the current branch to the branch with the same name, and only
   when the current branch is set to integrate with that remote
   branch, if you are pushing to the same remote as you fetch from; or

 - only the current branch to the branch with the same name, if you
   are pushing to a remote that is not where you usually fetch from.

Use the user preference configuration variable "push.default" to
change this.  If you are an old-timer who is used to the "matching"
semantics, you can set the variable to "matching" to keep the
traditional behaviour.  If you want to live in the future early, you
can set it to "simple" today without waiting for Git 2.0.

When "git add -u" (and "git add -A") is run inside a subdirectory and
does not specify which paths to add on the command line, it
will operate on the entire tree in Git 2.0 for consistency
with "git commit -a" and other commands.  There will be no
mechanism to make plain "git add -u" behave like "git add -u .".
Current users of "git add -u" (without a pathspec) should start
training their fingers to explicitly say "git add -u ."
before Git 2.0 comes.  A warning is issued when these commands are
run without a pathspec and when you have local changes outside the
current directory, because the behaviour in Git 2.0 will be different
from today's version in such a situation.

In Git 2.0, "git add " will behave as "git add -A ", so
that "git add dir/" will notice paths you removed from the directory
and record the removal.  Versions before Git 2.0, including this
release, will keep ignoring removals, but the users who rely on this
behaviour are encouraged to start using "git add --ignore-removal "
now before 2.0 is released.


Updates since v1.8.3


Foreign interfaces, subsystems and ports.

 * Cygwin port has been updated for more recent Cygwin 1.7.

 * "git rebase -i" now honors --strategy and -X options.

 * Git-gui has been updated to its 0.18.0 version.

 * MediaWiki remote helper (in contrib/) has been updated to use the
   credential helper interface from Git.pm.

 * Update build for Cygwin 1.[57].  Torsten Bögershausen reports that
   this is fine with Cygwin 1.7 ($gmane/225824) so let's try moving it
   ahead.

 * The credential helper to talk to keychain on OS X (in contrib/) has
   been updated to kick in not just when talking http/https but also
   imap(s) and smtp.

 * Remote transport helper has been updated to report errors and
   maintain ref hierarchy used to keep track of its own state better.

 * With "export" remote-helper protocol, (1) a push that tries to
   update a remote ref whose name is different from the pushing side
   does not work yet, and (2) the helper may not know how to do
   --dry-run; these problematic cases are disabled for now.

 * git-remote-hg/bzr (in contrib/) updates.

 * git-remote-mw (in contrib/) hints users to check the certificate,
   when https:// connection failed.

 * git-remote-mw (in contrib/) adds a command to allow previewing the
   contents locally before pushing it out, when working with a
   MediaWiki remote.


UI, Workflows & Features

 * Sample "post-receive-email" hook script got an enhanced replacement
   "multimail" (in contrib/).

 * Also in contrib/ is a new "contacts" script that runs "git blame"
   to find out the people who may be interested in a set of changes.

 * "git clean" command learned an interactive mode.

 * The "--head" option to "git show-ref" was only to add "HEAD" to the
   list of c

Re: git status resets permissions on index file -- Ubuntu 12.04 64b

2013-08-08 Thread Andrew Ruder
On Thu, Aug 08, 2013 at 03:33:32PM -0700, Justin Collum wrote:
> On Thu, Aug 8, 2013 at 3:18 PM, Andrew Ruder  wrote:
> > he is neither the user dev or the group dev
> 
> I am both. There's only one user on this machine and he is me.

If you are running as 'dev', then I'm not sure how the permissions in
the modified case would affect git's ability to work with the file.
Seems like everything should be working fine... what is the error you
get when things stop working?
--
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 status resets permissions on index file -- Ubuntu 12.04 64b

2013-08-08 Thread Justin Collum
doing

 sudo chmod 644 ./.git/index

instead of 777 resulted in the same result a bit later:

$ gs
fatal: index file open failed: Permission denied


On Thu, Aug 8, 2013 at 3:37 PM, Kyle J. McKay  wrote:
> On Aug 8, 2013, at 15:18, Andrew Ruder wrote:
>>
>> On Thu, Aug 08, 2013 at 11:35:35PM +0200, Stefan Beller wrote:
>>>
>>> On 08/08/2013 10:27 PM, Justin Collum wrote:

 [...]
 -rwxrwxrwx   1 dev dev  17K Aug  8 13:12 index
 [...]
 -rw-rw-r--   1 dev dev  17K Aug  8 13:16 index   # <---
>>>
>>>
>>> The permissions are set to reading for all and writing for you(r user)
>>> and your group. This should be no problem with standard git commands.
>>> Before you had the index file executable, why would you need that?
>>
>>
>> I'm about 90% sure the issue he's having is that the write bit for
>> other/world goes away and he is neither the user dev or the group dev
>> and the reason for all the executable bits is that he is regularly
>> running
>>
>>chmod -R 777 .
>>
>> Justin, if this is true, I will tell you that git respects your umask
>> but I just can't bring myself to really suggest someone type umask 000
>> ever. :(
>
>
>
> Justin,
>
> If you really want a repository that's writable by everyone, why not just do
> "git config core.sharedRepository 0666" ?
>
> If you just want them to be group-writable you may be happier with "git
> config core.sharedRepository true" or possibly "git config
> core.sharedRepository all".  The setting is described fully in "git help
> config".
--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Damien Robert
Junio C Hamano  wrote in message
<7v61vg9eht@alter.siamese.dyndns.org>:
> The "tutorial" was written in fairly early days of Git's history, in
> order to primarily help those who want to use the plumbing command
> to script their own Porcelain commands.  As it says at the very
> beginning, the end-user tutorial to use Git's Porcelain is
> gittutorial.txt and the user manual, not this document.

Yes, and even if it's old, it is a really well done tutorial to understand the
internals of git. I read it after gittutorial and gittutorial-2. It's just
that I was surprised to learn about this command, "much more powerful" than
git-log. To me it looked a lot like git log --raw, and I found git log -p
more useful, so I was wondering what I was missing until I read the source
to see that nowadays the two commands were mostly the same.

> The above section primarily explains the use of diff-tree and it was
> appropriate back when git-whatchanged was a script.  The intent of
> the whole document, not just this section, was to tickle the
> curiousity of the users and encourage them to see how the above
> "much more powerful" whatchanged was implemented by going to the
> source.

Well in this case you can say that the intent was successful since it made
me read the source code ;)

--
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: Remove old forgotten command: whatchanged

2013-08-08 Thread Junio C Hamano
>> The above section primarily explains the use of diff-tree and it was
>> appropriate back when git-whatchanged was a script.  The intent of
>> the whole document, not just this section, was to tickle the
>> curiousity of the users and encourage them to see how the above
>> "much more powerful" whatchanged was implemented by going to the
>> source.
>
> Well in this case you can say that the intent was successful since it made
> me read the source code ;)

No, it is a failure. "By going to the source" was meant for
"git-whatchanged.sh" source,
a scripted version of Porcelain, so that users can mimic what is done
in their script.

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


[PATCH] builtin/config.c: compilation fix

2013-08-08 Thread Junio C Hamano
Do not feed a random string as the first parameter to die(); use "%s"
as the format string instead.

Do the same for test-urlmatch-normalization.c while saving a single
pointer variable by turning a "const char *" constant string into
"const char []".

Signed-off-by: Junio C Hamano 
---
 builtin/config.c  | 2 +-
 test-urlmatch-normalization.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index c046f54..ae199e9 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -404,7 +404,7 @@ static int get_urlmatch(const char *var, const char *url)
config.cb = &values;
 
if (!url_normalize(url, &config.url))
-   die(config.url.err);
+   die("%s", config.url.err);
 
config.section = dup_downcase(var);
section_tail = strchr(config.section, '.');
diff --git a/test-urlmatch-normalization.c b/test-urlmatch-normalization.c
index 2603899..78c8b3a 100644
--- a/test-urlmatch-normalization.c
+++ b/test-urlmatch-normalization.c
@@ -3,7 +3,7 @@
 
 int main(int argc, char **argv)
 {
-   const char *usage = "test-urlmatch-normalization [-p | -l]  | 
 ";
+   const char usage[] = "test-urlmatch-normalization [-p | -l]  | 
 ";
char *url1, *url2;
int opt_p = 0, opt_l = 0;
 
-- 
1.8.3.3-1049-g890a991

--
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] builtin/config.c: compilation fix

2013-08-08 Thread Kyle J. McKay

On Aug 8, 2013, at 21:41, Junio C Hamano wrote:

Do not feed a random string as the first parameter to die(); use "%s"
as the format string instead.

Do the same for test-urlmatch-normalization.c while saving a single
pointer variable by turning a "const char *" constant string into
"const char []".

Signed-off-by: Junio C Hamano 
---
builtin/config.c  | 2 +-
test-urlmatch-normalization.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index c046f54..ae199e9 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -404,7 +404,7 @@ static int get_urlmatch(const char *var, const  
char *url)

config.cb = &values;

if (!url_normalize(url, &config.url))
-   die(config.url.err);
+   die("%s", config.url.err);

config.section = dup_downcase(var);
section_tail = strchr(config.section, '.');
diff --git a/test-urlmatch-normalization.c b/test-urlmatch- 
normalization.c

index 2603899..78c8b3a 100644
--- a/test-urlmatch-normalization.c
+++ b/test-urlmatch-normalization.c
@@ -3,7 +3,7 @@

int main(int argc, char **argv)
{
-	const char *usage = "test-urlmatch-normalization [-p | -l]   
|  ";
+	const char usage[] = "test-urlmatch-normalization [-p | -l]   
|  ";

char *url1, *url2;
int opt_p = 0, opt_l = 0;

--
1.8.3.3-1049-g890a991



Looks good to me except that there seems to be a missing part of the  
patch.  Did you also mean to include:


diff --git a/test-urlmatch-normalization.c b/test-urlmatch- 
normalization.c

index 2603899b..39017c20 100644
--- a/test-urlmatch-normalization.c
+++ b/test-urlmatch-normalization.c
@@ -42,7 +42,7 @@ int main(int argc, char **argv)
}

if (opt_p || opt_l)
-   die(usage);
+   die("%s", usage);

url1 = url_normalize(argv[1], NULL);
url2 = url_normalize(argv[2], NULL);


That's not terribly important here since we know the string will never  
contain any '%' characters, but the comment on the patch led me to  
believe that test-urlmatch-normalization would also get the die()  
change.

--
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: Repo with only one file

2013-08-08 Thread Johannes Sixt
Am 8/8/2013 23:11, schrieb Phil Hord:
> On Wed, Aug 7, 2013 at 5:07 PM, shawn wilson  wrote:
>> On Wed, Aug 7, 2013 at 6:43 AM, Johannes Sixt  wrote:
>>> Am 8/7/2013 8:24, schrieb shawn wilson:> ... create a repo for one of
 these scripts and I'd like to keep the commit history.

 Ok, so:
 % find -type f ! -iname "webban.pl" | while read f; do git
 filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f"
 HEAD ; done

 Which basically did it. But, I've got this one commit that seems to be
 orphaned - it doesn't change any files.
>>>
>>> Try this:
>>>
>>>   git filter-branch HEAD -- webban.pl
>>>
>>
>>  % git filter-branch HEAD -- webban.pl
>> Cannot create a new backup.
>> A previous backup already exists in refs/original/
>> Force overwriting the backup with -f
>>  % git filter-branch -f HEAD -- webban.pl
>> Rewrite 1e04b18c256c996312f167be808733bcc755f1e3 (9/9)
>> WARNING: Ref 'refs/heads/master' is unchanged
> 
> I think you can ignore the warning.  Maybe you want to create a new
> branch which only has this file in it now.
> 
>$ git checkout -b webban

I'm not sure. On second thought, my suggested command is not sufficient.
It does remove the empty commits, but it does not remove the other files.
So, Shawn's original filter-branch invocations are still needed.

-- Hannes
--
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


gitk: "Can't parse git log output: { }"

2013-08-08 Thread Peter Krefting

Hi!

In my local clone of git.git, currently with the v1.8.4-rc2 tag 
checked out and built (and installed on the system), starting up gitk 
yields an empty window, with a dialog in front of it:


  error

  Can't parse git log output: { }

  [ OK ]

Has anyone else seen this, and know what might have happened? I do not 
get the behaviour in other repositories (including other clones of 
git.git), only this particular one.


--
\\// Peter - http://www.softwolves.pp.se/
--
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] builtin/config.c: compilation fix

2013-08-08 Thread Junio C Hamano
On Thu, Aug 8, 2013 at 10:38 PM, Kyle J. McKay  wrote:
>> +   const char usage[] = "test-urlmatch-normalization [-p | -l] 

> Looks good to me except that there seems to be a missing part of the patch.
> Did you also mean to include:
>
> diff --git a/test-urlmatch-normalization.c b/test-urlmatch-normalization.c
> index 2603899b..39017c20 100644
> --- a/test-urlmatch-normalization.c
> +++ b/test-urlmatch-normalization.c
> @@ -42,7 +42,7 @@ int main(int argc, char **argv)
> }
>
> if (opt_p || opt_l)
> -   die(usage);
> +   die("%s", usage);
>
> That's not terribly important here since we know the string will never
> contain any '%' characters, but the comment on the patch led me to believe
> that test-urlmatch-normalization would also get the die() change.

It appears that "const char usage[] = ..." is enough to convince Gcc
that usage _is_
a constant and can never contain anything funky. But I agree that the
usage string
could be updated, and I agree that it is prudent to do the same
die("%s", ...) there.
--
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: Repo with only one file

2013-08-08 Thread shawn wilson
On Fri, Aug 9, 2013 at 2:25 AM, Johannes Sixt  wrote:
> Am 8/8/2013 23:11, schrieb Phil Hord:
>> On Wed, Aug 7, 2013 at 5:07 PM, shawn wilson  wrote:
>>> On Wed, Aug 7, 2013 at 6:43 AM, Johannes Sixt  wrote:
 Am 8/7/2013 8:24, schrieb shawn wilson:> ... create a repo for one of
> these scripts and I'd like to keep the commit history.
>
> Ok, so:
> % find -type f ! -iname "webban.pl" | while read f; do git
> filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f"
> HEAD ; done
>

> I'm not sure. On second thought, my suggested command is not sufficient.
> It does remove the empty commits, but it does not remove the other files.
> So, Shawn's original filter-branch invocations are still needed.
>

Yeah, I have tried this and haven't gotten any closer. I can either
remove all of the history or that one commit that has nothing to do
with my file is there. This is also reproducable in a new repo.

Is this a bug with filter-branch or git? This doesn't seem like a
feature (or how things should act).
--
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: Repo with only one file

2013-08-08 Thread Johannes Sixt
Am 8/9/2013 8:33, schrieb shawn wilson:
> On Fri, Aug 9, 2013 at 2:25 AM, Johannes Sixt  wrote:
>> Am 8/8/2013 23:11, schrieb Phil Hord:
>>> On Wed, Aug 7, 2013 at 5:07 PM, shawn wilson  wrote:
 On Wed, Aug 7, 2013 at 6:43 AM, Johannes Sixt  wrote:
> Am 8/7/2013 8:24, schrieb shawn wilson:> ... create a repo for one of
>> these scripts and I'd like to keep the commit history.
>>
>> Ok, so:
>> % find -type f ! -iname "webban.pl" | while read f; do git
>> filter-branch -f --index-filter "git rm --cached --ignore-unmatch $f"
>> HEAD ; done
>>
> 
>> I'm not sure. On second thought, my suggested command is not sufficient.
>> It does remove the empty commits, but it does not remove the other files.
>> So, Shawn's original filter-branch invocations are still needed.
>>
> 
> Yeah, I have tried this and haven't gotten any closer. I can either
> remove all of the history or that one commit that has nothing to do
> with my file is there. This is also reproducable in a new repo.
> 
> Is this a bug with filter-branch or git? This doesn't seem like a
> feature (or how things should act).

Let's check: After running your command above to remove other files, does
the command

   git filter-branch -f HEAD webban.pl

remove the empty commit (if necessary, replace HEAD by the branch name
that you are interested in)?

-- Hannes
--
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