* Re: [PATCH 2/3] mergetool-lib: specialize xxdiff options when in diff mode
From: Markus Heidelberg @ 2009-04-02 21:20 UTC (permalink / raw)
To: David Aguilar; +Cc: gitster, git
In-Reply-To: <1238668246-762-2-git-send-email-davvid@gmail.com>
David Aguilar, 02.04.2009:
> We don't need '-merged-file', 'save as', and the merge-status exit
> code when in diff mode
I guess the same as for xxdiff and kdiff3 also applies to opendiff,
ecmerge and emerge. But since I don't have them installed, I'm not
entirely sure not to break something.
On the other hand it should be save, since they were just copied from
git-mergetool without adjustment for diff use.
Markus
^ permalink raw reply
* Re: [PATCH] git remote update: New option --prune (-p)
From: Junio C Hamano @ 2009-04-02 20:52 UTC (permalink / raw)
To: Finn Arne Gangstad; +Cc: Jeff King, demerphq, git
In-Reply-To: <20090402201803.GA5397@pvv.org>
Finn Arne Gangstad <finnag@pvv.org> writes:
> On Thu, Apr 02, 2009 at 11:06:56AM -0700, Junio C Hamano wrote:
>> [...]
>>
>> I gave the patch an only cursory look, so I wouldn't comment on the
>> implementation; two things I would look at in the code would be if it
>> makes two connections to the remote to learn the same information (which
>> would be bad)
>
> How bad?
I'd say only "could be improved later" bad.
^ permalink raw reply
* Re: [PATCH] Fix 'git checkout <submodule>' to update the index
From: Johannes Schindelin @ 2009-04-02 20:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vprfuesra.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 2 Apr 2009, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > While 'git checkout <submodule>' should not update the submodule's
> > working directory, it should update the index. This is in line with
> > how submodules are handled in the rest of Git.
> >
> > While at it, test 'git reset [<commit>] <submodule>', too.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> >
> > I have no idea who thought that it would be a good idea to leave
> > the submodules alone, but I think it is highly counterintuitive
> > that
> >
> > git checkout <submodule>
> >
> > does not touch the index.
>
> I do not see anything that treats submodule specially in the scripted
> version and this check for S_ISGITLINK() has been there since the very
> beginning of C rewrite 782c2d6 (Build in checkout, 2008-02-07), so it is
> a regression fix that can go to maint (all the way down to 1.5.5 if you
> want).
I would appreciate that.
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH] git remote update: New option --prune (-p)
From: Finn Arne Gangstad @ 2009-04-02 20:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, demerphq, git
In-Reply-To: <7vab6zexq7.fsf@gitster.siamese.dyndns.org>
On Thu, Apr 02, 2009 at 11:06:56AM -0700, Junio C Hamano wrote:
> [...]
>
> I gave the patch an only cursory look, so I wouldn't comment on the
> implementation; two things I would look at in the code would be if it
> makes two connections to the remote to learn the same information (which
> would be bad)
How bad? git remote update execs "git fetch <remote>" to do the
fetching part, and after that the information is lost of course. It
might be possible to do a --prune option to fetch instead, and just
use that directly.
> and if it skips the pruning stage if the update stage failed
> (which would probably be a sane precaution).
Yes, this should be fixed.
- Finn Arne
^ permalink raw reply
* Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib
From: Markus Heidelberg @ 2009-04-02 20:16 UTC (permalink / raw)
To: Charles Bailey; +Cc: David Aguilar, gitster, git
In-Reply-To: <200904022213.15173.markus.heidelberg@web.de>
Markus Heidelberg, 02.04.2009:
> Huh, it seems as if some editors for difftool cannot work correctly
> currently, because this function is used, but defined in
> git-mergetool.sh.
Yes, it works...
# Overridden in git-mergetool
check_unchanged () {
true
}
But that's not really nice, is it? Maybe we'd be better with a
if merge_mode; then
inside this function and only define it in git-mergetool--lib.sh.
Markus
^ permalink raw reply
* Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib
From: Markus Heidelberg @ 2009-04-02 20:13 UTC (permalink / raw)
To: Charles Bailey; +Cc: David Aguilar, gitster, git
In-Reply-To: <20090402200235.GB13817@hashpling.org>
Charles Bailey, 02.04.2009:
> On Thu, Apr 02, 2009 at 12:54:22AM +0200, Markus Heidelberg wrote:
> > David Aguilar, 01.04.2009:
> > > + present=false
> > > + base_present &&
> > > + present=true
> > > +
> > > + run_mergetool "$merge_tool" "$present"
> > > + status=$?
> >
> > This last line has to be deleted, because the variable 'status' set in
> > run_mergetool would be overwritten then. In this case the merge will
> > succeed even if it didn't and the file will be staged.
>
> I think that it would be better if $status stayed as local as
> possible. If run_mergetool returned the value of $status as its exit
> code then the function will properly return whether the merge
> succeeded or not.
>
> This way run_mergetool's clients can just use its exit code, without
> having to know about a magic $status global variable.
Sure. Next time I have to think about the right solution before
spreading workarounds.
And shouldn't also check_unchanged() move from git-mergetool.sh to
git-mergetool--lib.sh, since it is only used there?
Huh, it seems as if some editors for difftool cannot work correctly
currently, because this function is used, but defined in
git-mergetool.sh.
Markus
^ permalink raw reply
* Re: [PATCH] {diff,merge}tool: rename helpers to remove them from tab-completion
From: Markus Heidelberg @ 2009-04-02 20:04 UTC (permalink / raw)
To: David Aguilar; +Cc: gitster, git
In-Reply-To: <1238672296-10265-1-git-send-email-davvid@gmail.com>
David Aguilar, 02.04.2009:
> diff --git a/Makefile b/Makefile
[..]
> SCRIPT_SH += git-merge-resolve.sh
> SCRIPT_SH += git-mergetool.sh
> -SCRIPT_SH += git-mergetool-lib.sh
> +SCRIPT_SH += git-mergetool--lib.sh
> SCRIPT_SH += git-parse-remote.sh
> SCRIPT_SH += git-pull.sh
> SCRIPT_SH += git-quiltimport.sh
This conflicts with commit 496c021 (Add a script to edit/inspect notes)
in 'pu', which seems to be just merged today. If you build your patches
on top of 'da/difftool' (99511d8) instead of 'pu', then these conflicts
won't arise and avoid hassle.
In fact I already applied all your patches on top 'da/difftool' and not
'pu' all the time, until now.
Thanks for your work on this
Markus
^ permalink raw reply
* Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib
From: Charles Bailey @ 2009-04-02 20:02 UTC (permalink / raw)
To: Markus Heidelberg; +Cc: David Aguilar, gitster, git
In-Reply-To: <200904020054.23147.markus.heidelberg@web.de>
On Thu, Apr 02, 2009 at 12:54:22AM +0200, Markus Heidelberg wrote:
> David Aguilar, 01.04.2009:
> > + present=false
> > + base_present &&
> > + present=true
> > +
> > + run_mergetool "$merge_tool" "$present"
> > + status=$?
>
> This last line has to be deleted, because the variable 'status' set in
> run_mergetool would be overwritten then. In this case the merge will
> succeed even if it didn't and the file will be staged.
I think that it would be better if $status stayed as local as
possible. If run_mergetool returned the value of $status as its exit
code then the function will properly return whether the merge
succeeded or not.
This way run_mergetool's clients can just use its exit code, without
having to know about a magic $status global variable.
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* Re: git-{diff,merge} refactor round 2
From: Charles Bailey @ 2009-04-02 19:59 UTC (permalink / raw)
To: David Aguilar; +Cc: gitster, git
In-Reply-To: <1238590514-41893-1-git-send-email-davvid@gmail.com>
On Wed, Apr 01, 2009 at 05:55:04AM -0700, David Aguilar wrote:
> Here's the 2nd round of refactoring.
Again, I reply in summary form...
[02/10] - I ack this.
[03-07 / 10] - fine. I'm not sure, but should they be one commit?
They're really about consolidating blocks from one place to another
and the change might be more 'obvious' as a single commit.
[08/10] Now that we've isolated the run_mergetool, can we get it to
return the value of '$status' (either the result of the tool or of
check_unchanged) as its exit code so that we know whether the merge
succeeded?
[10/10] Then this chunk:
+ run_mergetool "$merge_tool" "$present"
+ status=$?
if test "$status" -ne 0; then
can validly become:
if ! run_mergtool "$merge_tool" "$present"; then
which seems clearer to me.
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* Re: [PATCH] Fix 'git checkout <submodule>' to update the index
From: Junio C Hamano @ 2009-04-02 19:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <c267451c12c4ead4c222eebe6df460b46edd59c1.1238678601u.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> While 'git checkout <submodule>' should not update the submodule's
> working directory, it should update the index. This is in line with
> how submodules are handled in the rest of Git.
>
> While at it, test 'git reset [<commit>] <submodule>', too.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> I have no idea who thought that it would be a good idea to leave
> the submodules alone, but I think it is highly counterintuitive
> that
>
> git checkout <submodule>
>
> does not touch the index.
I do not see anything that treats submodule specially in the scripted
version and this check for S_ISGITLINK() has been there since the very
beginning of C rewrite 782c2d6 (Build in checkout, 2008-02-07), so it is a
regression fix that can go to maint (all the way down to 1.5.5 if you
want).
Thanks.
^ permalink raw reply
* Whitespace options
From: Gerd Knops @ 2009-04-02 19:18 UTC (permalink / raw)
To: git
Hi all,
I think there should be a whitespace option that ignores lines that
consists completely of whitespace vs lines with actual trailing
whitespace preceded by non-whitespace.
The reason is that many editors (especially those supporting indent-
based code folding) keep 'empty' lines at the current indentation
level. git (with default settings) flags all those lines.
I know I could just remove 'trailing-space' from 'core.whitespace',
but I would actually still like be notified about the case where a
line that is not whitespace only ends with trailing whitespace.
Thanks
Gerd
^ permalink raw reply
* Re: [PATCH] git remote update: New option --prune (-p)
From: demerphq @ 2009-04-02 19:05 UTC (permalink / raw)
To: Jeff King; +Cc: Finn Arne Gangstad, git, gitster
In-Reply-To: <20090402163213.GA28261@coredump.intra.peff.net>
2009/4/2 Jeff King <peff@peff.net>:
> On Thu, Apr 02, 2009 at 06:07:33PM +0200, demerphq wrote:
>
>> Er, personally i find that documentation pretty cryptic. And when i
>> check git config for group, i see this:
>>
>> remotes.<group>
>> The list of remotes which are fetched by "git remote update
>> <group>". See git-remote(1).
>
> Yeah, this should probably say "separated by space" or whatever (I
> actually don't even know). I would assume it contains the remote name; I
> can't imagine what other thing it would include. But it wouldn't hurt to
> make that more explicit.
>
> I'm sure a documentation patch would be welcome.
If/when i feel that i understand the subject sufficiently Ill make an
attempt. :-)
>> remote.<name>.skipDefaultUpdate
>> If true, this remote will be skipped by default when updating using
>> the update subcommand of git-remote(1).
>>
>> Neither of which really explain groups, how to define them properly,
>> (the list is separated by what? and includes the remote name?) or
>> whether there are implicit groups. I mean, it seems logical that if
>> you can have user defined groups that there are some built in ones
>> too, like "all" and "none" or perhaps groups defined by transport
>> "http" or "git" for instance.
>
> I think what is confusing is that there is exactly one implicit group,
> and it is "the default group", which contains every remote that doesn't
> have skipDefaultUpdate set. You refer to the "default group" by not
> mentioning any group.
Yes well an implicit group "default" would be nice.
>
> So no, there aren't other implicit groups (AFAIK).
>
> You can propose implicit groups, but I think they would have to have a
> compelling use case over simply creating them manually. To avoid
> conflict with groups people have already defined, they would only be
> used if no remotes.$whatever config existed.
Or perhpas simply use syntax not likely or expected to be used in the
name of a group. Like colon. Or something like that.
>
> I think having "git remote update foo" fall back to a group containing
> only the remote "foo" when "remotes.foo" does not exist makes sense.
> I'm not sure that "none", "http", or "git" is all that useful in
> practice (the only thing I can think of for the latter two is that you
> might use "git" versus "http" depending on restrictive firewall
> settings).
Well i was think of situations where somebody has coded something that
just must have a group. Thus 'none' would be essentially a no-op in
this case. I know you can argue "well dont do that", but people tend
to do silly things whatever they are told, and explicit arguments make
for less special cases in wrapper scripts and the like...
>
> You could give the unnamed "default group" a name (like "all"), but then
> you risk conflict with existing "remotes.all".
Id much prefer the default group to be called default. Not "all".
Ideally "all" would really be "all". :-)
> And in this case, it is
> hard to remain backwards compatible: "git remote update" will do
> something different now in the case that the user has configured
> remotes.all.
Well, id have a thought a better approach is to use a character that
cannot or is extremely unlikely to be used in an existing group
definition and cannot be used in a remote definition. Like maybe
":all" or something.
Or maybe: `git config remote.implicitgroups true` could be used to enable it?
cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
* [EGIT PATCH 4/5] Update Quickdiff tracing statements
From: Robin Rosenberg @ 2009-04-02 18:46 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1238697991-12990-4-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../egit/ui/internal/decorators/GitDocument.java | 9 ++++++---
.../internal/decorators/GitQuickDiffProvider.java | 2 ++
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
index 8c82a55..347e6fc 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
@@ -34,6 +34,7 @@
static Map<GitDocument,Repository> doc2repo = new WeakHashMap<GitDocument, Repository>();
static GitDocument create(final IResource resource) throws IOException {
+ Activator.trace("(GitDocument) create: " + resource);
GitDocument ret = null;
if (RepositoryProvider.getProvider(resource.getProject()) instanceof GitProvider) {
ret = new GitDocument(resource);
@@ -51,6 +52,7 @@ private GitDocument(IResource resource) {
}
void populate() throws IOException {
+ Activator.trace("(GitDocument) populate: " + resource);
set("");
final IProject project = resource.getProject();
RepositoryMapping mapping = RepositoryMapping.getMapping(project);
@@ -70,7 +72,7 @@ void populate() throws IOException {
}
TreeEntry blobEnry = baselineTree.findBlobMember(gitPath);
if (blobEnry != null) {
- Activator.trace("(GitQuickDiffProvider) compareTo: " + baseline);
+ Activator.trace("(GitDocument) compareTo: " + baseline);
ObjectLoader loader = repository.openBlob(blobEnry.getId());
byte[] bytes = loader.getBytes();
String charset;
@@ -91,13 +93,14 @@ void populate() throws IOException {
// to the content. We don't do that here.
String s = new String(bytes, charset);
set(s);
- Activator.trace("(GitQuickDiffProvider) has reference doc, size=" + s.length() + " bytes");
+ Activator.trace("(GitDocument) has reference doc, size=" + s.length() + " bytes");
} else {
- Activator.trace("(GitQuickDiffProvider) no revision.");
+ Activator.trace("(GitDocument) no revision.");
}
}
void dispose() {
+ Activator.trace("(GitDocument) dispose: " + resource);
doc2repo.remove(this);
Repository repository = getRepository();
if (repository != null)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
index 6c71f3c..91efb56 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
@@ -42,6 +42,7 @@
* Create the GitQuickDiffProvider instance
*/
public GitQuickDiffProvider() {
+ Activator.trace("(GitQuickDiffProvider) constructor");
// Empty
}
@@ -77,6 +78,7 @@ public boolean isEnabled() {
}
public void setActiveEditor(ITextEditor editor) {
+ Activator.trace("(GitQuickDiffProvider) setActiveEditor: " + editor.getTitle());
IEditorInput editorInput = editor.getEditorInput();
resource = ResourceUtil.getResource(editorInput);
}
--
1.6.2.1.345.g89fb
^ permalink raw reply related
* [EGIT PATCH 5/5] Cache resolved ids in quickdiff document for faster update
From: Robin Rosenberg @ 2009-04-02 18:46 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1238697991-12990-5-git-send-email-robin.rosenberg@dewire.com>
We do this by caching the commit, tree and blob ids and can then
very quickly decide whether a change in baseline actually results in a
changed version of the reference blob used for quickdiff.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../egit/ui/internal/decorators/GitDocument.java | 70 +++++++++++++++++---
1 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
index 347e6fc..59a738c 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
@@ -20,8 +20,11 @@
import org.spearce.egit.core.GitProvider;
import org.spearce.egit.core.project.RepositoryMapping;
import org.spearce.egit.ui.Activator;
+import org.spearce.jgit.lib.AnyObjectId;
+import org.spearce.jgit.lib.Commit;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.IndexChangedEvent;
+import org.spearce.jgit.lib.ObjectId;
import org.spearce.jgit.lib.ObjectLoader;
import org.spearce.jgit.lib.RefsChangedEvent;
import org.spearce.jgit.lib.Repository;
@@ -31,6 +34,11 @@
class GitDocument extends Document implements RepositoryListener {
private final IResource resource;
+
+ private AnyObjectId lastCommit;
+ private AnyObjectId lastTree;
+ private AnyObjectId lastBlob;
+
static Map<GitDocument,Repository> doc2repo = new WeakHashMap<GitDocument, Repository>();
static GitDocument create(final IResource resource) throws IOException {
@@ -51,29 +59,68 @@ private GitDocument(IResource resource) {
GitDocument.doc2repo.put(this, getRepository());
}
+ private void setResolved(final AnyObjectId commit, final AnyObjectId tree, final AnyObjectId blob, final String value) {
+ lastCommit = commit != null ? commit.copy() : null;
+ lastTree = tree != null ? tree.copy() : null;
+ lastBlob = blob != null ? blob.copy() : null;
+ set(value);
+ if (blob != null)
+ Activator.trace("(GitDocument) resolved " + resource + " to " + lastBlob + " in " + lastCommit + "/" + lastTree);
+ else
+ Activator.trace("(GitDocument) unresolved " + resource);
+ }
+
void populate() throws IOException {
Activator.trace("(GitDocument) populate: " + resource);
- set("");
final IProject project = resource.getProject();
RepositoryMapping mapping = RepositoryMapping.getMapping(project);
- if (mapping == null)
+ if (mapping == null) {
+ setResolved(null, null, null, "");
return;
+ }
final String gitPath = mapping.getRepoRelativePath(resource);
- final Repository repository = getRepository();
+ final Repository repository = mapping.getRepository();
String baseline = GitQuickDiffProvider.baseline.get(repository);
if (baseline == null)
baseline = Constants.HEAD;
- Tree baselineTree = repository.mapTree(baseline);
- if (baselineTree == null) {
+ ObjectId commitId = repository.resolve(baseline);
+ if (commitId != null) {
+ if (commitId.equals(lastCommit)) {
+ Activator.trace("(GitDocument) already resolved");
+ return;
+ }
+ } else {
Activator.logError("Could not resolve quickdiff baseline "
+ baseline + " corresponding to " + resource + " in "
+ repository, new Throwable());
+ setResolved(null, null, null, "");
+ return;
+ }
+ Commit baselineCommit = repository.mapCommit(commitId);
+ if (baselineCommit == null) {
+ Activator.logError("Could not load commit " + commitId + " for "
+ + baseline + " corresponding to " + resource + " in "
+ + repository, new Throwable());
+ setResolved(null, null, null, "");
+ return;
+ }
+ ObjectId treeId = baselineCommit.getTreeId();
+ if (treeId.equals(lastTree)) {
+ Activator.trace("(GitDocument) already resolved");
+ return;
+ }
+ Tree baselineTree = baselineCommit.getTree();
+ if (baselineTree == null) {
+ Activator.logError("Could not load tree " + treeId + " for "
+ + baseline + " corresponding to " + resource + " in "
+ + repository, new Throwable());
+ setResolved(null, null, null, "");
return;
}
- TreeEntry blobEnry = baselineTree.findBlobMember(gitPath);
- if (blobEnry != null) {
+ TreeEntry blobEntry = baselineTree.findBlobMember(gitPath);
+ if (blobEntry != null && !blobEntry.getId().equals(lastBlob)) {
Activator.trace("(GitDocument) compareTo: " + baseline);
- ObjectLoader loader = repository.openBlob(blobEnry.getId());
+ ObjectLoader loader = repository.openBlob(blobEntry.getId());
byte[] bytes = loader.getBytes();
String charset;
// Get the encoding for the current version. As a matter of
@@ -92,10 +139,13 @@ void populate() throws IOException {
// Finally we could consider validating the content with respect
// to the content. We don't do that here.
String s = new String(bytes, charset);
- set(s);
+ setResolved(commitId, baselineTree.getId(), blobEntry.getId(), s);
Activator.trace("(GitDocument) has reference doc, size=" + s.length() + " bytes");
} else {
- Activator.trace("(GitDocument) no revision.");
+ if (blobEntry == null)
+ setResolved(null, null, null, "");
+ else
+ Activator.trace("(GitDocument) already resolved");
}
}
--
1.6.2.1.345.g89fb
^ permalink raw reply related
* [EGIT PATCH 2/5] quickdiff - Don't add GitDocument as repository listener more than once
From: Robin Rosenberg @ 2009-04-02 18:46 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1238697991-12990-2-git-send-email-robin.rosenberg@dewire.com>
Doing so was very costly and led to sessions being locked
for a long time while refreshing the reference document used
by Eclipse's quickdiff feature.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../egit/ui/internal/decorators/GitDocument.java | 4 +++-
.../src/org/spearce/jgit/lib/Repository.java | 2 ++
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
index a9c0c7d..69e9295 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
@@ -35,6 +35,9 @@ static GitDocument create(final IResource resource) throws IOException {
if (RepositoryProvider.getProvider(resource.getProject()) instanceof GitProvider) {
ret = new GitDocument(resource);
ret.populate();
+ final Repository repository = ret.getRepository();
+ if (repository != null)
+ repository.addRepositoryChangedListener(ret);
}
return ret;
}
@@ -52,7 +55,6 @@ void populate() throws IOException {
return;
final String gitPath = mapping.getRepoRelativePath(resource);
final Repository repository = getRepository();
- repository.addRepositoryChangedListener(this);
String baseline = GitQuickDiffProvider.baseline.get(repository);
if (baseline == null)
baseline = Constants.HEAD;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index cfd92b8..0f396ed 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -1132,6 +1132,7 @@ public File getWorkDir() {
* @param l
*/
public void addRepositoryChangedListener(final RepositoryListener l) {
+ assert !listeners.contains(l);
listeners.add(l);
}
@@ -1150,6 +1151,7 @@ public void removeRepositoryChangedListener(final RepositoryListener l) {
* @param l
*/
public static void addAnyRepositoryChangedListener(final RepositoryListener l) {
+ assert !allListeners.contains(l);
allListeners.add(l);
}
--
1.6.2.1.345.g89fb
^ permalink raw reply related
* [EGIT PATCH 3/5] Move dcocument to repository mapping to GitDocument
From: Robin Rosenberg @ 2009-04-02 18:46 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1238697991-12990-3-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../egit/ui/internal/decorators/GitDocument.java | 23 ++++++++++++++++++-
.../internal/decorators/GitQuickDiffProvider.java | 8 +------
2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
index 69e9295..8c82a55 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitDocument.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
+ * Copyright (C) 2008, 2009 Robin Rosenberg <robin.rosenberg@dewire.com>
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
@@ -8,6 +8,8 @@
package org.spearce.egit.ui.internal.decorators;
import java.io.IOException;
+import java.util.Map;
+import java.util.WeakHashMap;
import org.eclipse.core.resources.IEncodedStorage;
import org.eclipse.core.resources.IProject;
@@ -29,6 +31,7 @@
class GitDocument extends Document implements RepositoryListener {
private final IResource resource;
+ static Map<GitDocument,Repository> doc2repo = new WeakHashMap<GitDocument, Repository>();
static GitDocument create(final IResource resource) throws IOException {
GitDocument ret = null;
@@ -44,7 +47,7 @@ static GitDocument create(final IResource resource) throws IOException {
private GitDocument(IResource resource) {
this.resource = resource;
- GitQuickDiffProvider.doc2repo.put(this, getRepository());
+ GitDocument.doc2repo.put(this, getRepository());
}
void populate() throws IOException {
@@ -95,6 +98,7 @@ void populate() throws IOException {
}
void dispose() {
+ doc2repo.remove(this);
Repository repository = getRepository();
if (repository != null)
repository.removeRepositoryChangedListener(this);
@@ -119,4 +123,19 @@ private Repository getRepository() {
return mapping.getRepository();
return null;
}
+
+ /**
+ * A change occurred to a repository. Update any GitDocument instances
+ * referring to such repositories.
+ *
+ * @param repository Repository which changed
+ * @throws IOException
+ */
+ static void refreshRelevant(final Repository repository) throws IOException {
+ for (Map.Entry<GitDocument, Repository> i : doc2repo.entrySet()) {
+ if (i.getValue() == repository) {
+ i.getKey().populate();
+ }
+ }
+ }
}
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
index 88f5ea0..6c71f3c 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/decorators/GitQuickDiffProvider.java
@@ -37,7 +37,6 @@
private IResource resource;
static Map<Repository,String> baseline = new WeakHashMap<Repository,String>();
- static Map<GitDocument,Repository> doc2repo = new WeakHashMap<GitDocument, Repository>();
/**
* Create the GitQuickDiffProvider instance
@@ -48,7 +47,6 @@ public GitQuickDiffProvider() {
public void dispose() {
Activator.trace("(GitQuickDiffProvider) dispose");
- doc2repo.remove(document);
if (document != null)
document.dispose();
}
@@ -96,11 +94,7 @@ public void setId(String id) {
*/
public static void setBaselineReference(final Repository repository, final String baseline) throws IOException {
GitQuickDiffProvider.baseline.put(repository, baseline);
- for (Map.Entry<GitDocument, Repository> i : doc2repo.entrySet()) {
- if (i.getValue() == repository) {
- i.getKey().populate();
- }
- }
+ GitDocument.refreshRelevant(repository);
}
}
--
1.6.2.1.345.g89fb
^ permalink raw reply related
* [EGIT PATCH 1/5] Make the equals method work for AnyObjectId, not just ObjectId
From: Robin Rosenberg @ 2009-04-02 18:46 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
In-Reply-To: <1238697991-12990-1-git-send-email-robin.rosenberg@dewire.com>
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
This has nothing to do with the series, just trying to squeeze it in... The
quickdiff fixes works fine with our without this patch, but it seems reasonable.
.../src/org/spearce/jgit/lib/AnyObjectId.java | 4 ++--
.../src/org/spearce/jgit/revwalk/RevObject.java | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java b/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java
index e2f70ca..2e3a43e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AnyObjectId.java
@@ -248,12 +248,12 @@ public int hashCode() {
* the other id to compare to. May be null.
* @return true only if both ObjectIds have identical bits.
*/
- public boolean equals(final ObjectId other) {
+ public boolean equals(final AnyObjectId other) {
return other != null ? equals(this, other) : false;
}
public boolean equals(final Object o) {
- return equals((ObjectId) o);
+ return equals((AnyObjectId) o);
}
/**
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
index e8fb29f..1a13d0a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevObject.java
@@ -75,7 +75,7 @@ public final ObjectId getId() {
}
@Override
- public final boolean equals(final ObjectId o) {
+ public final boolean equals(final AnyObjectId o) {
return this == o;
}
--
1.6.2.1.345.g89fb
^ permalink raw reply related
* [EGIT PATCH 0/5] Quickdiff improvements
From: Robin Rosenberg @ 2009-04-02 18:46 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
I noticed Eclipse could hang for long period, especially
during rebase. This series fixes that.
-- robin
Robin Rosenberg (5):
Make the equals method work for AnyObjectId, not just ObjectId
quickdiff - Don't add GitDocument as repository listener more than
once
Move dcocument to repository mapping to GitDocument
Update Quickdiff tracing statements
Cache resolved ids in quickdiff document for faster update
.../egit/ui/internal/decorators/GitDocument.java | 104 +++++++++++++++++---
.../internal/decorators/GitQuickDiffProvider.java | 10 +--
.../src/org/spearce/jgit/lib/AnyObjectId.java | 4 +-
.../src/org/spearce/jgit/lib/Repository.java | 2 +
.../src/org/spearce/jgit/revwalk/RevObject.java | 2 +-
5 files changed, 97 insertions(+), 25 deletions(-)
^ permalink raw reply
* Re: Broken t9001 tests
From: Junio C Hamano @ 2009-04-02 18:07 UTC (permalink / raw)
To: Jay Soffian; +Cc: Johannes Schindelin, git, Jeff King
In-Reply-To: <76718490904020714x3f00d5fu6f13587be383be1b@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> On Thu, Apr 2, 2009 at 9:40 AM, Jeff King <peff@peff.net> wrote:
> ...
> Sorry, as Jeff mentions:
>
>> Try
>>
>> http://article.gmane.org/gmane.comp.version-control.git/115280
>
> And BTW, it's not as if I didn't test. The "going interactive" issue
> is hidden by bash. But don't worry, I've since downloaded and
> installed dash just for running git tests, so it won't happen again.
>
>> Junio seems to have picked up the first part of the series, but not this
>> fix. I'm not sure why.
>
> Junio, may I please draw you attention to 3/4 and 4/4 at the link
> above. Please amend the commit message for 3/4 though. It says it
> corrects commit 67f1fe5 which is wrong. Please sed
> 's/67f1fe5/c18f75a/' in that message.
>
> Thanks,
Thanks, all of you. I have been swamped.
^ permalink raw reply
* Re: [PATCH] git remote update: New option --prune (-p)
From: Junio C Hamano @ 2009-04-02 18:06 UTC (permalink / raw)
To: Jeff King; +Cc: demerphq, Finn Arne Gangstad, git, gitster
In-Reply-To: <20090402134414.GB26699@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I think it makes sense under update as pruning is really just a
> different (and perhaps slightly more dangerous) form of update.
> Generally I would only want to run prune after having run update, so
> combining them makes sense from a workflow perspective.
I agree with you that "oh by the way please prune as well" makes perfect
sense, but I actually would even go stronger than that---if we _were_
adding this command today, I would probably make "update" prune by
default, perhaps with an option to skip the pruning step.
I gave the patch an only cursory look, so I wouldn't comment on the
implementation; two things I would look at in the code would be if it
makes two connections to the remote to learn the same information (which
would be bad) and if it skips the pruning stage if the update stage failed
(which would probably be a sane precaution).
^ permalink raw reply
* Re: [PATCH 3/3] mergetool-lib: add diffuse as merge and diff tool
From: Junio C Hamano @ 2009-04-02 18:06 UTC (permalink / raw)
To: David Aguilar; +Cc: Sebastian Pipping, git, spearce
In-Reply-To: <20090402104252.GA29223@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> git-send-email (probably gmail, too) won't let me send
> one patch in a series with a different "From:" header,
> so maybe you should resend this patch so that you can get the
> proper authorship recognition? I've attached the original
> patch file I sent in case you want to just grab it and
> send-email it back out for the purposes of getting the right
> authorship in there (actually, Junio, please advise...,
> I'm not exactly sure what you'd prefer)
The way you marked your 3/3 with a From: line in the body of the message
is the preferred way to say "David forwards a patch from Sebastian", and
our patch application tool knows about it.
If you wanted to preserve the original author date, you could have also
added a Date: line in-body ("David forwards a patch from Sebastian, who
created it at this date in this timezone"). You can find an earlier
example in:
http://article.gmane.org/gmane.comp.version-control.git/115289
^ permalink raw reply
* Re: [PATCH] match_tree_entry(): a pathspec only matches at directory boundaries
From: Linus Torvalds @ 2009-04-02 16:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Björn Steinbrink, Eric Wong, Anton Gyllenberg, git
In-Reply-To: <7vwsa3llac.fsf_-_@gitster.siamese.dyndns.org>
On Wed, 1 Apr 2009, Junio C Hamano wrote:
>
> Previously the code did a simple prefix match, which means that a path in
> a directory "frotz/" would have matched with pathspec "f".
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
I have this suspicion that we should be able to write it more readably,
but yes:
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
because the current code is clearly buggy.
Linus
^ permalink raw reply
* Re: [PATCH] git remote update: New option --prune (-p)
From: Jeff King @ 2009-04-02 16:32 UTC (permalink / raw)
To: demerphq; +Cc: Finn Arne Gangstad, git, gitster
In-Reply-To: <9b18b3110904020907i23f246aelccc2a0770acc2574@mail.gmail.com>
On Thu, Apr 02, 2009 at 06:07:33PM +0200, demerphq wrote:
> Er, personally i find that documentation pretty cryptic. And when i
> check git config for group, i see this:
>
> remotes.<group>
> The list of remotes which are fetched by "git remote update
> <group>". See git-remote(1).
Yeah, this should probably say "separated by space" or whatever (I
actually don't even know). I would assume it contains the remote name; I
can't imagine what other thing it would include. But it wouldn't hurt to
make that more explicit.
I'm sure a documentation patch would be welcome.
> remote.<name>.skipDefaultUpdate
> If true, this remote will be skipped by default when updating using
> the update subcommand of git-remote(1).
>
> Neither of which really explain groups, how to define them properly,
> (the list is separated by what? and includes the remote name?) or
> whether there are implicit groups. I mean, it seems logical that if
> you can have user defined groups that there are some built in ones
> too, like "all" and "none" or perhaps groups defined by transport
> "http" or "git" for instance.
I think what is confusing is that there is exactly one implicit group,
and it is "the default group", which contains every remote that doesn't
have skipDefaultUpdate set. You refer to the "default group" by not
mentioning any group.
So no, there aren't other implicit groups (AFAIK).
You can propose implicit groups, but I think they would have to have a
compelling use case over simply creating them manually. To avoid
conflict with groups people have already defined, they would only be
used if no remotes.$whatever config existed.
I think having "git remote update foo" fall back to a group containing
only the remote "foo" when "remotes.foo" does not exist makes sense.
I'm not sure that "none", "http", or "git" is all that useful in
practice (the only thing I can think of for the latter two is that you
might use "git" versus "http" depending on restrictive firewall
settings).
You could give the unnamed "default group" a name (like "all"), but then
you risk conflict with existing "remotes.all". And in this case, it is
hard to remain backwards compatible: "git remote update" will do
something different now in the case that the user has configured
remotes.all.
-Peff
^ permalink raw reply
* Re: [StGit PATCH] Convert "pop" to the lib infrastructure
From: Catalin Marinas @ 2009-04-02 16:20 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
In-Reply-To: <20090401120515.GA30918@diana.vm.bytemark.co.uk>
2009/4/1 Karl Hasselström <kha@treskal.com>:
> On 2009-03-31 12:30:27 +0100, Catalin Marinas wrote:
>
>> @@ -47,7 +47,7 @@ test_expect_success 'Try to reset with --hard' '
>> stg reset --hard master.stgit^~1 &&
>> stg status a > actual.txt &&
>> test_cmp expected.txt actual.txt &&
>> - test "$(echo $(stg series))" = "> p1 - p3 - p2"
>> + test "$(echo $(stg series))" = "> p1 - p2 - p3"
>> '
>
> Hmm, why this change in behavior? Something that should be noted in
> the commit message?
>
>> @@ -47,7 +47,7 @@ test_expect_success 'Try to undo with --hard' '
>> stg undo --hard &&
>> stg status a > actual.txt &&
>> test_cmp expected.txt actual.txt &&
>> - test "$(echo $(stg series))" = "> p1 - p3 - p2"
>> + test "$(echo $(stg series))" = "> p1 - p2 - p3"
>> '
>
> And I guess this is the same.
I think we now get a slightly different behaviour because of how the
transactions are generated with the new infrastructure. In the above
case, you have "pop p2 p3" and "push p3", the latter failing. The "pop
p2 p3" command results in the stack being "> p1 - p2 - p3" while "push
p3" performs a single step for pushing and reordering. The old push
caused a reorder followed by a push.
So I think I should place the push changes before the pop ones so that
pop itself doesn't fail.
I'll try to push them tonight as I'll go on holiday soon for two weeks.
--
Catalin
^ permalink raw reply
* Re: [PATCH] git remote update: New option --prune (-p)
From: demerphq @ 2009-04-02 16:07 UTC (permalink / raw)
To: Jeff King; +Cc: Finn Arne Gangstad, git, gitster
In-Reply-To: <20090402143112.GA26974@coredump.intra.peff.net>
2009/4/2 Jeff King <peff@peff.net>:
> On Thu, Apr 02, 2009 at 04:17:35PM +0200, demerphq wrote:
>
>> > But "git remote update" actually respects "remote groups", so it is not
>> > just "--all". I think what you want is "git remote prune <group>".
>>
>> Are there any implicit groups defined, like "all-remotes" or
>> something? It seems less than desirable to have to define such a group
>> for an operation that IMO is pretty reasonable to expect to happen
>> regularly.
>
> Yes. From "git help remote":
>
> update
> Fetch updates for a named set of remotes in the repository as
> defined by remotes.<group>. If a named group is not specified on
> the command line, the configuration parameter remotes.default will
> get used; if remotes.default is not defined, all remotes which do
> not have the configuration parameter
> remote.<name>.skipDefaultUpdate set to true will be updated. (See
> git-config(1)).
>
> So without defining any other config, "git remote update" will by
> default update everything
Er, personally i find that documentation pretty cryptic. And when i
check git config for group, i see this:
remotes.<group>
The list of remotes which are fetched by "git remote update
<group>". See git-remote(1).
and
remote.<name>.skipDefaultUpdate
If true, this remote will be skipped by default when updating using
the update subcommand of git-remote(1).
Neither of which really explain groups, how to define them properly,
(the list is separated by what? and includes the remote name?) or
whether there are implicit groups. I mean, it seems logical that if
you can have user defined groups that there are some built in ones
too, like "all" and "none" or perhaps groups defined by transport
"http" or "git" for instance.
>> I personally haven't found any use for defining remote groups yet to
>> be honest. Its a granularity of operation that hasnt served much
>> purpose for me yet. Although i could see it being useful in the
>> future.
>
> I haven't either. I suspect it would be useful if you had a complex set
> of repo relationships, like an integration manager pulling from an
> upstream but also from other developers.
Now that you have called my attention to them in more detail i suspect
ill end up using them for a few things. Maybe ill try to write up a
doc patch once i have.
>> Generally tho I either want to update and prune one remote only, with
>>
>> git fetch $remote; git prune $remote,
>
> It might be useful if "remote update" treated an unconfigured group as a
> simple remote. So that "git remote update --prune $remote" would do what
> you wanted here.
It seems reasonable to me that names for groups and remotes should
stay distinct and that remotes are treated as being groups which
contain only the remote of the same name. These would be yet more
implicit groups.
>
> I could even see "remote.*.autoprune" config being useful so you could
> avoid --prune. It is living dangerously, I suppose, for some workflows;
> but I generally consider whatever is in my remote tracking branches to
> be throwaway, and automatically pruning is not really dangerous.
Me too.
>> or i want to update and prune all with something like:
>>
>> git remote update; for r in $(git remote); do git remote prune $r; done;
>>
>> This patch makes the latter better huffman encoded, but I'd kind of
>> expect both to be doable as single commands in terms of how often I
>> want to do them.
>>
>> Maybe git fetch --prune would be a nice complement to this patch.
>
> I think we have tried to keep pruning out of fetch, as fetch does not
> necessarily use or know about tracking branches. But the "git remote
> update $remote" proposal I gave above would do basically the same thing
> (except you would call it "remote update" instead of "fetch").
Ok, that makes sense. I see why fetch would be left out. Thanks for explaining.
cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ 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