* Re: [PATCH v2 06/14] mingw: use real pid
From: Erik Faye-Lund @ 2010-01-19 19:23 UTC (permalink / raw)
To: Johannes Sixt; +Cc: msysgit, git
In-Reply-To: <201001191919.16070.j6t@kdbg.org>
On Tue, Jan 19, 2010 at 7:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Montag, 18. Januar 2010, Erik Faye-Lund wrote:
>> On Sat, Jan 16, 2010 at 10:12 AM, Erik Faye-Lund wrote:
>> > On Sat, Jan 16, 2010 at 9:03 AM, Johannes Sixt <j6t@kdbg.org> wrote:
>> >> On Freitag, 15. Januar 2010, Erik Faye-Lund wrote:
>> >>> No. If I do, the pid becomes invalid after the process is finished,
>> >>> and waitpid won't work. I couldn't find anywhere were we actually were
>> >>> closing the handle, even after it was finished. So I don't think we
>> >>> leak any more than we already did (for non-daemon purposes).
>> >>
>> >> Previously, this handle was closed by _cwait() (it was the "pid"), so we
>> >> didn't leak it.
>> >
>> > Oh, I see. My planned route with this (before I looked for where the
>> > handle was closed), was to maintain some sort of list of each started
>> > PID and their handle, and lookup in that list instead of using
>> > OpenProcess. I guess that would solve the problem here, but it feels a
>> > bit nasty. Not as nasty as introducing a leak, though.
>>
>> What I had in mind was something along these lines:
>
> Given that that the process ID is the user-visible (and system-wide unique)
> identifier of a process, this looks like the only reasonable way to go. Your
> implementation looks good as well.
>
>> + /* store process handle */
>
> /*
> * The process ID is the human-readable identifier of the process
> * that we want to present in log and error messages. The handle
> * is not useful for this purpose. But we cannot close it, either,
> * because it is not possible to turn a process ID into a process
> * handle after the process terminated.
> * Keep the handle in a list for waitpid.
> */
>
Much better, thanks.
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: Feature idea: Ignore content
From: Ramkumar Ramachandra @ 2010-01-19 19:24 UTC (permalink / raw)
To: Dirk Süsserott; +Cc: Peter Krefting, Git Mailing List
In-Reply-To: <4B56049C.6070705@dirk.my1.cc>
> http://marc.info/?t=125882165900001&r=1&w=2
Fantastic- I had no idea that such a feature even existed. I got it to
work nicely. Thanks!
^ permalink raw reply
* Re: Feature idea: Ignore content
From: Dirk Süsserott @ 2010-01-19 19:14 UTC (permalink / raw)
To: Peter Krefting; +Cc: Ramkumar Ramachandra, Git Mailing List
In-Reply-To: <alpine.DEB.2.00.1001191504020.23165@ds9.cixit.se>
Am 19.01.2010 15:05 schrieb Peter Krefting:
> Ramkumar Ramachandra:
>
>> Instead of moving that out to a separate file and ignoring that file,
>> is it a good idea to add a feature to Git to allow ignoring content
>> instead of whole files?
>
> You should be able to do that by setting up a filter. Please see
> gitattributes(5) for more information (search for "filter").
>
Hi Ramkumar,
please have a look at this thread:
http://marc.info/?t=125882165900001&r=1&w=2
especially Björn's suggestion to use a 'clean filter'. It worked fine
for me with a similar problem: I wanted Git to ignore a certain pattern
in my files.
HTH,
Dirk
^ permalink raw reply
* Re: [PATCH v2] rev-parse --namespace
From: Junio C Hamano @ 2010-01-19 20:06 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: Thomas Rast, git
In-Reply-To: <20100119183736.GA24204@Knoppix>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> ..., if matching power would be extended,
> probably the easiest extension would be full-blown extended regular
> expressions.
As refs behave like a filesystem path and we try to use fnmatch() for
anything that behave like a filesystem path, that would break consistency.
But a patch to add fnmatch() shouldn't be too bad; something like this
(not even compile tested)?
refs.c | 45 +++++++++++++++++++++++++++++++++++++--------
1 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/refs.c b/refs.c
index 5583f4b..0494c75 100644
--- a/refs.c
+++ b/refs.c
@@ -674,19 +674,48 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
return do_for_each_ref("refs/replace/", fn, 13, 0, cb_data);
}
+struct ref_glob_filter {
+ each_ref_fn *user_callback;
+ void *user_cb_data;
+ const char *pattern;
+ int pattern_len;
+};
+
+static int ref_glob_filter_cb(const char *refname,
+ const unsigned char *sha1,
+ int flags, void *cb_data)
+{
+ struct ref_glob_filter *cb = cb_data;
+
+ /*
+ * Reject if it does not produce a prefix match and
+ * it doesn't pass fnmatch().
+ */
+ if (!(cb->pattern_len <= strlen(refname)
+ && !memcmp(cb->pattern, refname, cb->pattern_len)) &&
+ fnmatch(cb->pattern, refname, 0))
+ return 0;
+ return cb->user_callback(refname, sha1, flags, cb->user_cb_data);
+}
+
int for_each_namespace_ref(each_ref_fn fn, const char *ns_name, void *cb_data)
{
- struct strbuf real_prefix = STRBUF_INIT;
+ struct ref_glob_filter filter;
+ struct strbuf pattern = STRBUF_INIT;
int ret;
if (prefixcmp(ns_name, "refs/"))
- strbuf_addstr(&real_prefix, "refs/");
- strbuf_addstr(&real_prefix, ns_name);
- if (real_prefix.buf[real_prefix.len - 1] != '/')
- strbuf_addch(&real_prefix, '/');
-
- ret = for_each_ref_in(real_prefix.buf, fn, cb_data);
- strbuf_release(&real_prefix);
+ strbuf_addstr(&pattern, "refs/");
+ strbuf_addstr(&pattern, ns_name);
+ if (pattern.buf[pattern.len - 1] != '/')
+ strbuf_addch(&pattern, '/');
+ filter.pattern = pattern.buf;
+ filter.pattern_len = pattern.len;
+ filter.user_callback = fn;
+ filter.user_cb_data = cb_data;
+
+ ret = for_each_ref(ref_glob_filter_cb, &filter);
+ strbuf_release(&pattern);
return ret;
}
^ permalink raw reply related
* Re: [PATCH] Fix errors in t6018
From: Junio C Hamano @ 2010-01-19 20:25 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git, Jeff King
In-Reply-To: <1263893748-23327-1-git-send-email-ilari.liusvaara@elisanet.fi>
Thanks, both of you, for being careful.
^ permalink raw reply
* Re: [PATCH] bisect: fix singular/plural grammar nit
From: Junio C Hamano @ 2010-01-19 20:53 UTC (permalink / raw)
To: David Ripton; +Cc: git
In-Reply-To: <20100119151333.GA9660@vidar.dreamhost.com>
David Ripton <dripton@ripton.net> writes:
> Remove the trailing 's' from "revisions" and "steps" when there is
> only one.
Thanks; will apply to 'maint'.
^ permalink raw reply
* Re: [PATCH v2] rev-parse --namespace
From: Ilari Liusvaara @ 2010-01-19 21:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thomas Rast, git
In-Reply-To: <7v3a21amh7.fsf@alter.siamese.dyndns.org>
On Tue, Jan 19, 2010 at 12:06:12PM -0800, Junio C Hamano wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
>
> > ..., if matching power would be extended,
> > probably the easiest extension would be full-blown extended regular
> > expressions.
>
> As refs behave like a filesystem path and we try to use fnmatch() for
> anything that behave like a filesystem path, that would break consistency.
Eh, remind me what commands take refs and shell-glob them? The only
'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
shell globbing...
-Ilari
^ permalink raw reply
* Re: [PATCH v2] rev-parse --namespace
From: Thomas Rast @ 2010-01-19 21:46 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: Junio C Hamano, git
In-Reply-To: <20100119214400.GA24911@Knoppix>
Ilari Liusvaara wrote:
> On Tue, Jan 19, 2010 at 12:06:12PM -0800, Junio C Hamano wrote:
> > As refs behave like a filesystem path and we try to use fnmatch() for
> > anything that behave like a filesystem path, that would break consistency.
>
> Eh, remind me what commands take refs and shell-glob them? The only
> 'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
> shell globbing...
fetchspecs?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [BUG] Git 1.6.6 can't clone tags after filter-branch
From: James Pickens @ 2010-01-19 22:05 UTC (permalink / raw)
To: Git ML, Nicolas Pitre
Hi,
This series of commands:
git init
touch file1
git add file1
git commit -m 1
echo >> file1
touch file2
git add file1 file2
git commit -m 2
git tag -m 'mytag' mytag
git filter-branch --index-filter 'git rm --cached --ignore-unmatch file2' \
--tag-name-filter cat -f -- --all
git clone file://$PWD temp
Results in an error from the clone command:
error: refs/tags/mytag does not point to a valid object!
The clone command exits with zero status, but the new repo doesn't contain
the tag 'mytag'.
Version 1.6.2.5 did not have this problem. It bisects to:
commit 5bdc32d3e50d8335c65e136e6b5234c5dd92a7a9
Author: Nicolas Pitre <nico@fluxnic.net>
Date: Fri Sep 25 23:54:42 2009 -0400
make 'git clone' ask the remote only for objects it cares about
James
^ permalink raw reply
* Re: [PATCH v2] rev-parse --namespace
From: Ilari Liusvaara @ 2010-01-19 22:12 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, git
In-Reply-To: <201001192246.53453.trast@student.ethz.ch>
On Tue, Jan 19, 2010 at 10:46:51PM +0100, Thomas Rast wrote:
> Ilari Liusvaara wrote:
> >
> > Eh, remind me what commands take refs and shell-glob them? The only
> > 'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
> > shell globbing...
>
> fetchspecs?
Ah, found one: 'ls-remote'. Documentation of that doesn't say what format
patterns are in...
Any others?
-Ilari
^ permalink raw reply
* Re: [PATCH v2] rev-parse --namespace
From: Junio C Hamano @ 2010-01-19 22:36 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: Thomas Rast, git
In-Reply-To: <20100119221258.GA25210@Knoppix>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> On Tue, Jan 19, 2010 at 10:46:51PM +0100, Thomas Rast wrote:
>> Ilari Liusvaara wrote:
>> >
>> > Eh, remind me what commands take refs and shell-glob them? The only
>> > 'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
>> > shell globbing...
>>
>> fetchspecs?
>
> Ah, found one: 'ls-remote'. Documentation of that doesn't say what format
> patterns are in...
>
> Any others?
What I gave you was "Please don't use regexp, because matching refs with
fnmatch() is the design guideline we follow". It was not "please follow
the precedence of existing commands". IOW, even if there was no command
that matched refs with globs, it is not an excuse to use regexp. You
didn't even have to find any single example. But here are some others, if
you are interested.
$ git grep fnmatch builtin-{for-each-ref,reflog,name-rev,show-branch,tag}.c
You find the design guideline to make refs behave as paths in a lot more
fundamental places, such as "if you have 'master' branch, you don't have
master/foo branch", and "you cannot use '?' in ref name because it is a
special character in globbing".
^ permalink raw reply
* Re: "warning: Updating the currently checked out branch may cause confusion" on bare repositories
From: Jeff King @ 2010-01-19 22:43 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Adam Megacz, git
In-Reply-To: <20100119185906.GF23466@spearce.org>
On Tue, Jan 19, 2010 at 10:59:06AM -0800, Shawn O. Pearce wrote:
> Adam Megacz <adam@megacz.com> wrote:
> > "Shawn O. Pearce" <spearce@spearce.org> writes:
> > > That should already be the case. Did you create the bare repository
> > > by taking a copy of a non-bare's .git directory? Check your bare
> > > repository's config file and see if core.bare = false, if so set
> > > it to true to signal it really is bare.
> >
> > Hrm, is there a reason why this is explicitly configured rather than
> > detected?
>
> I don't know why we do this. I think its because the guessing
> logic can guess wrong sometimes.
Yup, although I am having trouble finding any actual discussion of when
it can go wrong. There is some mention here:
http://article.gmane.org/gmane.comp.version-control.git/35993
and a little bit here in the 1.5.0 release notes:
http://article.gmane.org/gmane.comp.version-control.git/39612
But the latter talks about the heuristic being "does it end with .git",
which I don't think we actually use anymore. So who knows if the
heuristic failures are even trigger-able anymore. I tried looking at the
setup code for an answer, but my eyes started bleeding from the horrible
spaghetti code and I couldn't make out any of the text.
> Its rare to see it fail. But I think it can fail if you create
> a file called HEAD in the top level of your source code tree.
> (For example.)
I don't think that would do it. When looking for a git dir we check for
a well-formed HEAD, as well as objects and refs directories. The is_bare
check seems to be more about "did we actually find a workdir" these
days.
-Peff
^ permalink raw reply
* Re: [PATCH] Makefile: honor NO_CURL when setting REMOTE_CURL_* variables
From: Junio C Hamano @ 2010-01-19 23:31 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: Johannes Sixt, git
In-Reply-To: <20100119165858.GA24065@Knoppix>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> On Tue, Jan 19, 2010 at 04:39:12PM +0100, Johannes Sixt wrote:
>> Previously, these variables were set before there was a chance to set
>> NO_CURL.
>>
>> This made a difference only during 'make install', because by installing
>> $(REMOTE_CURL_ALIASES), the rule tries to access $(REMOTE_CURL_PRIMARY),
>> which was never installed. On Windows, this fails; on Unix, stale symbolic
>> links are created.
>
> <snip patch>
>
> I didn't even compile-test it, but based on quick look I don't see any
> obivous mistakes. There are no references to REMOTE_CURL_* in section
> moved over, and the variable values should not differ unless section skipped
> sets NO_CURL. So:
>
> Acked-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
>
> -Ilari
Thanks.
^ permalink raw reply
* git-svn: persistent memoization
From: Andrew Myrick @ 2010-01-19 23:37 UTC (permalink / raw)
To: git; +Cc: Sam Vilain, Eric Wong
git-svn uses the Memoize perl module to cache the return values of a few functions. This speeds up svn:mergeinfo processing considerably, but as currently implemented, this memoization table must be reconstructed on every run of git-svn. This isn't a problem on small projects, but it introduces a delay of several minutes when fetching from large repositories with a lot of merge info.
The Memoize module has support for storing its cache persistently (more below, and see http://perldoc.perl.org/Memoize.html for details), and I would love to take advantage of this support in git-svn. Unfortunately, it's not entirely obvious to me how to get this working. Here are the questions I have so far, moving from high level design questions to implementation details:
1) Is there any reason not to store these caches on disk?
2) Are there situations where the caches would need to be invalidated? Perhaps when git-svn rebuilds its metadata?
3) Where should I put these caches? I was thinking something like ".git/svn/caches/<cachefile>.db" would be appropriate.
4) What's the correct way to reference the path to those caches? I tried using "$ENV{GIT_DIR}/svn/caches/has_no_changes.db", but the memoize calls are in a BEGIN block, and it seems that $ENV{GIT_DIR} hadn't been initialized at that point. This has taken me way past my limited knowledge of Perl.
5) What backend should I pick to store the cache? Memoize supports storing to any tied hash that supports TIEHASH, FETCH, STORE, and EXISTS, like DB_File. It also supports storing to SDBM_File and NDBM_File through glue modules (they lack EXISTS support), and Storable, which isn't a tied hash at all, but just a serializer for hashes. I'm leaning towards using Storable since it seems simple and would fit the workload well, but I would appreciate any insight from someone with more domain knowledge here.
Regards,
Andrew
^ permalink raw reply
* Re: bug: git-bundle create foo --stdin -> segfault
From: Johannes Schindelin @ 2010-01-19 23:52 UTC (permalink / raw)
To: Joey Hess; +Cc: git, 554682
In-Reply-To: <20100119002641.GA31434@gnu.kitenet.net>
Hi,
On Mon, 18 Jan 2010, Joey Hess wrote:
> joey@gnu:~/tmp/new>echo master | git bundle create ../my.bundle --stdin
> zsh: segmentation fault git bundle create ../my.bundle --stdin
Current 'next' fails, too.
Some previous Git versions failed with a message like this:
error: unrecognized argument: --stdin'
Other previous Git versions failed at least with a message such as this:
fatal: exec rev-list failed.
error: rev-list died
Something similar happens to me when I run the initial official revision
of git-bundle. The reason is that "git rev-list --boundary
--pretty=oneline --stdin" refuses to run.
The bad versions either segfault, or "refuse to create empty bundles".
And while 8b3dce5 purports to clean things up (even acknowledging that
support code for --stdin was removed from bundle.c!), at that
time git-bundle was obviously not tested/fixed.
Now, I invested a lot of time into the new Git wiki, and into trying to
bisect this (it took many, many more steps than the suggested 13, and
somewhere in between, the number of commits to be tested even increased!).
If you want to fix it, I suggest requiring --stdin to be the only
parameter after the bundle file name, and adding a function using
strbuf_getline() to parse the stdin into a string_list. Once that is
done, you can substitute the argv for the rev-list call with that list
(for that, you need to prepopulate with "rev-list", "--boundary" and
"--pretty=oneline"). You can reuse that list for the call to
setup_revisions().
Alternatively, you can try to implement the rev-list --boundary by hand
(the --pretty=oneline is only needed to get a boundary marker IIRC),
taking care to reset the commit flags that were set in the process. (We
need to know the boundary commits before actually starting to write the
pack, because the bundle file format dictates that the boundary commits
are listed as prerequsites in the bundle header.)
If you want to go that route (which is arguably more elegant anyway), I
suggest having a look at the merge_bases() and get_merge_bases() functions
in commit.c, which do something similar (i.e. a revwalk without using
revision.c's functions -- because you cannot tell what flags they will use
in the future, and they have to be reset after the walk).
Ciao,
Dscho
^ permalink raw reply
* [PATCH] git status: do not require write permission
From: Johannes Schindelin @ 2010-01-20 0:06 UTC (permalink / raw)
To: git; +Cc: gitster
Today, git status played violin on my nerves for the very last time.
There is no good reason, really none, for git status to require
write permissions. If the index is not up-to-date, so be it, I
cannot commit anyway.
But in most cases, the index _is_ up-to-date, and now I can tell
my fellow former users that their repository XYZ.git does not have any
uncommitted changes, so can they please delete it to free up some disk
space, thank you very much.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin-commit.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 55676fd..9eccc51 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -325,11 +325,13 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
* We still need to refresh the index here.
*/
if (!pathspec || !*pathspec) {
- fd = hold_locked_index(&index_lock, 1);
- refresh_cache_or_die(refresh_flags);
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(&index_lock))
- die("unable to write new_index file");
+ fd = hold_locked_index(&index_lock, 0);
+ if (fd >= 0) {
+ refresh_cache_or_die(refresh_flags);
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(&index_lock))
+ die("unable to write new_index file");
+ }
commit_style = COMMIT_AS_IS;
return get_index_file();
}
--
1.6.4.297.gcb4cc
^ permalink raw reply related
* Re: [PATCH] git status: do not require write permission
From: Jeff King @ 2010-01-20 0:28 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.1001200105230.3164@intel-tinevez-2-302>
On Wed, Jan 20, 2010 at 01:06:17AM +0100, Johannes Schindelin wrote:
> Today, git status played violin on my nerves for the very last time.
> There is no good reason, really none, for git status to require
> write permissions. If the index is not up-to-date, so be it, I
> cannot commit anyway.
I agree it is annoying, but this patch does not apply (and is no longer
needed) on master since the status-is-no-longer-commit-dry-run series
has been merged.
I don't know if it is worth putting your fix onto maint.
-Peff
^ permalink raw reply
* Re: [PATCH] git status: do not require write permission
From: Johannes Schindelin @ 2010-01-20 0:35 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster
In-Reply-To: <20100120002836.GA16824@coredump.intra.peff.net>
Hi,
On Tue, 19 Jan 2010, Jeff King wrote:
> On Wed, Jan 20, 2010 at 01:06:17AM +0100, Johannes Schindelin wrote:
>
> > Today, git status played violin on my nerves for the very last time.
> > There is no good reason, really none, for git status to require
> > write permissions. If the index is not up-to-date, so be it, I
> > cannot commit anyway.
>
> I agree it is annoying, but this patch does not apply (and is no longer
> needed) on master since the status-is-no-longer-commit-dry-run series
> has been merged.
I was working on top of 'next'. And only after I fetched afresh from
git://repo.or.cz/git.git/.
So for me and my nerves, the patch series came way too late.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git status: do not require write permission
From: Johannes Schindelin @ 2010-01-20 0:38 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.1001200134380.3164@intel-tinevez-2-302>
Hi,
On Wed, 20 Jan 2010, Johannes Schindelin wrote:
> On Tue, 19 Jan 2010, Jeff King wrote:
>
> > On Wed, Jan 20, 2010 at 01:06:17AM +0100, Johannes Schindelin wrote:
> >
> > > Today, git status played violin on my nerves for the very last time.
> > > There is no good reason, really none, for git status to require
> > > write permissions. If the index is not up-to-date, so be it, I
> > > cannot commit anyway.
> >
> > I agree it is annoying, but this patch does not apply (and is no longer
> > needed) on master since the status-is-no-longer-commit-dry-run series
> > has been merged.
>
> I was working on top of 'next'. And only after I fetched afresh from
> git://repo.or.cz/git.git/.
To be precise, the commit I based mine on is
2cd0ddca4b6f2ff24fcd7b24030c97ec20ead85c which was done on Mon Jan 18
18:34:16 2010 -0800 according to Git.
Ciao,
Dscho "who will think 3 times before making a patch next time"
^ permalink raw reply
* Re: [PATCH] git status: do not require write permission
From: Junio C Hamano @ 2010-01-20 0:39 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <20100120002836.GA16824@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 20, 2010 at 01:06:17AM +0100, Johannes Schindelin wrote:
>
>> Today, git status played violin on my nerves for the very last time.
>> There is no good reason, really none, for git status to require
>> write permissions. If the index is not up-to-date, so be it, I
>> cannot commit anyway.
>
> I agree it is annoying, but this patch does not apply (and is no longer
> needed) on master since the status-is-no-longer-commit-dry-run series
> has been merged.
>
> I don't know if it is worth putting your fix onto maint.
I think the patch itself makes sense, even though the log message seems to
be with more noise and frustrationthan with useful information.
And "for the very last time" is probably a good characterization, as
status will no longer be "commit --dry-run" in the coming release ;-)
^ permalink raw reply
* Re: git-status segmentation fault in master / OS X
From: Jeff King @ 2010-01-20 0:41 UTC (permalink / raw)
To: Jonathan del Strother; +Cc: Git Mailing List
In-Reply-To: <57518fd11001190959n355a0f22p7caa7251b705efaf@mail.gmail.com>
On Tue, Jan 19, 2010 at 05:59:51PM +0000, Jonathan del Strother wrote:
> I've been running into a segmentation fault on running git status in
> my repository while there are staged changes.
I can't reproduce it here. Is there anything else interesting about the
repo you can tell us besides that it has staged changes?
> Bisecting suggests it was introduced in:
>
> commit 73d66323ac78c750ba42fef23b1cb8fd2110e023
> Merge: 054d2fa 8740773
> Author: Junio C Hamano <gitster@pobox.com>
> Date: Wed Jan 13 11:58:34 2010 -0800
>
> Merge branch 'nd/sparse'
>
>
> The last commit from nd/sparse (8740773) is fine, so I guess it's a
> bad merge...?
Could be a bad interaction between commits on nd/sparse and whatever was
done since it had branched. You can try rebasing nd/sparse and bisecting
a linearised version, like this:
bad_merge=73d66323
# pretend we are on nd/sparse
git checkout -b test $bad_merge^2
# rebase onto what we merged onto
git rebase $bad_merge^1
# now bisect. what we have now is presumably
# bad (though you should probably double check)
# and from the previous bisect we know that
# everything pre-merge is good
git bisect start
git bisect good $bad_merge^1
git bisect bad
-Peff
^ permalink raw reply
* Re: git-status segmentation fault in master / OS X
From: Junio C Hamano @ 2010-01-20 0:56 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan del Strother, Git Mailing List
In-Reply-To: <20100120004146.GB16824@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Could be a bad interaction between commits on nd/sparse and whatever was
> done since it had branched. You can try rebasing nd/sparse and bisecting
> a linearised version, like this:
>
> bad_merge=73d66323
> # pretend we are on nd/sparse
> git checkout -b test $bad_merge^2
> # rebase onto what we merged onto
> git rebase $bad_merge^1
That is a very good suggestion.
You will get hit by a few conflicts during the rebase, but I managed to
arrive at the same tree as $bad_merge after running the rebase procedure
above. Just for Jonathan's convenience, the result is at:
git://repo.or.cz/alt-git.git junk-linearized-nd-sparse-for-bisection
I'll remove this after a few days.
> # now bisect. what we have now is presumably
> # bad (though you should probably double check)
> # and from the previous bisect we know that
> # everything pre-merge is good
> git bisect start
> git bisect good $bad_merge^1
> git bisect bad
It would be interesting to hear the result of the test in the particular
repository Jonathan is seeing the problem with. The issue didn't
reproduce for me, either but I only tried "having a staged change" case
without any more detailed set-up.
^ permalink raw reply
* Re: [PATCH] git status: do not require write permission
From: Junio C Hamano @ 2010-01-20 1:09 UTC (permalink / raw)
To: Johannes Schindelin, Jeff King; +Cc: git
In-Reply-To: <7vhbqh7gpa.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> On Wed, Jan 20, 2010 at 01:06:17AM +0100, Johannes Schindelin wrote:
>>
>>> Today, git status played violin on my nerves for the very last time.
>>> There is no good reason, really none, for git status to require
>>> write permissions. If the index is not up-to-date, so be it, I
>>> cannot commit anyway.
>>
>> I agree it is annoying, but this patch does not apply (and is no longer
>> needed) on master since the status-is-no-longer-commit-dry-run series
>> has been merged.
>>
>> I don't know if it is worth putting your fix onto maint.
>
> I think the patch itself makes sense,...
Actually, I think it was somewhat a selfish patch and forgot that the
codepath is shared with a mode of operation where we should guarantee
that the index is updated, namely "git commit".
I think this would be a good addition to 'maint'. I am not sure if it is
worth forward-porting to "commit --dry-run", though. On one hand, it
might show what would happen if you ran "commit" better (i.e. you will be
told that you cannot write into it), but it is debatable if that is the
reason why people may want to run "commit --dry-run".
-- >8 --
Subject: status: don't require the repository to be writable
We need to update the index before hooks run when actually making a
commit, but we shouldn't have to write the index when running "status".
If we can, then we have already spent cycles to refresh the index and
it is a waste not to write it out, but it is not a disaster if we cannot
write it out. The main reason the user is running "git status" is to get
the "status", and refreshing the index is a mere side effect that we can
do without.
Discovery and initial attempted fix by Dscho.
---
builtin-commit.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 33aa593..83b7c35 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -278,11 +278,13 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
* We still need to refresh the index here.
*/
if (!pathspec || !*pathspec) {
- fd = hold_locked_index(&index_lock, 1);
+ fd = hold_locked_index(&index_lock, !is_status);
refresh_cache(refresh_flags);
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(&index_lock))
- die("unable to write new_index file");
+ if (0 <= fd) {
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(&index_lock))
+ die("unable to write new_index file");
+ }
commit_style = COMMIT_AS_IS;
return get_index_file();
}
^ permalink raw reply related
* git locate
From: John Tapsell @ 2010-01-20 1:17 UTC (permalink / raw)
To: Git List
Hey all,
Could we add a: git locate <filename> or git find <filename>
that is just an alias to "git ls-files | grep filename" ?
Reasons:
1) git ls-files does not autocomplete, since it's an low-level
plumbing command
2) It's nice having shell equivalents - e.g. git grep etc
John
^ permalink raw reply
* Re: [PATCH] git status: do not require write permission
From: Johannes Schindelin @ 2010-01-20 1:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vhbqh7gpa.fsf@alter.siamese.dyndns.org>
Hi,
On Tue, 19 Jan 2010, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Wed, Jan 20, 2010 at 01:06:17AM +0100, Johannes Schindelin wrote:
> >
> >> Today, git status played violin on my nerves for the very last time.
> >> There is no good reason, really none, for git status to require
> >> write permissions. If the index is not up-to-date, so be it, I
> >> cannot commit anyway.
> >
> > I agree it is annoying, but this patch does not apply (and is no longer
> > needed) on master since the status-is-no-longer-commit-dry-run series
> > has been merged.
> >
> > I don't know if it is worth putting your fix onto maint.
>
> I think the patch itself makes sense, even though the log message seems to
> be with more noise and frustrationthan with useful information.
If you think so.
> And "for the very last time" is probably a good characterization, as
> status will no longer be "commit --dry-run" in the coming release ;-)
This comment was because I was fully prepared to do the same as with so
many patches: keep them in my personal repository until time finally tells
that that patch should go into 'next'.
And as you realized, I managed to forget a commit --amend, which would
have replaced 1 with !is_status.
BTW you can tell me directly that you do not want any patches from me
anymore, rather than dancing around the issue. I kind of figured that
already anyway.
Ciao,
Dscho
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox