* Re: [EGIT PATCH 4/6] Add tags to the graphical history display.
From: Robin Rosenberg @ 2008-10-06 21:58 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081006080834.GC27516@spearce.org>
måndagen den 6 oktober 2008 10.08.34 skrev Shawn O. Pearce:
> FYI, my comments don't fully cover this patch yet.
>
> > - protected PlotCommit(final AnyObjectId id) {
> > + protected PlotCommit(final AnyObjectId id, final Ref[] tags) {
> > super(id);
> > + this.refs = tags;
>
> this.refs isn't final. There is no reason to be adding it to the
> constructor and having this ripple effect all the way up into the
> RevWalk class.
Eh, it doesn't, ecept I added a methos to RevWalk than can return tags,
which it doesn't unless the information is attached.
> Maybe PlotWalk should override next() and only set PlotCommit.refs on
> things returned from super.next()? This way we only do tag lookup
> on commits that the filtering rules have said should be shown in
> to the application, but the refs should still be inserted prior to
> the application seeing the RevCommit.
I missed something here I think. Thanks. Maybe I thought... doesn't matter.
I'll rewrite the changes in and related to this completely.
> > @@ -54,6 +66,7 @@
> > public PlotWalk(final Repository repo) {
> > super(repo);
> > super.sort(RevSort.TOPO, true);
> > + reverseRefMap = repo.getAllRefsByPeeledObjectId();
>
> I wonder if this shouldn't be done as part of the StartGenerator
> (or something like it but specialized for PlotWalk). I only say
> that because a reused PlotWalk may want to make sure its ref map
> is current with the repository its about to walk against.
noted.
> You are inside of a PlotWalk, which extends RevWalk, which has very
> aggressive caching of RevCommit and RevTag data instances, and very
> fast parsers. Much faster than the legacy Commit and Tag classes.
Maybe we should rid us of them completely and make new commit from
these classes of possible more lightweight ones. The old classes were
straightforward but are on overtime now.
> I think we should use the RevCommit and RevTag classes to handle
> sorting here. RevCommit already has the committer time cached
> and stored in an int. RevCommit probably should learn to do the
> same for its "tagger" field. The tiny extra bloat (1 word) that
> adds to a RevTag instance is worth it when we consider implementing
> something like this and/or git-describe where sorting tags by their
> dates is useful.
As you noted earlier we usually have at most one ref per commit to sort.
Not much to optimize for speed here with current repos. For a one-element
list the compare routine wil not even be invoked.
>
> > + tags = list.toArray(new Ref[list.size()]);
>
> I wonder if using a Ref[] here even makes sense given that the data
> is stored in a List<Ref>. I use RevCommit[] inside of RevCommit
> generally because the number of entries in the array is 1 or 2 and
> the array is smaller than say an ArrayList.
>
> In hindsight those RevCommit[] probably should be a List<RevCommit>
> with different list implementations based on the number of parents
> needed:
>
> 0 parents: Collections.emptyList()
> 1 parent: Collections.singletonList()
> 2 parents: some especialized AbstractList subclass
> 3 parents: ArrayList
>
> I think it would actually use less memory per instance, which is
> a huge bonus, but we'd pay a downcasting penalty on each access.
> HotSpot is supposed to be really good at removing the downcast
> penalty from say java.util.List, but I don't if it can beat a typed
> array access.
I guess measuring is the right way. For these small arrays, my bet
is that List is way faster because we do not need any bounds checking.
Hotspot is reasonably good at optimizing those away, but that won't
help for much small arrays.
> Sorry I got off on a bit of a tangent here. I'm just trying to
> point out that the primary reason I've usd an array before is
> probably moot here since the data is already in a List.
Could it be that arrays are often better then lists.
> > diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
> > index d7e4c58..41d57c6 100644
> > --- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
> > +++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
> > @@ -53,6 +53,7 @@
>
> IMHO this class shouldn't need to be modified.
>
> > @@ -541,7 +542,7 @@ public RevTree lookupTree(final AnyObjectId id) {
> > public RevCommit lookupCommit(final AnyObjectId id) {
> > RevCommit c = (RevCommit) objects.get(id);
> > if (c == null) {
> > - c = createCommit(id);
> > + c = createCommit(id, getTags(id));
>
> This code is performance critical to commit parsing. Trying to
> lookup tags for every commit we evaluate, especially ones that we
> will never show in the UI (because they are uninteresting) but that
> we still need to parse in order to derive the merge base is just
> a huge waste of time.
>
> Same applies for the lookupAny and parseAny methods.
>
Ack.
-- robin
^ permalink raw reply
* files missing after converting a cvs repository to git
From: Adam Mercer @ 2008-10-06 22:02 UTC (permalink / raw)
To: GIT
Hi
One of the prrojects I am involved with is currently looking into
migrating from cvs to git, therefore we have been investigating this
by setting up a git repository that tracks cvs, however there are some
very strange things going on and I was hoping someone could offer some
insight into what is going on.
I use the following git cvsimport command to import the repository:
$ git cvsimport -v -a -i -k
-d:pserver:user@server:port/path/to/cvs/repo -C /path/to/new/git/repo
module
this ran successfully with no warnings or errors. However, when I
checkout the new git repository that are several files missing from
the git checkout that are present in the cvs checkout.
Does anyone know why this would happen, or how to find out? This is a
major problem because we will be unable to migrate to cvs until this
can be solved.
Cheers
Adam
^ permalink raw reply
* Re: [EGIT PATCH 4/6] Add tags to the graphical history display.
From: Shawn O. Pearce @ 2008-10-06 22:14 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810062358.44954.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> måndagen den 6 oktober 2008 10.08.34 skrev Shawn O. Pearce:
> > Maybe PlotWalk should override next() [...]
>
> I missed something here I think. Thanks. Maybe I thought... doesn't matter.
> I'll rewrite the changes in and related to this completely.
OK, looking forward to the changes.
> > > @@ -54,6 +66,7 @@
> > > public PlotWalk(final Repository repo) {
> > > super(repo);
> > > super.sort(RevSort.TOPO, true);
> > > + reverseRefMap = repo.getAllRefsByPeeledObjectId();
> >
> > I wonder if this shouldn't be done as part of the StartGenerator
> > (or something like it but specialized for PlotWalk). I only say
> > that because a reused PlotWalk may want to make sure its ref map
> > is current with the repository its about to walk against.
>
> noted.
Be careful. StartGenerator is package access only and I think
PlotWalk is in a different package.
IMHO binding into the StartGenerator (by subclassing it and replacing
it) feels like the right thing to me, but it means making a bunch
of changes to RevWalk and StartGenerator to make that type public
and allow PlotWalk to inject a different subclass of StartGenerator
for itself.
The StartGenerator trick is very useful though; it allows the RevWalk
to perform init activity only on the first invocation of next()
and then removes the init code entirely from the call path. It is a
virtual dispatch, but I figured its a virtual dispatch anyway due to
the way the other generators are chained together based the filters
so we at least avoid a "if (first) {...}" test on every call to next.
> > You are inside of a PlotWalk, which extends RevWalk, which has very
> > aggressive caching of RevCommit and RevTag data instances, and very
> > fast parsers. Much faster than the legacy Commit and Tag classes.
>
> Maybe we should rid us of them completely and make new commit from
> these classes of possible more lightweight ones. The old classes were
> straightforward but are on overtime now.
I'm trying to go in that direction. However there are three reasons
why we aren't ready yet:
- Commits can only be made by the old-style Commit class.
- Tags can only be made by the old-style Tag class.
- Trees can only be made by the old-style Tree class.
I have some experimental code in my merge branch to solve the last
one by teaching DirCache how to write out the index and update the
cache trees with the ObjectIds of the newly created trees, but I
have not had a chance to clean the patches up with test cases and
docs to submit to the git ML yet.
So I'm actively working on fixing the last item.
Oh, and of course existing callers have to be converted. But I am
trying to avoid introducing new callers.
> As you noted earlier we usually have at most one ref per commit to sort.
> Not much to optimize for speed here with current repos. For a one-element
> list the compare routine wil not even be invoked.
Oh, good point.
> > In hindsight those RevCommit[] probably should be a List<RevCommit>
> > with different list implementations based on the number of parents
> > needed:
> >
> > 0 parents: Collections.emptyList()
> > 1 parent: Collections.singletonList()
> > 2 parents: some especialized AbstractList subclass
> > 3 parents: ArrayList
>
> I guess measuring is the right way. For these small arrays, my bet
> is that List is way faster because we do not need any bounds checking.
> Hotspot is reasonably good at optimizing those away, but that won't
> help for much small arrays.
Yup. Its something to explore. But I lack the time right now so
I'm going going to be making any changes and measuring it anytime
soon. ;-)
Maybe someone who wants to get involved can look at this. Or any
other number of issues we still have open.
But without measurements to back up the code either way I won't be
making changes to what we have right now.
--
Shawn.
^ permalink raw reply
* RE: gitweb improvements
From: Tjernlund @ 2008-10-06 22:17 UTC (permalink / raw)
To: 'Jakub Narebski'; +Cc: 'git'
In-Reply-To: <200810060154.06934.jnareb@gmail.com>
> -----Original Message-----
> From: Jakub Narebski [mailto:jnareb@gmail.com]
> Sent: den 6 oktober 2008 01:54
> To: Tjernlund
> Cc: 'git'
> Subject: Re: gitweb improvements
>
> On Mon, 6 Oct 2008, Tjernlund wrote:
> > Jakub Narebski wrote:
> >> "Tjernlund" <tjernlund@tjernlund.se> writes:
>
> >>> 2) looking at a merge like:
> >>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
> 2.6.git;a=commit;h=66120005e65eed8a05b14a36ab448bdec42f0d6b
> >>> is somewhat confusing. It really doesn't tell you which commits that is
> >>> included in the merge.
> >>
> >> I don't understand you there. First, you have "(merge: 0d0f3ef 9778e9a)"
> >> in the navbar, so you can easily go to commit view for parents. Second,
> >> among commit headers you have two "parent", where SHA-1 of a commit is
> >> hidden link, and there are also 'commit' and 'diff' link for those.
> >
> > hmm, looks like I overlooked "(merge: 0d0f3ef 9778e9a)" part. However, I can't
> > find the "ALSA: make the CS4270 driver a new-style I2C driver" from within
> > this page.
>
> I think you don't quite understand the situation. The history looks
> like this:
>
> M Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6
> |\
> | 2 ALSA: ASoC: Fix another cs4270 error path
> | * ALSA: make the CS4270 driver a new-style I2C driver
> | |
> 1 | Merge git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi-rc-fixes-2.6
>
> Parents of commit 'M' (for merge) are '1' and '2', not 2,* or 1,2,*.
>
> Now the fact that commit message for merge contains shortlog of merged
> branch does not mean that there must be direct link to such shortlog.
> You can go to shortlog (well, kind of) if you click on second parent,
> _then_ click on shortlog link at top of the page.
You are quite right, I didn't quite follow the situation. I do however have one
observation, would it be possible to list the commits in a merge the same way
you list commits on the top level, that is, more or less make the headings
"ALSA: ASoC: Fix another cs4270 error path"
"ALSA: make the CS4270 driver a new-style I2C driver"
links one can follow?
Jocke
^ permalink raw reply
* [PATCH] Implement git clone -v
From: Miklos Vajna @ 2008-10-06 22:19 UTC (permalink / raw)
To: Alex Riesen; +Cc: Constantine Plotnikov, git
In-Reply-To: <81b0412b0810041442i3aa29628lf66ef9b188fe8ce7@mail.gmail.com>
The new -v option forces the progressbar, even in case the output is not
a terminal.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Sat, Oct 04, 2008 at 11:42:15PM +0200, Alex Riesen <raa.lkml@gmail.com> wrote:
> 2008/10/3 Constantine Plotnikov <constantine.plotnikov@gmail.com>:
> > Is there a way to force a progress output on stderr for git clone
> > preferably using options or environment variables?
>
> No, but "-v" option is not used for anything yet, so you are free to
> implement it.
Something like this?
Documentation/git-clone.txt | 5 +++++
builtin-clone.c | 4 ++++
transport.c | 2 +-
transport.h | 2 ++
4 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 0e14e73..95f08b9 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -90,6 +90,11 @@ then the cloned repository will become corrupt.
Operate quietly. This flag is also passed to the `rsync'
command when given.
+--verbose::
+-v::
+ Display the progressbar, even in case the standard output is not
+ a terminal.
+
--no-checkout::
-n::
No checkout of HEAD is performed after the clone is complete.
diff --git a/builtin-clone.c b/builtin-clone.c
index 49d2eb9..df71b23 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -38,9 +38,11 @@ static int option_local, option_no_hardlinks, option_shared;
static char *option_template, *option_reference, *option_depth;
static char *option_origin = NULL;
static char *option_upload_pack = "git-upload-pack";
+static int option_verbose;
static struct option builtin_clone_options[] = {
OPT__QUIET(&option_quiet),
+ OPT__VERBOSE(&option_verbose),
OPT_BOOLEAN('n', "no-checkout", &option_no_checkout,
"don't create a checkout"),
OPT_BOOLEAN(0, "bare", &option_bare, "create a bare repository"),
@@ -506,6 +508,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (option_quiet)
transport->verbose = -1;
+ else if (option_verbose)
+ transport->progress = 1;
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
diff --git a/transport.c b/transport.c
index 5110c56..1c510a3 100644
--- a/transport.c
+++ b/transport.c
@@ -644,7 +644,7 @@ static int fetch_refs_via_pack(struct transport *transport,
args.include_tag = data->followtags;
args.verbose = (transport->verbose > 0);
args.quiet = (transport->verbose < 0);
- args.no_progress = args.quiet || !isatty(1);
+ args.no_progress = args.quiet || (!transport->progress && !isatty(1));
args.depth = data->depth;
for (i = 0; i < nr_heads; i++)
diff --git a/transport.h b/transport.h
index d0b5205..6bbc1a8 100644
--- a/transport.h
+++ b/transport.h
@@ -25,6 +25,8 @@ struct transport {
int (*disconnect)(struct transport *connection);
char *pack_lockfile;
signed verbose : 2;
+ /* Force progress even if the output is not a tty */
+ unsigned progress : 1;
};
#define TRANSPORT_PUSH_ALL 1
--
1.6.0.2
^ permalink raw reply related
* Re: [EGIT PATCH 3/6] Add a method to get refs by object Id
From: Robin Rosenberg @ 2008-10-06 22:37 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081006081554.GD27516@spearce.org>
måndagen den 6 oktober 2008 10.15.54 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > 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 dfce1b8..3fc5236 100644
> > --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> > +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> > @@ -939,6 +940,33 @@ public String getBranch() throws IOException {
> > }
> >
> > /**
> > + * @return a map with all objects referenced by a peeled ref.
> > + */
> > + public Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {
>
> Do we really want to promise List here? Can we make it just
> Collection instead?
Sure. Our promise is actually slightly better, it is Set, but Java doesn't have a suitable class for that.
>
> > + Map<String, Ref> allRefs = getAllRefs();
> > + HashMap<AnyObjectId, List<Ref>> ret = new HashMap<AnyObjectId, List<Ref>>(allRefs.size());
> > + for (Map.Entry<String,Ref> e : allRefs.entrySet()) {
> > + Ref ref = e.getValue();
>
> I think this is cleaner:
>
> for (Ref ref : allRefs.values()) {
>
> as you never use the key.
Yes. I was thinking it might be less efficient, but the JDK implementation looks quite well optimized in 1.6 at least
so values() is slightly faster.
> > + AnyObjectId target = ref.getPeeledObjectId();
> > + if (target == null)
> > + target = ref.getObjectId();
> > + List<Ref> list = ret.get(target);
> > + if (list == null) {
> > + list = Collections.singletonList(ref);
> > + } else {
> > + if (list.size() == 1) {
> > + ArrayList<Ref> list2 = new ArrayList<Ref>(2);
> > + list2.add(list.get(0));
> > + list = list2;
> > + }
> > + list.add(ref);
> > + }
> > + ret.put(target, list);
>
> Hmm. Putting the list every time is pointless. This is may run
> faster because we (on average) only do one hash lookup per target,
> not 2:
>
> List<Ref> nl = Collections.singletonList(ref);
> List<Ref> ol = ret.put(target, nl);
> if (ol != null) {
> if (ol.size() == 1) {
> nl = new ArrayList<Ref>(2);
> nl.add(ol.get(0));
> nl.add(ref);
> ret.put(target, nl);
> } else {
> ol.add(ref)
> ret.put(target, ol);
> }
> }
ok, I guess one just has has to include the comment on why for the casual reader.
-- robin
^ permalink raw reply
* Re: [EGIT PATCH 3/6] Add a method to get refs by object Id
From: Shawn O. Pearce @ 2008-10-06 22:43 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200810070037.53841.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> måndagen den 6 oktober 2008 10.15.54 skrev Shawn O. Pearce:
> > Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > >
> > > /**
> > > + * @return a map with all objects referenced by a peeled ref.
> > > + */
> > > + public Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {
> >
> > Do we really want to promise List here? Can we make it just
> > Collection instead?
>
> Sure. Our promise is actually slightly better, it is Set, but Java doesn't have a suitable class for that.
java.util.Set? java.util.HashSet?
What am I missing?
> > > + for (Map.Entry<String,Ref> e : allRefs.entrySet()) {
> > > + Ref ref = e.getValue();
> >
> > I think this is cleaner:
> >
> > for (Ref ref : allRefs.values()) {
> >
> > as you never use the key.
>
> Yes. I was thinking it might be less efficient, but the JDK implementation looks quite well optimized in 1.6 at least
> so values() is slightly faster.
I wasn't concerned about performance, I was going for readability.
This loop is probably not performance critical as its done once
early when the PlotWalk starts. I just found that grabbing the
entrySet when all you care about is the values was odd.
> > List<Ref> nl = Collections.singletonList(ref);
> > List<Ref> ol = ret.put(target, nl);
> > if (ol != null) {
> > if (ol.size() == 1) {
> > nl = new ArrayList<Ref>(2);
> > nl.add(ol.get(0));
> > nl.add(ref);
> > ret.put(target, nl);
> > } else {
> > ol.add(ref)
> > ret.put(target, ol);
> > }
> > }
>
> ok, I guess one just has has to include the comment on why for the casual reader.
I'm probably too used to this pattern of "put, then test" because
I use it a lot when the odds of put returning null are very high.
Its an odd idiom. I think most developers wouldn't write it.
So I guess a comment of some sort is probably a good idea if you
chose to use this mess. ;-)
--
Shawn.
^ permalink raw reply
* [PATCH] Replace xmalloc/memset(0) pairs with xcalloc
From: Brandon Casey @ 2008-10-06 23:39 UTC (permalink / raw)
To: Git Mailing List
Many call sites immediately initialize allocated memory with zero after
calling xmalloc. A single call to xcalloc can replace this two-call
sequence.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
builtin-merge.c | 3 +--
builtin-pack-objects.c | 4 +---
builtin-unpack-objects.c | 3 +--
merge-tree.c | 3 +--
remote.c | 3 +--
5 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 5c65a58..dcf8987 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -123,8 +123,7 @@ static struct strategy *get_strategy(const char *name)
exit(1);
}
- ret = xmalloc(sizeof(struct strategy));
- memset(ret, 0, sizeof(struct strategy));
+ ret = xcalloc(1, sizeof(struct strategy));
ret->name = xstrdup(name);
return ret;
}
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 1158e42..59c30d1 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1369,12 +1369,10 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
int window, int depth, unsigned *processed)
{
uint32_t i, idx = 0, count = 0;
- unsigned int array_size = window * sizeof(struct unpacked);
struct unpacked *array;
unsigned long mem_usage = 0;
- array = xmalloc(array_size);
- memset(array, 0, array_size);
+ array = xcalloc(window, sizeof(struct unpacked));
for (;;) {
struct object_entry *entry = *list++;
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index d2796b6..9f4bdd3 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -477,8 +477,7 @@ static void unpack_all(void)
if (!quiet)
progress = start_progress("Unpacking objects", nr_objects);
- obj_list = xmalloc(nr_objects * sizeof(*obj_list));
- memset(obj_list, 0, nr_objects * sizeof(*obj_list));
+ obj_list = xcalloc(nr_objects, sizeof(*obj_list));
for (i = 0; i < nr_objects; i++) {
unpack_one(i);
display_progress(progress, i + 1);
diff --git a/merge-tree.c b/merge-tree.c
index 02fc10f..2d1413e 100644
--- a/merge-tree.c
+++ b/merge-tree.c
@@ -158,9 +158,8 @@ static int same_entry(struct name_entry *a, struct name_entry *b)
static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsigned char *sha1, const char *path)
{
- struct merge_list *res = xmalloc(sizeof(*res));
+ struct merge_list *res = xcalloc(1, sizeof(*res));
- memset(res, 0, sizeof(*res));
res->stage = stage;
res->path = path;
res->mode = mode;
diff --git a/remote.c b/remote.c
index c45d96e..a2d7ab1 100644
--- a/remote.c
+++ b/remote.c
@@ -751,8 +751,7 @@ int remote_find_tracking(struct remote *remote, struct refspec *refspec)
struct ref *alloc_ref(unsigned namelen)
{
- struct ref *ret = xmalloc(sizeof(struct ref) + namelen);
- memset(ret, 0, sizeof(struct ref) + namelen);
+ struct ref *ret = xcalloc(1, sizeof(struct ref) + namelen);
return ret;
}
--
1.6.0.2.445.g1e1e0
^ permalink raw reply related
* merge -s ffonly
From: Shawn O. Pearce @ 2008-10-06 23:56 UTC (permalink / raw)
To: git; +Cc: Randal L. Schwartz
I really don't care about this feature. But Randal's whining on
#git made me stop what I was doing and write something that might
turn into it.
Totally untested code. It might reformat your C:\ drive and install
Windows ME. Install as $(git --exec-path)/git-merge-ffonly and
call as `git merge -s ffonly`.
If you care about this sort of feature, test it, write tests for it,
make a formal patch, and send it for review. No, I will not do this
for you. As I said, I don't care about this as a feature.
--8<--
diff --git a/git-merge-ffonly.sh b/git-merge-ffonly.sh
new file mode 100644
index 0000000..24363b5
--- /dev/null
+++ b/git-merge-ffonly.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+while test $# -gt 0
+do
+ if test "z$1" = z--
+ then
+ shift
+ break
+ else
+ shift
+ fi
+done
+
+while test $# -gt 0
+do
+ if test -n "$(git rev-list $1..HEAD)"
+ then
+ exit 2
+ fi
+ shift
+done
--
Shawn.
^ permalink raw reply related
* [PATCH 1/2 v2] check-attr: add an internal check_attr() function
From: Dmitry Potapov @ 2008-10-07 0:14 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Alexander Gavrilov, git, Shawn O. Pearce, Paul Mackerras
In-Reply-To: <48E9B997.1010006@viscovery.net>
This step is preparation to introducing --stdin-paths option.
I have also added maybe_flush_or_die() at the end of main() to ensure that
we exit with the zero code only when we flushed the output successfully.
Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
maybe_flush_or_die() is added in this version of patch.
builtin-check-attr.c | 42 ++++++++++++++++++++++++------------------
1 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/builtin-check-attr.c b/builtin-check-attr.c
index cb783fc..786256e 100644
--- a/builtin-check-attr.c
+++ b/builtin-check-attr.c
@@ -6,6 +6,27 @@
static const char check_attr_usage[] =
"git check-attr attr... [--] pathname...";
+static void check_attr(int cnt, struct git_attr_check *check,
+ const char** name, const char *file)
+{
+ int j;
+ if (git_checkattr(file, cnt, check))
+ die("git_checkattr died");
+ for (j = 0; j < cnt; j++) {
+ const char *value = check[j].value;
+
+ if (ATTR_TRUE(value))
+ value = "set";
+ else if (ATTR_FALSE(value))
+ value = "unset";
+ else if (ATTR_UNSET(value))
+ value = "unspecified";
+
+ quote_c_style(file, NULL, stdout, 0);
+ printf(": %s: %s\n", name[j], value);
+ }
+}
+
int cmd_check_attr(int argc, const char **argv, const char *prefix)
{
struct git_attr_check *check;
@@ -42,23 +63,8 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
check[i].attr = a;
}
- for (i = doubledash; i < argc; i++) {
- int j;
- if (git_checkattr(argv[i], cnt, check))
- die("git_checkattr died");
- for (j = 0; j < cnt; j++) {
- const char *value = check[j].value;
-
- if (ATTR_TRUE(value))
- value = "set";
- else if (ATTR_FALSE(value))
- value = "unset";
- else if (ATTR_UNSET(value))
- value = "unspecified";
-
- quote_c_style(argv[i], NULL, stdout, 0);
- printf(": %s: %s\n", argv[j+1], value);
- }
- }
+ for (i = doubledash; i < argc; i++)
+ check_attr(cnt, check, argv+1, argv[i]);
+ maybe_flush_or_die(stdout, "attribute to stdout");
return 0;
}
--
1.6.0.2.447.g3befd
^ permalink raw reply related
* [PATCH 2/2 v2] check-attr: Add --stdin-paths option
From: Dmitry Potapov @ 2008-10-07 0:16 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Alexander Gavrilov, git, Shawn O. Pearce, Paul Mackerras
In-Reply-To: <48E9B997.1010006@viscovery.net>
This allows multiple paths to be specified on stdin.
Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
On Mon, Oct 06, 2008 at 09:09:11AM +0200, Johannes Sixt wrote:
>
> The least that is needed is fflush(stdout) in this loop (after each
> iteration!)
Thanks. Somehow, I forgot about it though it is quite obvious.
I have added maybe_flush_or_die().
Documentation/git-check-attr.txt | 4 ++
builtin-check-attr.c | 70 ++++++++++++++++++++++++++++++++------
t/t0003-attributes.sh | 17 +++++++++
3 files changed, 80 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 2b821f2..0839a57 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -9,6 +9,7 @@ git-check-attr - Display gitattributes information.
SYNOPSIS
--------
'git check-attr' attr... [--] pathname...
+'git check-attr' --stdin-paths attr... < <list-of-paths
DESCRIPTION
-----------
@@ -17,6 +18,9 @@ For every pathname, this command will list if each attr is 'unspecified',
OPTIONS
-------
+--stdin-paths::
+ Read file names from stdin instead of from the command-line.
+
\--::
Interpret all preceding arguments as attributes, and all following
arguments as path names. If not supplied, only the first argument will
diff --git a/builtin-check-attr.c b/builtin-check-attr.c
index 786256e..fa1e4d5 100644
--- a/builtin-check-attr.c
+++ b/builtin-check-attr.c
@@ -2,9 +2,19 @@
#include "cache.h"
#include "attr.h"
#include "quote.h"
+#include "parse-options.h"
-static const char check_attr_usage[] =
-"git check-attr attr... [--] pathname...";
+static int stdin_paths;
+static const char * const check_attr_usage[] = {
+"git check-attr attr... [--] pathname...",
+"git check-attr --stdin-paths attr... < <list-of-paths>",
+NULL
+};
+
+static const struct option check_attr_options[] = {
+ OPT_BOOLEAN(0 , "stdin-paths", &stdin_paths, "read file names from stdin"),
+ OPT_END()
+};
static void check_attr(int cnt, struct git_attr_check *check,
const char** name, const char *file)
@@ -27,17 +37,44 @@ static void check_attr(int cnt, struct git_attr_check *check,
}
}
+static void check_attr_stdin_paths(int cnt, struct git_attr_check *check,
+ const char** name)
+{
+ struct strbuf buf, nbuf;
+
+ strbuf_init(&buf, 0);
+ strbuf_init(&nbuf, 0);
+ while (strbuf_getline(&buf, stdin, '\n') != EOF) {
+ if (buf.buf[0] == '"') {
+ strbuf_reset(&nbuf);
+ if (unquote_c_style(&nbuf, buf.buf, NULL))
+ die("line is badly quoted");
+ strbuf_swap(&buf, &nbuf);
+ }
+ check_attr(cnt, check, name, buf.buf);
+ maybe_flush_or_die(stdout, "attribute to stdout");
+ }
+ strbuf_release(&buf);
+ strbuf_release(&nbuf);
+}
+
int cmd_check_attr(int argc, const char **argv, const char *prefix)
{
struct git_attr_check *check;
int cnt, i, doubledash;
+ const char *errstr = NULL;
+
+ argc = parse_options(argc, argv, check_attr_options, check_attr_usage,
+ PARSE_OPT_KEEP_DASHDASH);
+ if (!argc)
+ usage_with_options(check_attr_usage, check_attr_options);
if (read_cache() < 0) {
die("invalid cache");
}
doubledash = -1;
- for (i = 1; doubledash < 0 && i < argc; i++) {
+ for (i = 0; doubledash < 0 && i < argc; i++) {
if (!strcmp(argv[i], "--"))
doubledash = i;
}
@@ -45,26 +82,37 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
/* If there is no double dash, we handle only one attribute */
if (doubledash < 0) {
cnt = 1;
- doubledash = 1;
+ doubledash = 0;
} else
- cnt = doubledash - 1;
+ cnt = doubledash;
doubledash++;
- if (cnt <= 0 || argc < doubledash)
- usage(check_attr_usage);
+ if (cnt <= 0)
+ errstr = "No attribute specified";
+ else if (stdin_paths && doubledash < argc)
+ errstr = "Can't specify files with --stdin-paths";
+ if (errstr) {
+ error (errstr);
+ usage_with_options(check_attr_usage, check_attr_options);
+ }
+
check = xcalloc(cnt, sizeof(*check));
for (i = 0; i < cnt; i++) {
const char *name;
struct git_attr *a;
- name = argv[i + 1];
+ name = argv[i];
a = git_attr(name, strlen(name));
if (!a)
return error("%s: not a valid attribute name", name);
check[i].attr = a;
}
- for (i = doubledash; i < argc; i++)
- check_attr(cnt, check, argv+1, argv[i]);
- maybe_flush_or_die(stdout, "attribute to stdout");
+ if (stdin_paths)
+ check_attr_stdin_paths(cnt, check, argv);
+ else {
+ for (i = doubledash; i < argc; i++)
+ check_attr(cnt, check, argv, argv[i]);
+ maybe_flush_or_die(stdout, "attribute to stdout");
+ }
return 0;
}
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 3d8e06a..f6901b4 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -47,6 +47,23 @@ test_expect_success 'attribute test' '
'
+test_expect_success 'attribute test: read paths from stdin' '
+
+ cat <<EOF > expect
+f: test: f
+a/f: test: f
+a/c/f: test: f
+a/g: test: a/g
+a/b/g: test: a/b/g
+b/g: test: unspecified
+a/b/h: test: a/b/h
+a/b/d/g: test: a/b/d/*
+EOF
+
+ sed -e "s/:.*//" < expect | git check-attr --stdin-paths test > actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'root subdir attribute test' '
attr_check a/i a/i &&
--
1.6.0.2.447.g3befd
^ permalink raw reply related
* Re: [PATCH] error out if path is invalid
From: Dmitry Potapov @ 2008-10-07 0:22 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Shawn O. Pearce, git
In-Reply-To: <48E9B7FE.2000503@viscovery.net>
On Mon, Oct 06, 2008 at 09:02:22AM +0200, Johannes Sixt wrote:
> Dmitry Potapov schrieb:
> > if (!verify_path(path))
> > - return -1;
> > + return error("Invalid path '%s'", path);
>
> Look at this change. Didn't the code error out before, too?
It is certainly did not here. As to its caller, it depends. In fact,
there are two chunks like that in my patch, so I am not sure to which
one you refer here. If we speak about add_cacheinfo() then though the
function did not error out, its caller died with one of the following
messages:
git update-index: unable to update some-file-name
or
git update-index: --cacheinfo cannot add some-file-name
However, if we speak about add_index_entry_with_check then the caller
will not produce any error. The git would exit successfully (it still
does) and there was no error message as if everything was fine.
Perhaps, the exit code should be corrected too, but if the git just dies
when add_index_entry() fails it may cause that having one invalid path
will prevent to check out other files, which does not seem to be the
right thing to do.
As to correction to correction to make_cache_entry then after my
previous patch, it started to error out:
make_cache_entry failed for path 'some-file-name'
before that it silently segfaulted.
> Same in the
> other cases. Hence, your patch subject does not describe the patch.
Should I include the above explanation in the commit message or do you
have any objection to having the above error message in cases where the
caller already produce some message when it dies?
Dmitry
^ permalink raw reply
* error pushing stash ?
From: David Bryson @ 2008-10-07 0:34 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]
I have a git mirror remote setup on a few of my repositories:
[remote "backup"]
url = /users/dbryson/backup/janus.git/
fetch = +refs/heads/*:refs/remotes/origin/*
receivepack = sudo -u dbryson git-receive-pack
mirror = 1
I send my refs to the backup with:
$ git push backup
Only to find some odd error messages:
Counting objects: 133, done.
Compressing objects: 100% (109/109), done.
Writing objects: 100% (109/109), 31.25 KiB, done.
Total 109 (delta 82), reused 0 (delta 0)
error: refusing to create funny ref 'refs/stash' remotely
To /users/dbryson/backup/janus.git/
549f8a4..8e93d51 8654 -> 8654
ef6195b..549f8a4 origin/8654 -> origin/8654
+ 623e7cb...63d7262 origin/master -> origin/master (forced update)
! [remote rejected] refs/stash -> refs/stash (funny refname)
error: failed to push some refs to '/users/dbryson/backup/janus.git/'
Should I be concnerned about this or is it normal ? To be honest the
fact that the stash isn't pushing doesn't bother me. But maybe it is a
symptom of a larger problem ?
Dave
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: error pushing stash ?
From: Shawn O. Pearce @ 2008-10-07 0:40 UTC (permalink / raw)
To: David Bryson; +Cc: git
In-Reply-To: <20081007003435.GZ5774@eratosthenes.cryptobackpack.org>
David Bryson <david@statichacks.org> wrote:
> [remote "backup"]
...
> mirror = 1
>
> Only to find some odd error messages:
>
> $ git push backup
> Counting objects: 133, done.
> Compressing objects: 100% (109/109), done.
> Writing objects: 100% (109/109), 31.25 KiB, done.
> Total 109 (delta 82), reused 0 (delta 0)
> error: refusing to create funny ref 'refs/stash' remotely
> To /users/dbryson/backup/janus.git/
> 549f8a4..8e93d51 8654 -> 8654
> ef6195b..549f8a4 origin/8654 -> origin/8654
> + 623e7cb...63d7262 origin/master -> origin/master (forced update)
> ! [remote rejected] refs/stash -> refs/stash (funny refname)
> error: failed to push some refs to '/users/dbryson/backup/janus.git/'
refs/stash is a funny refname because it contains only 1 '/'.
Normally a valid ref has at least 2 '/', e.g. refs/heads/8654 or
refs/tags/v1.0.
Naming the stash refs/stash was perhaps funny in the first place
since it cannot be moved about on the transport protocol, but then
again the bulk of the stash data is actually in the reflog for the
stash (and not the stash ref itself) so there is basically no point
in pushing or fetching a stash directly.
--
Shawn.
^ permalink raw reply
* Re: Files with colons under Cygwin
From: Dmitry Potapov @ 2008-10-07 0:53 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <48E9B634.6040909@viscovery.net>
On Mon, Oct 06, 2008 at 08:54:44AM +0200, Johannes Sixt wrote:
>
> IIUC, verify_path() checks paths that were found in the database or the
> index.
It also checks paths that was given to git add and other commands to
prevent an invalid path to enter to the index. If I am not mistaken,
if invalid path has entered in the index then it will be committed to
the database without any further checks.
> As such, it checks for the integrity of the database. And paths
> with backslashes or colons certainly do not violate the database integrity.
No, it has nothing to do with the database. You can run git fsck --full
on a repository that contains '..' or '.' or '.git', and there will be
no error. Having those names does not violate the database integrity,
as the database is concerned all names are just bytes separated by '/',
so having name '.' is not a problem for it. However, names '.' and '..'
have special meaning for the filesystem, and paths starting with .git/
have special meaning for Git repository. If you work in a bare repo
then those names are not a problem, but once you have a working tree,
you want make sure that there is nothing wrong with it.
>
> More precisely, the exchange of path names between the index and tree
> objects (both directions) should not do this new check, nor if a path is
> added to the index. The check is only meaningful[*] when a path is read
> from the index or a tree object and "applied" to the working directory.
> Unfortunately, I think there are lots of places where this happens.
>
> [*] I say "meaningful" and not "necessary" because the situation is just
> like when you grab some random SoftwarePackage.tar.gz, and run ./configure
> without looking first what it is going to do.
When I grab any tar, I can look at its context without myself of any
risk that some files can be overwritten on my file system. And when
I want to look at some remote git repository, I usually do:
git clone URL
If it can overwrite some files behind my back, it is security a hole.
BTW, it was the reason why the idea of allowing .gitconfig to be stored
in git repository (similar to .gitignore) was stroke down about a year ago
though it could help some clueless users...
On Linux (or other sane file systems), we have all required checks to
prevent that from happening, and they are places in verify_path, which
prevents malicious names entering into the index and thus to the file
system too. So, we should do all required checks on Windows too.
Now, I've realized that my checks were not sufficient (due to Windows
being case-insensitive), so I will add more checks and resend this
patch later.
Dmitry
^ permalink raw reply
* Re: [PATCH 0/4] diff text conversion filter
From: Jeff King @ 2008-10-07 1:20 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Johannes Sixt, git
In-Reply-To: <vpqtzbpwy9h.fsf@bauges.imag.fr>
On Mon, Oct 06, 2008 at 05:15:22PM +0200, Matthieu Moy wrote:
> Actually, I understand you don't want git gui and gitk to load MS-Word
> anytime you click something, but I'd love to see the textconv+diff in
> gitk.
>
> (yeah, that's pretty hard to specify right, the ideal requirement
> seems to be "in a gui, use the good part of the diff driver, but not
> the other" :-\).
I think it is even more complex than that. Sometimes when doing "git
show" I want to see the textconv'd version, and sometimes I don't. So I
really want a command-line flag or environment variable that I can use
to control it (with a sane default).
So probably the way forward is to stop relying on "oh, we just didn't
parse some of the config, so it won't get used" to just parsing the
config all the time and having a diffopt for "do textconv, do external
diff, etc". And then plumbing can make sure that those flags never get
set, and porcelain can tie them to command-line options.
-Peff
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Dmitry Potapov @ 2008-10-07 1:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <alpine.LFD.2.00.0810061246460.3208@nehalem.linux-foundation.org>
On Mon, Oct 06, 2008 at 12:47:16PM -0700, Linus Torvalds wrote:
>
> On Mon, 6 Oct 2008, Johannes Schindelin wrote:
> >
> > It is not a bug.
>
> If it's unwanted functionality, that's a bug. That's kind of the whole
> (and only) difference between "bug" and "feature". Do people want it?
It looks like you have changed your opinion since what you wrote half
a year ago.
Back then you said:
> Yeah. Why not just rather than the whole ok/cancel discussion, go with a
> single button saying "good for me!" and be done with it.
>
> IOW, the license thing should be considered *informational* rather than a
> choice. Because to a user, that's exactly what the GPL is.
http://article.gmane.org/gmane.comp.version-control.msysgit/1677
Also, please, re-read what Junio said in this thread:
"I think removing the license dialog is a bad idea. You need to
tell the end-user about his rights, and one of the things is
that he can get source to git under the terms of GPLv2. The bug
is not about showing the license, but is about refusing to
install unless the end user agrees with it."
I think such a step as removing the license should be approved by
Junio and not be taken behind his back as it's happening now.
Dmitry
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Dmitry Potapov @ 2008-10-07 1:59 UTC (permalink / raw)
To: Heikki Orsila
Cc: Johannes Schindelin, Linus Torvalds, Jakub Narebski, Petr Baudis,
msysgit, git
In-Reply-To: <20081006201125.GI16289@zakalwe.fi>
On Mon, Oct 06, 2008 at 11:11:25PM +0300, Heikki Orsila wrote:
>
> I disagree VIOLENTLY with you. I've been utterly struck with this
> Windows crap here.. I spent a day installing stupid trivial software
> and answering pointless EULAs. I REALLY REALLY hate extra questions..
Git installer should NOT ask you whether you agree with the license,
but it merely shows you the license. (At least, it is what was decided
half a year ago).
> This is my 20th reboot..
I am pretty sure that 20th reboot has nothing to do with how many times
the license has been shown...
Dmitry
^ permalink raw reply
* RFH: gitk equivalent of "git log -p --full-diff file.cpp"
From: Eric Raible @ 2008-10-07 2:04 UTC (permalink / raw)
To: Git Mailing List; +Cc: paulus
Hello All -
Hopefully I'm just being dense, but there's a useful tortoisesvn feature that
(so far) I haven't found in gitk. Blasphemy! Sniveling windoze luser, etc^2.
Anyway what I'm looking for is the gitk equivalent of:
git log -p --full-diff file.cpp
That is to say: I want to see all commits which touch a given
file along with all of the other files changed by that commit.
gitk -p --full-diff EntryPoints.cpp
doesn't quite do it - it treats the "extra" diffs as part of the commit msg.
Anyone got more if a clue than me?
Thanks - Eric
PS - Aside from aliases, am I missing a simpler spelling of the above
git log command?
^ permalink raw reply
* Re: Files with colons under Cygwin
From: Joshua Juran @ 2008-10-07 2:05 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <20081004233945.GM21650@dpotapov.dyndns.org>
On Oct 4, 2008, at 4:39 PM, Dmitry Potapov wrote:
> On Thu, Oct 02, 2008 at 04:02:23PM +0200, Giovanni Funchal wrote:
>>
>> Cygwin does not allow files with colons, I think this is Windows
>> stuff
>> one just can't avoid.
>
> I wonder if the problem exists on Mac OS X too. From what I heard, it
> does not treat ':' as a normal symbol.
The short answer is that Mac OS X's POSIX implementation works as
expected (internally replacing ':' with '/') without issue.
Furthermore, my POSIX-like environment Lamp (Lamp ain't Mac POSIX)
that runs on classic Mac OS (much in the manner of Cygwin) provides
the same Unix filing behavior, so Mac filing syntax isn't an issue in
running git on Mac OS 9. (I'm sure everybody's breathing a HUGE sigh
of relief at this news...)
Josh
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Linus Torvalds @ 2008-10-07 2:10 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Johannes Schindelin, Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <20081007015336.GU21650@dpotapov.dyndns.org>
On Tue, 7 Oct 2008, Dmitry Potapov wrote:
>
> It looks like you have changed your opinion since what you wrote half
> a year ago.
Back then, nobody had really complained and sent in a patch to make it
optional.
That changes things. Once some user actually complains, and sends in a
fix to make the whole dialog optional, I don't see why anybody would ever
argue against such a patch being accepted.
Linus
^ permalink raw reply
* Re: [PATCH RFC] rebase--interactive: if preserving merges, use first-parent to limit what is shown.
From: Stephen Haberman @ 2008-10-07 2:20 UTC (permalink / raw)
To: Avi Kivity; +Cc: git
In-Reply-To: <20081006102118.3e817a0f.stephen@exigencecorp.com>
> I agree with Avi on what the rebase -i -p behavior should be for his
> scenario. This patch makes it so. However, the bane of my existence,
> t3404 is failing ~12 tests in, which is a real PITA to debug, so
> please let me know if this is a worthwhile tangent to continue on.
Ah, good old t3404--it caught me on an aspect I had considered but
wanted to avoid--Avi's (and my) preferred "--first-parent" way of
listing merges works great if the right hand side of the merge commits
are outside of the branch getting rebased.
E.g. my use case is when I merge in a stable release with ~100 commits
or so and could potentially want to move it around, perhaps squash
around it as Avi pointed it, I don't want all 100 commits that are in
the stable branch to be listed in my todo file.
However, t3404 makes a good point that if the right hand of the merge
has parents that are going to get rebased, the right hand side does
need to be included/shown/rewritten.
I also went and looked at the git-sequencer rewrite of rebase-i and it
looks slick. I don't fully understand it yet, but I'm much more
inclined now to just let Stephan (& sponors/list) ably handle the
problem. Especially since its moving to builtin, it moves the required
technical ability to contribute above my current skillset--perhaps that
is the intent. :-)
So, unless I think of something else, I'm done hacking on this and
am withdrawing the patch.
Though I am curious--with the sequencer, is the Avi/my request of not
listing out-of-band commits in the todo file going to be handled?
Some sort of "--first-parent-unless-included-in-rebase" flag.
Thanks,
Stephen
^ permalink raw reply
* Re: RFH: gitk equivalent of "git log -p --full-diff file.cpp"
From: Jeff King @ 2008-10-07 2:27 UTC (permalink / raw)
To: Eric Raible; +Cc: Git Mailing List, paulus
In-Reply-To: <279b37b20810061904n4f50f650l81590cb6fd239282@mail.gmail.com>
On Mon, Oct 06, 2008 at 07:04:14PM -0700, Eric Raible wrote:
> Anyway what I'm looking for is the gitk equivalent of:
>
> git log -p --full-diff file.cpp
Try turning off the option "Limit diffs to listed paths" in the
preferences menu (or adding "set limitdiffs 0" to your ~/.gitk).
> That is to say: I want to see all commits which touch a given
> file along with all of the other files changed by that commit.
>
> gitk -p --full-diff EntryPoints.cpp
You definitely don't want the "-p" here, which is what is causing the
extra diffs as part of the commit message. What happens is that gitk
actually gets the diff separately from the history (since it gets each
diff on the fly as you highlight it). The command line arguments go
towards getting the history; so gitk isn't expecting parse any diff (and
you will make gitk startup a _lot_ slower as it waits for git to
generate the diffs for every revision).
The "--full-diff" is simply ignored, since the actual diff happens
during a later command. In theory, gitk could remember your --full-diff
and use it later. But I think for your use, the preference setting
is probably good enough.
> PS - Aside from aliases, am I missing a simpler spelling of the above
> git log command?
Nope, it looks right to me. At one point, we discussed a "full-diff"
config option (and I think I maybe even posted a patch), but nobody
really cared.
-Peff
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Avery Pennarun @ 2008-10-07 2:40 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Heikki Orsila, Johannes Schindelin, Linus Torvalds,
Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <20081007015942.GV21650@dpotapov.dyndns.org>
On Mon, Oct 6, 2008 at 9:59 PM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Mon, Oct 06, 2008 at 11:11:25PM +0300, Heikki Orsila wrote:
>>
>> I disagree VIOLENTLY with you. I've been utterly struck with this
>> Windows crap here.. I spent a day installing stupid trivial software
>> and answering pointless EULAs. I REALLY REALLY hate extra questions..
>
> Git installer should NOT ask you whether you agree with the license,
> but it merely shows you the license. (At least, it is what was decided
> half a year ago).
FWIW, the only reason *installers* ever show licenses is because they
want to give you *additional* restrictions on top of normal copyright,
and making you "click through" the license before using the
application might (or might not) make those restrictions more legally
enforceable than simply including that license along with the program.
Installers don't show you the license to increase visibility; they do
it for legal reasons.
The most visible place to put a license is in either a splash screen
or a Help|About dialog. If I'm using a program and I want to know the
license, that's where I look.
This whole discussion is silly because end users, who are the ones we
claim to be worried about, will not be the ones running the installer
anyway at a large company. It'll be the IT department guy, who
already knows the license and will have to run the installer 500
times. And if it's not a large company, individuals will see the
license when they go to download the software.
Have fun,
Avery
^ permalink raw reply
* Re: [msysGit] [FYI][PATCH] Customizing the WinGit installer
From: Dmitry Potapov @ 2008-10-07 3:08 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Schindelin, Jakub Narebski, Petr Baudis, msysgit, git
In-Reply-To: <alpine.LFD.2.00.0810061909260.3208@nehalem.linux-foundation.org>
On Mon, Oct 06, 2008 at 07:10:37PM -0700, Linus Torvalds wrote:
>
> That changes things. Once some user actually complains, and sends in a
> fix to make the whole dialog optional, I don't see why anybody would ever
> argue against such a patch being accepted.
First, he did not complain. He did not even mention that in the commit
message. He mentioning some other things like removing release notes,
but not the license. Second, I would expect that any change that goes
against the previous achieved agreement may deserve some discussion,
and not blindly accepted just because the user sent a patch. Okay, I
don't really care whether the installer shows the license or not...
Perhaps, something like Help|License would be a better place for it.
Dmitry
^ 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