Re: [PATCH 08/17] revision: split some overly-long lines
On 05/21/2013 07:34 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- revision.c | 20 ++-- revision.h | 32 +--- 2 files changed, 35 insertions(+), 17 deletions(-) Looks obviously good for *.c file, but I am on the fence for *.h one, as the reason we kept these long single lines in *.h files was to help those who want to grep in *.h files to let them view the full function signature. It probably is OK to tell them to use git grep -A$n instead, though. My goal with this patch was not to set a new policy but rather just to make the code conform a little better to the existing policy as described in CodingGuidelines. *If* it is preferred that header files list all parameters on a single line, then by all means adjust the CodingGuidelines and I will just as happily make header files conform to *that* policy when I touch them :-) (That being said, my personal preference is to apply the 80-character limit for header files too.) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/17] get_revision_internal(): make check less mysterious
On 05/21/2013 07:38 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: The condition under which gc_boundary() is called was previously if (alloc = nr) . But by construction, nr can never exceed alloc, so the check looks unnecessarily mysterious. In fact, the purpose of the check is to try to avoid a realloc() call by shrinking the array if possible if it is at its allocation limit when a new element is about to be added. So change the check to if (nr == alloc) and add a comment to explain what's going on. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Please check that I have properly described the purpose of this check. The way the code is written, it looks like a bad pattern of growth and shrinkage of the array (namely, just under the resize limit) could cause gc_boundary() to be called over and over again with (most of) the same data. I hope that the author had some reason to believe that such a pattern is unlikely. That is about comparing with alloc, not having high and low watermarks, right? I do not see alloc = nr is mysterious at all; it is merely being defensive. If nr would ever exceed alloc, then the code would be broken and would probably have already performed an illegal memory access. Pretending to support nr alloc here is not defensive but misleading, because by that time the ship is going down anyway. On a more practical level, when I saw this code I thought to myself that's strange, I'd better look into it because it suggests that I don't understand the meaning of nr and alloc. I think that the suggested change will help prevent the next reader from repeating the same pointless investigation. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/17] gc_boundary(): move the check alloc = nr to caller
On 05/21/2013 07:49 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
There is no logical reason for this test to be here. At the caller we
might be able to figure out its meaning.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
I do not think this change is justified, *unless* the caller later
in the series gains a better heuristics than what can be done with
information in the object_array (namely, alloc and nr) to decide
when to trigger gc.
And I was hoping to see such a cleverness added to the caller, but I
do not think I saw any.
Correct.
I would have to say gc_boundary() knows better when it needs to gc
with the code at this point in the series, and that is true also in
the final code after all the patches in this series.
If we keep the when to gc logic inside gc, in 11/17 this caller
can no longer call directly to object_array_filter(). It should
call gc_boundary(), but I see it as a merit, not a downside. The
gc function can later be taught the high/low watermark logic you
alluded to in 10/17, and the growth/shrinkage characteristic you
would take advantage of while doing gc is specific to this
codepath. And the logic still does not have to have access to
anything only the caller has access to; gc can work on what can be
read from the object_array-{alloc,nr} that is given to it.
I don't feel strongly about this patch and if you prefer to have it
dropped I will do so. But let me explain my reasoning:
1. The function name gc_boundary() suggests that it will do a garbage
collection unconditionally. In fact, it might or might not depending on
this test. At the caller, this made it look like a gc was happening
each time through the loop, which made me nervous about the performance.
The new version makes it clear at the caller that the gc is only
happening occasionally.
2. Even assuming that gc_boundary() were renamed to maybe_gc_boundary(),
the function has hopelessly little information on which to base its
decision whether or not to gc, and the choice of policy can only be
justified based on some implicit knowledge about how the array is grown
and shrunk. But the growing/shrinking is done at the layer of the
caller, and therefore the choice of gc policy also belongs at the layer
of the caller.
3. As you point out, if the gc policy is ever to be made more
intelligent, then gc_boundary() is unlikely to have enough information
to implement the new policy (e.g., it would have no place to record
low/high water marks). Separating concerns at the correct level would
make a change like that easier.
At the moment I am not interested in improving the gc policy of this
code. The only reason that I am mucking about here is to change it to
use object_array_filter(), which is needed to centralize where
object_array_entries are created and destroyed so that the name memory
can be copied and freed consistently. That can be done with or without
patches 09 and 10. Please let me know what you prefer.
Michael
revision.c | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/revision.c b/revision.c
index 8ac88d6..2e0992b 100644
--- a/revision.c
+++ b/revision.c
@@ -2437,23 +2437,19 @@ static struct commit *get_revision_1(struct rev_info
*revs)
static void gc_boundary(struct object_array *array)
{
-unsigned nr = array-nr;
-unsigned alloc = array-alloc;
+unsigned nr = array-nr, i, j;
struct object_array_entry *objects = array-objects;
-if (alloc = nr) {
-unsigned i, j;
-for (i = j = 0; i nr; i++) {
-if (objects[i].item-flags SHOWN)
-continue;
-if (i != j)
-objects[j] = objects[i];
-j++;
-}
-for (i = j; i nr; i++)
-objects[i].item = NULL;
-array-nr = j;
+for (i = j = 0; i nr; i++) {
+if (objects[i].item-flags SHOWN)
+continue;
+if (i != j)
+objects[j] = objects[i];
+j++;
}
+for (i = j; i nr; i++)
+objects[i].item = NULL;
+array-nr = j;
}
static void create_boundary_commit_list(struct rev_info *revs)
@@ -2577,7 +2573,8 @@ static struct commit *get_revision_internal(struct
rev_info *revs)
if (p-flags (CHILD_SHOWN | SHOWN))
continue;
p-flags |= CHILD_SHOWN;
-gc_boundary(revs-boundary_commits);
+if (revs-boundary_commits.alloc = revs-boundary_commits.nr)
+gc_boundary(revs-boundary_commits);
add_object_array(p, NULL, revs-boundary_commits);
}
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to
Re: [PATCH 04/17] builtin_diff_tree(): make it obvious that function wants two entries
On 05/21/2013 07:27 PM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
Instead of accepting an array and using exactly two elements from the
array, take two single (struct object_array_entry *) arguments.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/diff.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index 8c2af6c..ba68c6c 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -153,7 +153,8 @@ static int builtin_diff_index(struct rev_info *revs,
static int builtin_diff_tree(struct rev_info *revs,
int argc, const char **argv,
- struct object_array_entry *ent)
+ struct object_array_entry *ent0,
+ struct object_array_entry *ent1)
{
const unsigned char *(sha1[2]);
int swap = 0;
@@ -161,13 +162,13 @@ static int builtin_diff_tree(struct rev_info *revs,
if (argc 1)
usage(builtin_diff_usage);
-/* We saw two trees, ent[0] and ent[1].
- * if ent[1] is uninteresting, they are swapped
+/* We saw two trees, ent0 and ent1.
+ * if ent1 is uninteresting, they are swapped
*/
While you are touching this comment is a perfect time to fix the
existing style violation.
Yes. Will fix in next version.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/17] cmd_diff(): use an object_array for holding trees
On 05/21/2013 07:30 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Change cmd_diff() to use a (struct object_array) for holding the trees that it accumulates, rather than rolling its own equivalent. A significant detail missing here is that this lifts the hardcoded 100 tree limit in combined diff but that does not matter in practice, I would suppose ;-). I'll note it anyway in v2 of the patch series. Thanks for all of your comments. I will send a re-roll after I hear back from you regarding patches 08, 09, and 10. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current
Junio C Hamano wrote: I have largedir I want to get rid of, but there is a directory I want to save, largedir/precious, in it, so I do cp -R largedir/precious precious and then run 'rm -rf largedir' in another terminal in parallel. I would argue that there is something to fix, but that fix involves making the cp a purely atomic operation which is super-complicated, and totally not worth it. Would you _not_ like the above example to work? Then how can you say that there's nothing to be fixed? Consider a slightly different example: I rename a file while having an active file handle open in a process that's reading the file. Will the rename fail or will the fread() in the process fail? Nope, both work fine. Replace rename with remove, and we still have the same answer. Ofcourse there are no guarantees: I can start up another process to overwrite the sectors corresponding to that file's data with zeros; unless the complete file is there in the kernel buffer, a read() will eventually end up slurping in the zeros (or fail?), right? It's just that it works in practice. Yet another example: I have a terminal pointing to a directory, and I remove that directory in another terminal. When I switch back to the original terminal, I can't cd .., because getcwd() fails. This has annoyed me endlessly in practice, and I would really like to fix this if I can. Don't accept the way things are, and assume that there's nothing to be fixed. In my opinion, if something about a piece of software annoys you, there is always something to fix. It just depends on what _can_ be fixed in a reasonable amount of time with a good engineering solution. There's no need to go to the other extreme: I'm not interested in rewriting the whole operating system in Haskell and providing theoretical guarantees for everything. Coming back to our push example, I don't see why you think HEAD is special: I could even say git push master and expect it to race with an update-ref. But nobody is complaining about that: if someone does complain, I would seriously consider copying master to PUSH_HEAD early (and push that). With HEAD, however, someone is complaining (namely, me): pushing usually means that I've finished working on that branch, and want to switch to another branch and continue working. Why should I have to wait for the push to complete? I've hit this bug several times (from terminal as well as Magit), and this patch fixes the problem for me in practice. That said, I agree that my patch does not guarantee anything (and I will modify my commit message to clarify this). I'm just expressing my opinion on the issue of fixing problems. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Geolocation support
Alessandro Di Marco wrote: this is a hack I made a couple of years ago in order to store my current location in git commits (I travel a lot and being able to associate a place with the commit date helps me to quickly recover what were doing at that time). Long story short, the screeenshot at http://tinypic.com/r/wars40/5 shows the new gitk interface once this patch has been integrated. Geolocation is controlled by two envvars GIT_AUTHOR_PLACE and COMMITTER_PLACE, respectively. You can set them via something like this: Obviously very interesting. Now, how do we mainline (parts of) this feature? I'll raise some questions here: 0. We already have timezone information, but this is obviously insufficient for any sensible geolocation data. 1. Does it make sense to make it an optional field in the commit object? I can see how generic optional fields in the commit object can be useful: a lot of code-review systems put the code-review ID in the commit message, and I can see how an optional field would benefit them. Will it break existing parsers (shouldn't they ignore unknown fields)? 2. How accurate should this geolocation information for it to be invariant enough? If we blindly store what a GPS gives us, the centering error is obviously a problem. What should be the resolution of the lat/long that we store? 3. Failing (2), can we put the geolocation data in the commit message, and proceed? If so, does it need to be part of git-core, or should an external client (gitk, or other clients) write/ parse the geolocation information? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current
On Thu, 23 May 2013 13:25:55 +, Ramkumar Ramachandra wrote: Junio C Hamano wrote: I have largedir I want to get rid of, but there is a directory I want to save, largedir/precious, in it, so I do cp -R largedir/precious precious and then run 'rm -rf largedir' in another terminal in parallel. 'mv largedir/precious precious; rm -rf largedir'? No race here. ... Consider a slightly different example: I rename a file while having an active file handle open in a process that's reading the file. Will the rename fail or will the fread() in the process fail? Nope, both work fine. Replace rename with remove, and we still have the same answer. Ofcourse there are no guarantees: I can start up another process to overwrite the sectors corresponding to that file's data with zeros; unless the complete file is there in the kernel buffer, a read() will eventually end up slurping in the zeros (or fail?), right? Oh, there are guarantees, they just don't include the case where you take a shotgun to the disk. (Or do it on an nfs mount and delete the file from another machine.) ... Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Wed, 22 May 2013 11:07:07 +, Junio C Hamano wrote: ... If you have a four-commit segment in your commit ancestry graph I never had yet. :-( (time flows from left to right; turn your head 90-degrees to the right if you want a gitk representation): ---A--X \/ /\ ---B--Y where X and Y are both merges between A and B, having A as their first parent, how would you express such a graph with first-parent chain going a straight line? Of course there are multiple possible straight lines and how it looks depends on the order I use the existing heads to fish them out. (That is, when the straight lines join, I need to bend one of them.) Assuming I take the one where X is on, I expect a look like -A---X- \ | +- Y | | -B+--+ Branch heads that are reachable from other head are picked after those that aren't reachable. The point is to get the feature branches being displayed on separate lanes (and thus visibly sticking out) and not being intermingled with the longer-living branches. ... Don't do that, then. :-) Problem is, in this case 'I' expands to about 17 people I need to educate on this. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 5:06 AM, Andreas Krey a.k...@gmx.de wrote: [snip] ... Don't do that, then. :-) Problem is, in this case 'I' expands to about 17 people I need to educate on this. This is a feature of `git pull` that I really despise. I really wish `git pull` treated the remote as the first parent in its merge operation. -John -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Geolocation support
On Thu, May 23, 2013 at 10:45 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Alessandro Di Marco wrote: this is a hack I made a couple of years ago in order to store my current location in git commits (I travel a lot and being able to associate a place with the commit date helps me to quickly recover what were doing at that time). Long story short, the screeenshot at http://tinypic.com/r/wars40/5 shows the new gitk interface once this patch has been integrated. Geolocation is controlled by two envvars GIT_AUTHOR_PLACE and COMMITTER_PLACE, respectively. You can set them via something like this: Obviously very interesting. Now, how do we mainline (parts of) this feature? I'll raise some questions here: I'm really not convinced this kind of changes should make it into Junio's tree (of course, he's the only one to decide). I really believe this is a very specific solution to a very specific problem (that is not for me to judge if the problem is real). Bloating the commit object with this kind of information doesn't feel like a good idea. I think it could be nice to provide a simple shell script to build the location, callable from a post-commit hook, to construct a geolocation note. Gitk could be programmed to read the notes to get the location, but once again, I'm not sure it should be mainlined. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added guilt.reusebranch configuration option.
Theodore Ts'o wrote:
Right now I do this just by being careful, but if there was an
automatic safety mechanism, it would save me a bit of work, since
otherwise I might not catch my mistake until I do the git push
publish, at which point I curse and then start consulting the reflog
to back the state of my tree out, and then reapplying the work I had
to the right tree.
My scenario is a bit different, and I think this safety feature is
highly overrated. It's not that I'll never rewind some branches, but
rewind other branches, but rather I might rewind anything at any
time, but I want immediate information so I can quickly inspect @{1}
to see if that was undesirable. To put it another way, my philosophy
is not auto-deny unintended changes, but rather tell me immediately
about undesirable changes. To this effect, my prompt looks like:
artagnon|push-current-head=:~/src/git$
The = indicates that I'm in sync with upstream, and that there's
nothing to push. When I make some changes, that character changes to
, which means that there are ff changes to push. Finally,
artagnon|push-current-head:~/src/git$
has my immediate attention. means that I've diverged from
upstream. Since the prompt is present all the time, I catch the
divergence just-in-time. Moreover, I push very frequently resetting
the prompt to = periodically.
So, do you still need this rewinding safety thing?
So what I do is something like this:
git push publish ; git push repo ; git push code
While we can definitely make the UI better for this (maybe push
--multiple?), there is no fundamental change: we have to re-initialize
all the refspecs, connect to the remote via the transport layer and
prepare a packfile to send. In other words, it's impossible to make
it any faster than what you get with the above.
where
[remote publish]
url = ssh://gitol...@ra.kernel.org/pub/scm/fs/ext2/e2fsprogs.git
fetch = +refs/heads/*:refs/heads/*
push = next
push = master
push = maint
push = debian
push = +pu
So you're a batched-push person. And the above makes it clear that
you don't want to explicitly differentiate between a push and push -f
(the +pu thing). And this assumes that you never create any new
branches (I branch out all the time), otherwise you'd have rules for
refs/heads/*. Just out of curiosity, do you ever have ref-renaming
requirements (like push = refs/heads/*:refs/heads/tt/*)? We were
discussing that on another thread, but I haven't found an
implementation I'm happy with yet.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #05; Mon, 20)
Junio C Hamano venit, vidit, dixit 22.05.2013 18:36: Michael J Gruber g...@drmicha.warpmail.net writes: * mg/more-textconv (2013-05-10) 7 commits - grep: honor --textconv for the case rev:path - grep: allow to use textconv filters - t7008: demonstrate behavior of grep with textconv - cat-file: do not die on --textconv without textconv filters - show: honor --textconv for blobs - diff_opt: track whether flags have been set explicitly - t4030: demonstrate behavior of show with textconv I think this is ready for 'next'; not that it matters during the prerelease feature freeze. Oh, I'm sorry, I thought we were still in discussions about the default mechanism (config or attributes) and the implementation (tacking context onto each object)? Therefore, I didn't hurry to polish and follow up over my vacation. I'm not sure I had smoothed out all minor things (honor/obey and such) when the object struct size issue came up. I'll check today or tomorrow. (Freeze, yes, but we don't want too many next rewrites, and one is coming soon...) I thought this was fine as-is, but we can kick it back to 'pu' and replace it with a reroll after 1.8.3 if that is necessary. Didn't you have concerns about storing the context in the object struct? I can't quite judge how much of an issue this can be for fsck and such. I don't want to increase the memory footprint unnecessarily, of course. Other than that, the mechanism was still up for discussion (separate show attribute or a config) given that the default behavior for showing blobs is not to change. Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug when git rev-list options --first-parent and --ancestry-path are used together?
It seems to me that git rev-list --first-parent --ancestry-path A..B is well-defined and should list the commits in the intersection between git rev-list --first-parent A..B and git rev-list--ancestry-path A..B But in many cases the first command doesn't provide any output even though there are commits common to the output of the last two commands. For example, take as an example the DAG from test t6019: # D---E---F # / \ \ #B---C---G---H---I---J # / \ # A---K---L--M (The merges are always downwards; e.g., the first parent of commit L is K.) The command git rev-list --first-parent --ancestry-path D..J doesn't generate any output, whereas I would expect it to output H I J. Similarly, git rev-list --first-parent --ancestry-path D..M doesn't generate any output, whereas I would expect it to output L M. For fun, the attached script computes the output for all commit pairs in this DAG and outputs the discrepancies that it finds. (It should be run in directory t/trash directory.t6019-rev-list-ancestry-path after t6019 was run with -d.) Is this a bug or are my expectations wrong? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ x.sh Description: Bourne shell script
Re: first parent, commit graph layout, and pull merge direction
- Mail original - On Thu, May 23, 2013 at 5:06 AM, Andreas Krey a.k...@gmx.de wrote: [snip] ... Don't do that, then. :-) Problem is, in this case 'I' expands to about 17 people I need to educate on this. This is a feature of `git pull` that I really despise. I really wish `git pull` treated the remote as the first parent in its merge operation. seconded... github's network pages (which display the commit graph of projects) seems to follow the first parent at the top rule and the pull merges are standing out as wrong because of that... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, 23 May 2013 05:48:38 +, John Szakmeister wrote: ... This is a feature of `git pull` that I really despise. I really wish `git pull` treated the remote as the first parent in its merge operation. I'd actually only like it that way when pulling from the tracking branch, not for any pull. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Random thoughts on upstream
Junio C Hamano wrote: And that was done with extensivility your example implied in mind: you may later be allowed to push other branches as well to origin. That is why the refspec definition for 'origin' does not hardcode the name of the branch you are permitted to push there at this moment. The fact that hot-branch goes to origin is encapsulated in the branch.hot-branch.pushremote. The rule, under which the name of any branch that goes to the origin is renamed, is encapsulated in remote.origin.push refspec (the introduction of the new mode push.default = single is necessary to make this work). My problem with this entire scheme _is_ the magic push.default = single. Currently, push.default only kicks in when no remote.name.push refspec is specified (in other words, it is a default value of remote.name.push for all remotes), and I don't think we should change this. If the user wants to configure the push refspec (either for rewriting, or for determining what to push), there is exactly one thing to change: remote.name.push. So I can have: [remote theodore] push = master push = +pu This means that I will always push master without force and pu with force, irrespective of the branch I'm on. [remote origin] push = refs/heads/*:refs/heads/rr/* This means that I will always push all branches without force with rewriting, irrespective of the branch I'm on. [remote ram] push = HEAD:refs/heads/rr/%1 This means that I will always push the current branch without force, with rewriting. [remote felipe] # empty This means that remote.felipe.push falls back to the refspec specified by push.default. Isn't branch.name.push is completely unnecessary? Does this make sense to you? Isn't it more straightforward and general (how do I get a push.default = single on a per-remote basis) than your solution? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
push not resolving commit-ish?
Looks like push can't resolve tags to commits. Why is that? linux$ git push -f $PWD v3.10-rc2:refs/heads/vhost-next error: Trying to write non-commit object a8c6d53c4d84b3a5eb182758a9cdceceba4691da to branch refs/heads/vhost-next To /scm/linux ! [remote rejected] v3.10-rc2 - vhost-next (failed to write) error: failed to push some refs to '/scm/linux' linux$ git log v3.10-rc2|head -5 commit c7788792a5e7b0d5d7f96d0766b4cb6112d47d75 Author: Linus Torvalds torva...@linux-foundation.org Date: Mon May 20 14:37:38 2013 -0700 Linux 3.10-rc2 linux$ $ git push -f $PWD c7788792a5e7b0d5d7f96d0766b4cb6112d47d75:refs/heads/vhost-next Everything up-to-date -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Random thoughts on upstream
On Thu, May 23, 2013 at 5:42 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
Junio C Hamano wrote:
And that was done with extensivility your example implied in mind:
you may later be allowed to push other branches as well to origin.
That is why the refspec definition for 'origin' does not hardcode
the name of the branch you are permitted to push there at this
moment. The fact that hot-branch goes to origin is encapsulated in
the branch.hot-branch.pushremote. The rule, under which the name of
any branch that goes to the origin is renamed, is encapsulated in
remote.origin.push refspec (the introduction of the new mode
push.default = single is necessary to make this work).
My problem with this entire scheme _is_ the magic push.default =
single. Currently, push.default only kicks in when no
remote.name.push refspec is specified (in other words, it is a
default value of remote.name.push for all remotes), and I don't
think we should change this. If the user wants to configure the push
refspec (either for rewriting, or for determining what to push), there
is exactly one thing to change: remote.name.push. So I can have:
[remote theodore]
push = master
push = +pu
This means that I will always push master without force and pu with
force, irrespective of the branch I'm on.
[remote origin]
push = refs/heads/*:refs/heads/rr/*
This means that I will always push all branches without force with
rewriting, irrespective of the branch I'm on.
[remote ram]
push = HEAD:refs/heads/rr/%1
This means that I will always push the current branch without force,
with rewriting.
[remote felipe]
# empty
This means that remote.felipe.push falls back to the refspec
specified by push.default.
Isn't branch.name.push is completely unnecessary?
No, it's not; 'git push --set-downstream' is not going to configure
that for the user. Nor will the user be able to see the downstream
with 'git branch -vv', nor will the user be able to see the downstream
with 'git rev-parse branch@{downstream}'.
You keep ignoring those use-cases I already mentioned multiple times
because they don't fit your idealistic model.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
On Thu, May 23, 2013 at 01:53:10PM +0300, Michael S. Tsirkin wrote: Looks like push can't resolve tags to commits. Why is that? linux$ git push -f $PWD v3.10-rc2:refs/heads/vhost-next error: Trying to write non-commit object a8c6d53c4d84b3a5eb182758a9cdceceba4691da to branch refs/heads/vhost-next To /scm/linux ! [remote rejected] v3.10-rc2 - vhost-next (failed to write) error: failed to push some refs to '/scm/linux' linux$ git log v3.10-rc2|head -5 commit c7788792a5e7b0d5d7f96d0766b4cb6112d47d75 Author: Linus Torvalds torva...@linux-foundation.org Date: Mon May 20 14:37:38 2013 -0700 Linux 3.10-rc2 linux$ $ git push -f $PWD c7788792a5e7b0d5d7f96d0766b4cb6112d47d75:refs/heads/vhost-next Everything up-to-date Forgot to say - this is recent git.git master: 1.8.3.rc3.8.g5e49f30 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
On Thu, May 23, 2013 at 5:53 AM, Michael S. Tsirkin m...@redhat.com wrote:
Looks like push can't resolve tags to commits.
Why is that?
linux$ git push -f $PWD v3.10-rc2:refs/heads/vhost-next
Perhaps v3.10-rc2^{}. Yeah, totally and completely not-user-friendly,
I already complained about it to no avail.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] sha1_name: fix error message for @{u}
Junio C Hamano wrote:
If you have to ask why, and cannot answer the question yourself,
then you would not know if such a caller exists. After a code
audit, I know there is no such caller that appends @{u} but if you
were writing a quick-and-dirty caller, I would not be surprised if
you find it more convenient to form a textual extended SHA-1
expression and have get_sha1() do its work, instead of asking the
same question programmatically.
I'll mention that a simple grep for @{u} and @{upstream} found nothing.
In this case, I think you already checked there is no such problem,
and it is a more straight-forward justification to say that you did
a code-audit and made sure that all the callers that used to hit one
of these errors() want to die().
It's not about callers eventually wanting to die(); it's about whether
callers want to die() without doing anything useful after getting this
-1. And that is impossible for me to say with confidence, unless I do
a _very_ extensive code review (which I didn't do).
Also such a caller, if existed, would either
(1) want to die itself, in which case these error() messages are
superfluous; or
(2) want to continue (possibly dying with its own message), in
which case these error() messages are unwanted.
Because you are changing only the existing call sites of error()
into die(), and not changing silent -1 returns to die(), this change
is safe for both kinds of such callers, I think.
Take the example of git branch (-vv) output: let's imagine a universe
in which it determines upstream by calling in with a hard-coded @{u}
string. Should the entire program die() and stop printing the rest of
the branches? Ofcourse not. Is your argument that no caller should
do this in the first place, because of spurious error() messages
polluting the output (of git branch)? How is this argument stronger
than my grep for @{u}?
+ die(
_(Upstream branch '%s' not stored as a
remote-tracking branch),
upstream-merge[0]-src);
OK, but I would fix the indentation while at it if I were doing this change.
But my Emacs reports that the indentation is correct. Did you mean:
diff --git a/sha1_name.c b/sha1_name.c
index b7e008a..b00ea0f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1051,8 +1051,7 @@ int interpret_branch_name(const char *name,
struct strbuf *buf)
die(_(No upstream configured for branch '%s'),
upstream-name);
}
- die(
- _(Upstream branch '%s' not stored as a remote-tracking
branch),
+ die(_(Upstream branch '%s' not stored as a remote-tracking
branch),
upstream-merge[0]-src);
}
free(cp);
Yeah, I'll do this.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 12:29:59PM +0200, Andreas Krey wrote: On Thu, 23 May 2013 05:48:38 +, John Szakmeister wrote: ... This is a feature of `git pull` that I really despise. I really wish `git pull` treated the remote as the first parent in its merge operation. I'd actually only like it that way when pulling from the tracking branch, not for any pull. I'll add my voice to the annoyed by this pile ;-) I've been annoyed by this at $DAYJOB recently. A lot of people seem to blindly git pull without much thought about how the history is ending up and what they actually want to do. I wonder if it would make sense for git pull (with no arguments) to pass --ff-only to git-merge, allowing this to be overridden with --rebase and --merge (which doesn't currently exist). With some suitable advice output we could hopefully educate users about how to shape their history. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Wed, May 22, 2013 at 6:50 AM, Andreas Krey a.k...@gmx.de wrote: Hi everyone, I'm just looking into better displays of the commit graph (as displayed with gitk, smartgit, fisheye) - they tend to quickly dissolve into a heap of spaghetti. We had the idea that treating the first parent specially would have some advantage here - including graphically indicating which one of the parents of a commit is the first parent. (For instance, by letting that line leave the commit node at the top/bottom, and the other(s) to the side.) I don't understand; gitk already shows the first parent starting from the bottom, and the merge commits arrive from the right side. What am I missing? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Geolocation support
Antoine Pelisse apeli...@gmail.com writes: On Thu, May 23, 2013 at 10:45 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Alessandro Di Marco wrote: this is a hack I made a couple of years ago in order to store my current location in git commits (I travel a lot and being able to associate a place with the commit date helps me to quickly recover what were doing at that time). Long story short, the screeenshot at http://tinypic.com/r/wars40/5 shows the new gitk interface once this patch has been integrated. Geolocation is controlled by two envvars GIT_AUTHOR_PLACE and COMMITTER_PLACE, respectively. You can set them via something like this: Obviously very interesting. Now, how do we mainline (parts of) this feature? I'll raise some questions here: I think it could be nice to provide a simple shell script to build the location, callable from a post-commit hook, to construct a geolocation note. Gitk could be programmed to read the notes to get the location, but once again, I'm not sure it should be mainlined. Well, I don't see how the file can be kept synchronized with the tree, but in case it would be also suitable for the author/committer name, email and date :-) Seriously, this is just a hack; the other nice thing coming out from this patch is what I called the Project's Patch Graph (or PPG), ie. a DAG starting from the project founder location and spreading all over the world (depending on the project of course!) IMHO it's an interesting snapshot of how the project is evolving. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, 23 May 2013 06:34:38 +, Felipe Contreras wrote: ... I don't understand; gitk already shows the first parent starting from the bottom, and the merge commits arrive from the right side. What am I missing? That this isn't (consistently) the case in complicated situations. I'll need to make a picture (as in png). Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bug report: git grep seems to use the wrong .gitattributes when invoked in a subdirectory
Greetings, I bumped into a problem where git grep thinks files in repo/a/data are binary files when it is invoked from repo/a and repo/data/.gitattributes contains * binary. I can reproduce this on 1.7.9.5 and 1.7.10.4. Unfortunately I don't have a newer version at hand. How to reproduce: [pseudo:~/tmp]% git --version git version 1.7.10.4 [pseudo:~/tmp]% git init git-test Initialized empty Git repository in /home/opqdonut/tmp/git-test/.git/ [pseudo:~/tmp]% cd git-test [pseudo:~/tmp/git-test:master()]% mkdir -p a/data [pseudo:~/tmp/git-test:master()]% mkdir data [pseudo:~/tmp/git-test:master()]% echo '* binary' data/.gitattributes [pseudo:~/tmp/git-test:master()]% echo foo a/data/foo [pseudo:~/tmp/git-test:master()]% git add -A [pseudo:~/tmp/git-test:master()]% git commit -m foo [master (root-commit) 20fafbb] foo 2 files changed, 1 insertion(+) create mode 100644 a/data/foo create mode 100644 data/.gitattributes [pseudo:~/tmp/git-test:master()]% git grep foo a/data/foo:foo [pseudo:~/tmp/git-test:master()]% cd a [pseudo:~/tmp/git-test/a:master()]% git grep foo Binary file data/foo matches (Please CC me on replies, I'm not subscribed to the list.) -- Joel Kaasinen joel.kaasi...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Document push --no-verify
ec9 (push: Add support for pre-push hooks, 2013-01-13) forgot to add a note to git-push(1) about the new --no-verify option. Signed-off-by: Thomas Rast tr...@inf.ethz.ch --- Junio replied privately that it should also mention the --verify possibility. So why not. But this needs to be fixed across the board eventually; 0f1930c (parse-options: allow positivation of options starting, with no-, 2012-02-25) did not update any docs, so none of the other --no- options mention their positive forms. Documentation/git-push.txt | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index d514813..df5be26 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -11,7 +11,7 @@ SYNOPSIS [verse] 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=git-receive-pack] [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream] - [repository [refspec...]] + [--no-verify] [repository [refspec...]] DESCRIPTION --- @@ -195,6 +195,11 @@ useful if you write an alias or script around 'git push'. be pushed. If on-demand was not able to push all necessary revisions it will also be aborted and exit with non-zero status. +--[no-]verify:: + Toggle the pre-push hook (see linkgit:githooks[5]). The + default is \--verify, giving the hook a chance to prevent the + push. With \--no-verify, the hook is bypassed completely. + include::urls-remotes.txt[] -- 1.8.3.rc3.486.gfe16094 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git hangs on pthread_join
Hi, I'm running a rather special configuration, basically i have a gerrit server pushing git data over openvpn connections (company regulations n' stuff)... git 1.8.2.1 is started by xinetd ... port= 9418 socket_type = stream wait= no user= gerrit2 server = /usr/bin/git server_args = daemon --inetd --syslog --export-all --enable=receive-pack --init-timeout=3 --timeout=180 --base-path=path ... nice= 10 per_source = UNLIMITED instances = UNLIMITED flags = KEEPALIVE NODELAY --- Keepalive and nodelay has been added post fact, the same goes for the timeouts. I have found git receive-packs that has been running for days/weeks without terminating Attaching gdb and doing a trace results in: #0 0x003261207b35 in pthread_join () from /lib64/libpthread.so.0 #1 0x004ce58b in finish_async () #2 0x0045744b in cmd_receive_pack () #3 0x00404851 in handle_internal_command () #4 0x00404c9d in main () (sorry don't have any debug data for the binary packages apparenlty (rpms was built from the official source)) (RHEL 5 machine with glibc 2.5-65.el5_7.1) Anyone that has any clues about what could be going wrong? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-send-email: fix handling of special characters
When patch sender's name has special characters,
git send-email did not quote it before matching
against the author name.
As a result it would produce mail like this:
Date: Thu, 23 May 2013 16:36:00 +0300
From: Michael S. Tsirkin m...@redhat.com
To: qemu-de...@nongnu.org
Cc: Michael S. Tsirkin m...@redhat.com
Subject: [PATCH 0/9] virtio: switch to linux headers
Message-Id: 1369316169-20181-1-git-send-email-...@redhat.com
From: Michael S. Tsirkin m...@redhat.com
Fix by sanitizing before matching to patch author name.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
git-send-email.perl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index bd13cc8..c4dc438 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1400,7 +1400,8 @@ foreach my $t (@files) {
$subject = quote_subject($subject, $auto_8bit_encoding);
}
- if (defined $author and $author ne $sender) {
+ my $sanitized_sender = sanitize_address($sender);
+ if (defined $author and $author ne $sanitized_sender) {
$message = From: $author\n\n$message;
if (defined $author_encoding) {
if ($has_content_type) {
--
MST
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-send-email: another fix for special characters
When patch sender's name has special characters,
git send-email did not quote it before matching
against the author name.
As a result suppress_cc = self did not work:
sender is still Cc'd.
Fix by sanitizing before matching to patch author name.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
git-send-email.perl | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index c4dc438..a3fed7c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1309,7 +1309,10 @@ foreach my $t (@files) {
elsif (/^From:\s+(.*)$/i) {
($author, $author_encoding) =
unquote_rfc2047($1);
next if $suppress_cc{'author'};
- next if $suppress_cc{'self'} and $author eq
$sender;
+ if ($suppress_cc{'self'}) {
+my $sanitized_sender =
sanitize_address($sender);
+next if $author eq $sanitized_sender;
+}
printf((mbox) Adding cc: %s from line '%s'\n,
$1, $_) unless $quiet;
push @cc, $1;
--
MST
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Document push --no-verify
On Thu, May 23, 2013 at 03:34:11PM +0200, Thomas Rast wrote: ec9 (push: Add support for pre-push hooks, 2013-01-13) forgot to add a note to git-push(1) about the new --no-verify option. Signed-off-by: Thomas Rast tr...@inf.ethz.ch Thanks. FWIW Reviewed-by: Michael S. Tsirkin m...@redhat.com --- Junio replied privately that it should also mention the --verify possibility. So why not. But this needs to be fixed across the board eventually; 0f1930c (parse-options: allow positivation of options starting, with no-, 2012-02-25) did not update any docs, so none of the other --no- options mention their positive forms. Documentation/git-push.txt | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index d514813..df5be26 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -11,7 +11,7 @@ SYNOPSIS [verse] 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=git-receive-pack] [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream] -[repository [refspec...]] +[--no-verify] [repository [refspec...]] DESCRIPTION --- @@ -195,6 +195,11 @@ useful if you write an alias or script around 'git push'. be pushed. If on-demand was not able to push all necessary revisions it will also be aborted and exit with non-zero status. +--[no-]verify:: + Toggle the pre-push hook (see linkgit:githooks[5]). The + default is \--verify, giving the hook a chance to prevent the + push. With \--no-verify, the hook is bypassed completely. + include::urls-remotes.txt[] -- 1.8.3.rc3.486.gfe16094 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #05; Mon, 20)
Michael J Gruber g...@drmicha.warpmail.net writes: Didn't you have concerns about storing the context in the object struct? I can't quite judge how much of an issue this can be for fsck and such. I don't want to increase the memory footprint unnecessarily, of course. Yes. I thought I had a weather-balloon patch to fsck to use its own so that we have something to fall back on if it turns out to be a problem (and also so that anybody can see how big the difference is), but I highly suspect that any user of object-array other than what you are changing in the series wants to use the slim variant, which suggests that the information does not belong there. Other than that, the mechanism was still up for discussion (separate show attribute or a config) given that the default behavior for showing blobs is not to change. My understanding was that the series as-is (not the implementation but the external interface) makes us honor what the user tells us better, without changing the behaviour for people who don't instruct us to do anything differently, so I thought it was a good place to stop at. The 'show attribute or config' discussion would/should involve the possibility of flipping the default, no? After all, I was getting the impression, especially from the config, that this was If we had known better when we introduced textconv, we would have defined it to apply in any situation where you would want a textual form of a blob, not limited to diff kind of thing. That is a much longer term thing and my impression was that it can built later on top of the series (once its implementation settles). So, yes, thanks for pointing out these two points. The bloat in the object array element I do care today, the feature and the interface I do not think we have to worry about them today to hold the series back. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current
Ramkumar Ramachandra artag...@gmail.com writes: I would argue that there is something to fix, but that fix involves making the cp a purely atomic operation which is super-complicated, and totally not worth it. Would you _not_ like the above example to work? No. I do not think I like the above example to work, at all. I know we live in the real world, and I do not want to see such serializing implementations of cp or rm that try to fix, as it will interfere my everyday use where such a race does not matter. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] Let's get that @{push}!
[7/7] is the meat. Sorry it's in such a messy state: I was having a
field day tracing what push is actually doing. Anyway, I wanted to
send out the series now to get early feedback.
In other news: why on earth is push doing _so_ much processing before
pushing? Is it written very badly, or am I missing something?
Thanks.
(based on rr/die-on-missing-upstream)
Ramkumar Ramachandra (7):
sha1_name: abstract upstream_mark() logic
sha1_name: factor out die_no_upstream()
sha1_name: remove upstream_mark()
remote: expose parse_push_refspec()
remote: expose get_ref_match()
sha1_name: prepare to introduce AT_KIND_PUSH
sha1_name: implement finding @{push}
remote.c| 4 +--
remote.h| 4 +++
sha1_name.c | 111
3 files changed, 88 insertions(+), 31 deletions(-)
--
1.8.3.rc3.17.gd95ec6c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] sha1_name: abstract upstream_mark() logic
Currently, the only non-numerical @{...} expression we support is
@{u[pstream]}. Since we're slowly growing git to support triangular
workflows, it might make sense to have a @{p[ush]} and various other
special @{...} expressions in the future. To prepare for this, abstract
out the upstream_mark() logic to accept any suffix, while preserving the
upstream_mark() interface.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
sha1_name.c | 40 +++-
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 6928cc7..766e4e9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -23,6 +23,8 @@ struct disambiguate_state {
unsigned always_call_fn:1;
};
+#define AT_KIND_UPSTREAM 0
+
static void update_candidates(struct disambiguate_state *ds, const unsigned
char *current)
{
if (ds-always_call_fn) {
@@ -416,20 +418,40 @@ static int ambiguous_path(const char *path, int len)
return slash;
}
-static inline int upstream_mark(const char *string, int len)
+static inline int at_mark(const char *string, int len, int *kind)
{
- const char *suffix[] = { @{upstream}, @{u} };
- int i;
-
- for (i = 0; i ARRAY_SIZE(suffix); i++) {
- int suffix_len = strlen(suffix[i]);
- if (suffix_len = len
-!memcmp(string, suffix[i], suffix_len))
- return suffix_len;
+ int i, j;
+
+ static struct {
+ int kind;
+ const char *suffix[2];
+ } at_form[] = {
+ { AT_KIND_UPSTREAM, { @{upstream}, @{u} } }
+ };
+
+ for (j = 0; j ARRAY_SIZE(at_form); j++) {
+ for (i = 0; i ARRAY_SIZE(at_form[j].suffix); i++) {
+ int suffix_len = strlen(at_form[j].suffix[i]);
+ if (suffix_len = len
+!memcmp(string, at_form[j].suffix[i],
suffix_len)) {
+ if (kind)
+ *kind = at_form[j].kind;
+ return suffix_len;
+ }
+ }
}
return 0;
}
+static inline int upstream_mark(const char *string, int len)
+{
+ int suffix_len, kind;
+ suffix_len = at_mark(string, len, kind);
+ if (suffix_len kind == AT_KIND_UPSTREAM)
+ return suffix_len;
+ return 0;
+}
+
static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned
lookup_flags);
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
--
1.8.3.rc3.17.gd95ec6c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] remote: expose parse_push_refspec()
parse_fetch_refspec() is already available to other callers via
remote.h. There's no reason why parse_push_refspec() shouldn't be.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
remote.c | 2 +-
remote.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index 68eb99b..99c44da 100644
--- a/remote.c
+++ b/remote.c
@@ -660,7 +660,7 @@ struct refspec *parse_fetch_refspec(int nr_refspec, const
char **refspec)
return parse_refspec_internal(nr_refspec, refspec, 1, 0);
}
-static struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)
+struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)
{
return parse_refspec_internal(nr_refspec, refspec, 0, 0);
}
diff --git a/remote.h b/remote.h
index cf56724..2497b93 100644
--- a/remote.h
+++ b/remote.h
@@ -94,6 +94,7 @@ void ref_remove_duplicates(struct ref *ref_map);
int valid_fetch_refspec(const char *refspec);
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
+struct refspec *parse_push_refspec(int nr_refspec, const char **refspec);
void free_refspec(int nr_refspec, struct refspec *refspec);
--
1.8.3.rc3.17.gd95ec6c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] sha1_name: prepare to introduce AT_KIND_PUSH
Introduce an AT_KIND_PUSH to be represented as @{p[ush]}. Determine
it using branch.remote.push_refspec.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
sha1_name.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 106716e..5f6958b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -23,7 +23,7 @@ struct disambiguate_state {
unsigned always_call_fn:1;
};
-#define AT_KIND_UPSTREAM 0
+enum at_kind { AT_KIND_UPSTREAM, AT_KIND_PUSH };
static void update_candidates(struct disambiguate_state *ds, const unsigned
char *current)
{
@@ -426,7 +426,8 @@ static inline int at_mark(const char *string, int len, int
*kind)
int kind;
const char *suffix[2];
} at_form[] = {
- { AT_KIND_UPSTREAM, { @{upstream}, @{u} } }
+ { AT_KIND_UPSTREAM, { @{upstream}, @{u} } },
+ { AT_KIND_PUSH, { @{push}, @{p} } }
};
for (j = 0; j ARRAY_SIZE(at_form); j++) {
@@ -1022,6 +1023,12 @@ static void die_no_upstream(struct branch *upstream,
char *name) {
* given buf and returns the number of characters parsed if
* successful.
*
+ * - branch@{push} finds the name of the ref that
+ * branch is configured to push to (missing branch defaults
+ * to the current branch), and places the name of the branch in the
+ * given buf and returns the number of characters parsed if
+ * successful.
+ *
* If the input is not of the accepted format, it returns a negative
* number to signal an error.
*
@@ -1077,6 +1084,8 @@ int interpret_branch_name(const char *name, struct strbuf
*buf)
free(cp);
cp = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
break;
+ case AT_KIND_PUSH:
+ break;
}
strbuf_reset(buf);
--
1.8.3.rc3.17.gd95ec6c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/7] sha1_name: implement finding @{push}
Try this now: configure your current branch's pushremote to push to
refs/heads/*:refs/heads/rr/*. Now, type 'git show @{p}'. Voila!
It currently only works when:
1. remote.name.push is explicitly specified.
2. There is a pattern to match (*).
Proof-of-concept only.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
sha1_name.c | 20
1 file changed, 20 insertions(+)
diff --git a/sha1_name.c b/sha1_name.c
index 5f6958b..283d538 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1008,6 +1008,24 @@ static void die_no_upstream(struct branch *upstream,
char *name) {
}
}
+static void find_push_ref(struct branch *branch) {
+ struct remote *remote = pushremote_get(NULL);
+ const struct refspec *pat = NULL;
+ char raw_ref[PATH_MAX];
+ struct ref *this_ref;
+ char *dst_name;
+ int len;
+
+ sprintf(raw_ref, refs/heads/%s, branch-name);
+ len = strlen(raw_ref) + 1;
+ this_ref = xcalloc(1, sizeof(*this_ref) + len);
+ memcpy(this_ref-name, raw_ref, len);
+
+ dst_name = get_ref_match(remote-push, remote-push_refspec_nr,
+ this_ref, MATCH_REFS_ALL, 0, pat);
+ printf(dst_name = %s\n, dst_name);
+}
+
/*
* This reads short-hand syntax that not only evaluates to a commit
* object name, but also can act as if the end user spelled the name
@@ -1085,6 +1103,8 @@ int interpret_branch_name(const char *name, struct strbuf
*buf)
cp = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
break;
case AT_KIND_PUSH:
+ find_push_ref(branch);
+ die(Done!);
break;
}
--
1.8.3.rc3.17.gd95ec6c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] remote: expose get_ref_match()
We need it.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
remote.c | 2 +-
remote.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index 99c44da..9ae1fc5 100644
--- a/remote.c
+++ b/remote.c
@@ -1168,7 +1168,7 @@ static int match_explicit_refs(struct ref *src, struct
ref *dst,
return errs;
}
-static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct
ref *ref,
+char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref *ref,
int send_mirror, int direction, const struct refspec **ret_pat)
{
const struct refspec *pat;
diff --git a/remote.h b/remote.h
index 2497b93..5671fe0 100644
--- a/remote.h
+++ b/remote.h
@@ -173,4 +173,7 @@ struct ref *guess_remote_head(const struct ref *head,
/* Return refs which no longer exist on remote */
struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref
*fetch_map);
+char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref *ref,
+ int send_mirror, int direction, const struct refspec **ret_pat);
+
#endif
--
1.8.3.rc3.17.gd95ec6c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] sha1_name: remove upstream_mark()
The first caller get_sha1_basic() just wants to make sure that no
non-numerical @{...} form was matched, so that it can proceed with
processing numerical @{...} forms. Since we're going to introduce more
non-numerical @{...} forms, replace this upstream_mark() call with a
call to at_mark() passing NULL as the last argument; we don't care what
the kind is: all we need to know is if the return value is zero (parse
failure).
The second caller interpret_branch_name() will be expanded in the future
to handle all possible AT_KIND_* values. So, replace the
upstream_mark() call with an upstream_mark() call capturing at_kind and
using it in a switch statement to perform the appropriate action.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
sha1_name.c | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 7aabd94..106716e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -443,15 +443,6 @@ static inline int at_mark(const char *string, int len, int
*kind)
return 0;
}
-static inline int upstream_mark(const char *string, int len)
-{
- int suffix_len, kind;
- suffix_len = at_mark(string, len, kind);
- if (suffix_len kind == AT_KIND_UPSTREAM)
- return suffix_len;
- return 0;
-}
-
static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned
lookup_flags);
static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
@@ -469,7 +460,7 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
if (len str[len-1] == '}') {
for (at = len-2; at = 0; at--) {
if (str[at] == '@' str[at+1] == '{') {
- if (!upstream_mark(str + at, len - at)) {
+ if (!at_mark(str + at, len - at, NULL)) {
reflog_len = (len-1) - (at+2);
len = at;
}
@@ -1044,6 +1035,7 @@ int interpret_branch_name(const char *name, struct strbuf
*buf)
int namelen = strlen(name);
int len = interpret_nth_prior_checkout(name, buf);
int tmp_len;
+ int at_kind;
if (!len)
return len; /* syntax Ok, not enough switches */
@@ -1072,15 +1064,21 @@ int interpret_branch_name(const char *name, struct
strbuf *buf)
cp = strchr(name, '@');
if (!cp)
return -1;
- tmp_len = upstream_mark(cp, namelen - (cp - name));
+ tmp_len = at_mark(cp, namelen - (cp - name), at_kind);
if (!tmp_len)
return -1;
len = cp + tmp_len - name;
cp = xstrndup(name, cp - name);
branch = branch_get(*cp ? cp : NULL);
- die_no_upstream(branch, cp);
- free(cp);
- cp = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+
+ switch (at_kind) {
+ case AT_KIND_UPSTREAM:
+ die_no_upstream(branch, cp);
+ free(cp);
+ cp = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+ break;
+ }
+
strbuf_reset(buf);
strbuf_addstr(buf, cp);
free(cp);
--
1.8.3.rc3.17.gd95ec6c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #05; Mon, 20)
Junio C Hamano venit, vidit, dixit 23.05.2013 16:40: Michael J Gruber g...@drmicha.warpmail.net writes: Didn't you have concerns about storing the context in the object struct? I can't quite judge how much of an issue this can be for fsck and such. I don't want to increase the memory footprint unnecessarily, of course. Yes. I thought I had a weather-balloon patch to fsck to use its own so that we have something to fall back on if it turns out to be a problem (and also so that anybody can see how big the difference is), but I highly suspect that any user of object-array other than what you are changing in the series wants to use the slim variant, which suggests that the information does not belong there. Other than that, the mechanism was still up for discussion (separate show attribute or a config) given that the default behavior for showing blobs is not to change. My understanding was that the series as-is (not the implementation but the external interface) makes us honor what the user tells us better, without changing the behaviour for people who don't instruct us to do anything differently, so I thought it was a good place to stop at. The 'show attribute or config' discussion would/should involve the possibility of flipping the default, no? After all, I was getting the impression, especially from the config, that this was If we had known better when we introduced textconv, we would have defined it to apply in any situation where you would want a textual form of a blob, not limited to diff kind of thing. That is a much longer term thing and my impression was that it can built later on top of the series (once its implementation settles). So, yes, thanks for pointing out these two points. The bloat in the object array element I do care today, the feature and the interface I do not think we have to worry about them today to hold the series back. Well, if we decide showing blobs with textconv is fundamentally different from showing diffs with textconv then --textconv should not apply any textconv filters on blobs unless the user has specified them using a separate attribute (different from diff). Therefore, I hesitate introducing the behavior of the current series. For me, it would introduce something of a mixed beast. I wouldn't hesitate introducing textconv on by default for blobs the same as for diffs, but that would change existing behavior and the opposition is strong, for a good reason :) So, since that one isn't possible, I'm leaning towards the other extreme: treat the blob and diff case completely separately in the sense that different attributes are used. Then, even with --textconv there wouldn't be any blob filtering (show/grep) unless the user specified so using an attribute different from diff. I'd rather try out the latter before having the mixed beast trickle down too much, even both its change in behavior and the one from the attribute version do not show up in daily use until you specify --textconv explicitly. Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report: git grep seems to use the wrong .gitattributes when invoked in a subdirectory
Am 23.05.2013 14:44, schrieb Joel Kaasinen: [pseudo:~/tmp]% git --version git version 1.7.10.4 [pseudo:~/tmp]% git init git-test Initialized empty Git repository in/home/opqdonut/tmp/git-test/.git/ [pseudo:~/tmp]% cd git-test [pseudo:~/tmp/git-test:master()]% mkdir -p a/data [pseudo:~/tmp/git-test:master()]% mkdir data [pseudo:~/tmp/git-test:master()]% echo '* binary' data/.gitattributes [pseudo:~/tmp/git-test:master()]% echo foo a/data/foo [pseudo:~/tmp/git-test:master()]% git add -A [pseudo:~/tmp/git-test:master()]% git commit -m foo [master (root-commit) 20fafbb] foo 2 files changed, 1 insertion(+) create mode 100644 a/data/foo create mode 100644 data/.gitattributes [pseudo:~/tmp/git-test:master()]% git grep foo a/data/foo:foo [pseudo:~/tmp/git-test:master()]% cd a [pseudo:~/tmp/git-test/a:master()]% git grep foo Binary file data/foo matches Just checked, with git 1.8.2.3 the last command shows a matching line. The issue has probably been fixed by 55c6168 (grep: stop looking at random places for .gitattributes). The lowest release with that patch is git 1.8.0.1. René -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
John Keeping j...@keeping.me.uk writes: I've been annoyed by this at $DAYJOB recently. A lot of people seem to blindly git pull without much thought about how the history is ending up and what they actually want to do. I think these two are essentially the same thing, and having an option to flip the heads of a merge only solves a half of the problem. A merge that shows everybody else's work merged into your history means you are the integrator, the keeper of the main history. And the first-parent view of the history is useful only when the keeper of the main history takes good care of the main history. When you are using a central shared repository workflow, if you had and used an option to flip the heads of a merge to record what you have done so far as a side branch of what everybody else did to do the merge, or if you rebased your work on top of what everybody else did, the first-parent view would make a bit more sense than what you currently get. At least, everybody else's work will not appear as a side branch that does 47 unrelated things, and your work will appear as a side branch. That is a big plus. But the other half of the problem still remains, i.e. what they actually want to do. People tend to do too many pull when their work is not ready, only to catch up, and that is the real problem. Instead of having a nice these six commits marked as 'x' were done on a branch forked some time ago, to address only this one issue and to address it fully history that explains how these commits were related and these commits are the full solution to a single issue: x---x---x---x---x---x / \ ---o---o---o---o---o---o---M---o---o---... they end up with something like this, even with the flip the heads of a merge option, by pulling too often: x---x x---x---x x / \ / \ / \ ---o---o---M---o---o---M---M---o---o---... The result fragments otherwise a logical and clean single strand of pearls to fully address the issue, consisting of 6 commits, into separate and seemingly unrelated pieces. Imagine that other people are working the same way, and the commits marked with 'o' are merges of side branches they add their half-way work to the main history similar to what happened in the second illustration above. You would get this history: x---x x---x---x x / \ / \ / \ ---o---o---M---o---M---M---M---o---o---... \ / y---y Nothing, other than the labels I used in the picture, ties these 'x's together while differentiating them from 'y's, so you lost an important information. Unless people stop doing that too many pulls that are used only to catch up, even with the flip the heads of a merge option, you will not get a history that yields a good first-parent view. That gets back to what I said in the second paragraph of this message. When you pull from the central shared repository, with the flip the heads of a merge option, you are acting as the keeper of the main history at that point, and you are responsible for taking good care of it. If you make a 2+3+1=6 mess as depicted in the last illustration above, you are failing to do so. One obvious way to solve it is to use a topic branch workflow (the first picture above; 'x's are built not on local 'master'), and you do a git pull from the shared repository while you are on your 'master', which is free of your 'x's until that 6-commit series is complete and ready. Then you locally merge that topic branch and push it back for everybody to see, which will give you the first picture in this message. Incidentally, this does not need the flip the heads option. Solving half a problem is better than solving no problem, and especially because not all changes need to be multi-commit series but can be done directly, perfectly and fully on the local 'master' (i.e. 2+3+1=6 split would not happen for such changes). For these reasons, I personally am not strongly opposed to a flip the heads option, if implemented cleanly. But people need to realize that it is not solving the other half, a more fundamental problem some people have in their workflow. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 09:01:15AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: I've been annoyed by this at $DAYJOB recently. A lot of people seem to blindly git pull without much thought about how the history is ending up and what they actually want to do. I think these two are essentially the same thing, and having an option to flip the heads of a merge only solves a half of the problem. A merge that shows everybody else's work merged into your history means you are the integrator, the keeper of the main history. And the first-parent view of the history is useful only when the keeper of the main history takes good care of the main history. When you are using a central shared repository workflow, if you had and used an option to flip the heads of a merge to record what you have done so far as a side branch of what everybody else did to do the merge, or if you rebased your work on top of what everybody else did, the first-parent view would make a bit more sense than what you currently get. At least, everybody else's work will not appear as a side branch that does 47 unrelated things, and your work will appear as a side branch. That is a big plus. But the other half of the problem still remains, i.e. what they actually want to do. People tend to do too many pull when their work is not ready, only to catch up, and that is the real problem. ... One obvious way to solve it is to use a topic branch workflow (the first picture above; 'x's are built not on local 'master'), and you do a git pull from the shared repository while you are on your 'master', which is free of your 'x's until that 6-commit series is complete and ready. Then you locally merge that topic branch and push it back for everybody to see, which will give you the first picture in this message. Incidentally, this does not need the flip the heads option. Yes, I don't think this is as much of a problem when using a topic branch workflow, because it's clear what the history should look like and users are expected to get it right. Where I see this is when people are aiming for a linear history but don't get that because with git pull to catch up they end up with these backwards merges. In these cases, I think what users really want is git pull --rebase. I have to wonder how often git pull with no arguments actually does what users really want (even if they don't know it!) when it doesn't result in a fast-forward (and pull.rebase isn't configured). Hence my suggestion to error when git pull doesn't result in a fast-forward and no branch name is specified. We could give some advice like: Your local changes are not included in the local branch and you haven't told Git how to preserve them. If you want to rebase your changes onto the modified upstream branch, run: git pull --rebase Solving half a problem is better than solving no problem, and especially because not all changes need to be multi-commit series but can be done directly, perfectly and fully on the local 'master' (i.e. 2+3+1=6 split would not happen for such changes). For these reasons, I personally am not strongly opposed to a flip the heads option, if implemented cleanly. But people need to realize that it is not solving the other half, a more fundamental problem some people have in their workflow. Yes, but some users don't realise that their workflow is broken, and perhaps we can nudge them in the right direction. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
Felipe Contreras felipe.contre...@gmail.com writes:
On Wed, May 22, 2013 at 11:07 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
On Wed, May 22, 2013 at 7:08 PM, Junio C Hamano gits...@pobox.com wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
IIRC, git-gui runs two blames, one without any -C and one with (I do
not offhand recall how many -C it uses) to show both.
'git blame' is a very expensive operation, perhaps we should add
another option so users don't need to run two blames to find this.
Yes, you could add a struct origin *suspect_in_the_same_file next
to the current struct origin *suspect to the blame_entry in order
to represent the origin you would get if you were to stop at a move
across files event, and keep digging, when you are operating under
-C -C or higher mode.
No, this is what I meant:
But that would not print the right line numbers, so perhaps:
Users of full-output may want to be able to see the same thing.
I have a suspicion that you misread what assign_blame() does. The
first inner loop to find one suspect is really what it says. It
loops over blame entries in the scoreboard but it is not about
finding one entry, but is to find one suspect. The ent found
in the loop that you store in tmp_ent is no more special than any
other ent that shares the same suspect as its origin.
Imagine that your scoreboard originally has three blocks of text
(i.e. blame_entry) A, B, and C, and the current suspect for A and C
are the same, while we already know what the final guilty party for
B is (which may be some descendant of the suspect).
Once we find one suspect in the first inner loop, the call to
pass_blame() does everything it can do to exonerate that suspect.
It runs a single diff between the suspect and each of its parents to
find lines in both A and C that can be blamed to one of these
parents, and blame entries A and C are split into pieces, some of
which still have the same suspect as their origin, which are where
their lines originate from, while others are attributable to these
parents or their ancestors.
If you keep the original entry for A to do something special, like
printing what the original range of A was before it was split by
pass_blame(), wouldn't you need to do the same for C? We will not
be visiting C later in the outer while(1) loop, as a single call to
pass_blame() for suspect we did when we found it in A has already
taken care of it.
I am not sure what you are trying to achieve with that found-guilty
logic, even if the save away in tmp_ent logic is changed to keep
all the blame entries that have suspect we are looking at as their
origin. When the current suspect is found to have passed all lines
intact from its parents, we will see found-guilty left to be false.
How would it make the original blame_entry (perhaps halfway) guilty
in that situation?
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1500,25 +1500,16 @@ static int emit_one_suspect_detail(struct
origin *suspect, int repeat)
return 1;
}
-/*
- * The blame_entry is found to be guilty for the range. Mark it
- * as such, and show it in incremental output.
- */
-static void found_guilty_entry(struct blame_entry *ent)
+static void print_guilty_entry(struct blame_entry *ent)
{
- if (ent-guilty)
- return;
- ent-guilty = 1;
- if (incremental) {
- struct origin *suspect = ent-suspect;
-
- printf(%s %d %d %d\n,
-sha1_to_hex(suspect-commit-object.sha1),
-ent-s_lno + 1, ent-lno + 1, ent-num_lines);
- emit_one_suspect_detail(suspect, 0);
- write_filename_info(suspect-path);
- maybe_flush_or_die(stdout, stdout);
- }
+ struct origin *suspect = ent-suspect;
+
+ printf(%s %d %d %d\n,
+ sha1_to_hex(suspect-commit-object.sha1),
+ ent-s_lno + 1, ent-lno + 1, ent-num_lines);
+ emit_one_suspect_detail(suspect, 0);
+ write_filename_info(suspect-path);
+ maybe_flush_or_die(stdout, stdout);
}
/*
@@ -1531,14 +1522,19 @@ static void assign_blame(struct scoreboard *sb, int
opt)
struct rev_info *revs = sb-revs;
while (1) {
- struct blame_entry *ent;
+ struct blame_entry *ent, tmp_ent;
struct commit *commit;
struct origin *suspect = NULL;
+ int found_guilty = 0;
/* find one suspect to break down */
- for (ent = sb-ent; !suspect ent; ent = ent-next)
- if (!ent-guilty)
+ for (ent = sb-ent; ent; ent = ent-next)
+ if (!ent-guilty) {
+ tmp_ent = *ent;
suspect = ent-suspect;
+ break;
+ }
+
if (!suspect)
return; /* all done */
@@ -1564,9 +1560,20 @@ static
Re: git stash deletes/drops changes of
Adeodato Simó dato at net.com.org.es writes: I was unpleasantly surprised to discover yesterday that doing `git stash` on a repository where I had previously run `git update-index --assume-unchanged FOO` completely lost all changes I had in file FOO. I just ran into this today. Was a decision about this behavior reached in the intervening time? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Feature Request: existence-only tracking
In my work, we have a lot of autogenerated files that need to exist so a script will replace their contents, but tracking the contents creates a lot of unnecessary conflicts. I would love to see an option for a different tracking method that just makes sure a file or directory exists. This would also obviate the need for adding things like .empty files in a directory to force the directory to exist. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
Felipe Contreras felipe.contre...@gmail.com writes: On Wed, May 22, 2013 at 10:23 PM, Felipe Contreras felipe.contre...@gmail.com wrote: This doesn't make any sense: Ah, never mind, it's COPYING the one being modified, not EXTRACTING. Yes. The different levels of -C happens to correspond to the software engineering practice from best to sloppy: - When you refactor to have a block of lines in a file , the original of that block would have come from another file (or the same file) you touched to remove duplication, so a single -C looks for an origin only from modified files (best). - When you start a new file, you may have to start from some boilerplate material that already exists in another file that is not (and does not have to be) changed in the commit you add that new file, so double -C -C looks for an origin of lines from other files, even unmodified ones, when looking at the lines in a new file (unfortunate but acceptable). - When you copy and paste without making any effort to refactor, you modify a file by adding new lines that match identically from another existing file, the latter of which does not change in the commit you do that copy and paste. To find this, you need to look for an origin for any file from all the other files, and triple -C -C -C lets you do so (sloppy). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/17] revision: split some overly-long lines
Michael Haggerty mhag...@alum.mit.edu writes: On 05/21/2013 07:34 PM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- revision.c | 20 ++-- revision.h | 32 +--- 2 files changed, 35 insertions(+), 17 deletions(-) Looks obviously good for *.c file, but I am on the fence for *.h one, as the reason we kept these long single lines in *.h files was to help those who want to grep in *.h files to let them view the full function signature. It probably is OK to tell them to use git grep -A$n instead, though. My goal with this patch was not to set a new policy but rather just to make the code conform a little better to the existing policy as described in CodingGuidelines. *If* it is preferred that header files list all parameters on a single line, then by all means adjust the CodingGuidelines and I will just as happily make header files conform to *that* policy when I touch them :-) (That being said, my personal preference is to apply the 80-character limit for header files too.) Yeah, that is why I said I am on the fence but it probably is OK to break the unwritten but guessable rule. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug when git rev-list options --first-parent and --ancestry-path are used together?
Michael Haggerty mhag...@alum.mit.edu writes: It seems to me that git rev-list --first-parent --ancestry-path A..B is well-defined and should list the commits in the intersection between git rev-list --first-parent A..B and git rev-list--ancestry-path A..B But in many cases the first command doesn't provide any output even though there are commits common to the output of the last two commands. For example, take as an example the DAG from test t6019: # D---E---F # / \ \ #B---C---G---H---I---J # / \ # A---K---L--M (The merges are always downwards; e.g., the first parent of commit L is K.) The command git rev-list --first-parent --ancestry-path D..J doesn't generate any output, whereas I would expect it to output H I J. As I do not see how only show first-parent chains from near the tip but stop immediately when the chain deviates from the ancestry path could be a sensible operation (in other words, I do not offhand think of examples of what useful things you can do with that information), I actually expect that -f-p -a-p D..J should error out, instead of giving no output. You are correct to point out that sometimes -f-p and -a-p _could_ be compatible, e.g. -f-p -a-p A..M, or -f-p -a-p B..M. But I think the only case that they are compatible is when -f-p output is a strict subset of what -a-p without -f-p would give. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Random thoughts on upstream
Ramkumar Ramachandra artag...@gmail.com writes: [remote theodore] push = master push = +pu This means that I will always push master without force and pu with force, irrespective of the branch I'm on. [remote origin] push = refs/heads/*:refs/heads/rr/* This means that I will always push all branches without force with rewriting, irrespective of the branch I'm on. Exactly. And some people are unhappy about it, because they prefer to work per-branch basis, as opposed to having to perfect everything into a publishable state first and then finally push everything out in one go. I am not saying default=single would be _the_ single right way to solve it, but when you have default=single, remote.$name.push is used only to describe the mapping, not forcing you to push everything out at once is one possible solution to that. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #05; Mon, 20)
Michael J Gruber g...@drmicha.warpmail.net writes: Well, if we decide showing blobs with textconv is fundamentally different from showing diffs with textconv then --textconv should not apply any textconv filters on blobs unless the user has specified them using a separate attribute (different from diff). I had an impression that the ship has already sailed wrt to diff being pretty much interchangeable as text (or non binary) in the attribute system long time ago. Therefore, I hesitate introducing the behavior of the current series. For me, it would introduce something of a mixed beast. I wouldn't hesitate introducing textconv on by default for blobs the same as for diffs, I would. But I wouldn't for the user asks for --textconv, the user expects the blob shown with mangling, which sounds like a good thing to do. And there is anything wrong to refine later where exactly that textconv filter is defined. At this moment, diff.textconv is the only place the user could even contemplate setting one, and there is no risk for confusion. blob.textconv can be introduced much later when some users actually want to have a pair of different filters, at that point diff.textconv will become a backward compatiblity fallback for that filter. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Random thoughts on upstream
Junio C Hamano wrote: I am not saying default=single would be _the_ single right way to solve it, but when you have default=single, remote.$name.push is used only to describe the mapping, not forcing you to push everything out at once is one possible solution to that. The big advantage it has, in my opinion, is ease of implementation: you'd essentially have to grab the remote.name.push refspec, rewrite it to replace refs/heads/*: with $HEAD: and feed that refspec to the transport layer. Compare that to inventing a new refspec syntax, which is a mammoth task. We can always extend the refspec later, if we want that. I wonder if it makes sense to bake the functionality into current though: will we be breaking anything? (My opinion has changed after wrestling with the horrible transport layer) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Geolocation support
Antoine Pelisse apeli...@gmail.com writes: I'm really not convinced this kind of changes should make it into Junio's tree (of course, he's the only one to decide). I really believe this is a very specific solution to a very specific problem (that is not for me to judge if the problem is real). Bloating the commit object with this kind of information doesn't feel like a good idea. I think it could be nice to provide a simple shell script to build the location, callable from a post-commit hook, to construct a geolocation note. Gitk could be programmed to read the notes to get the location, but once again, I'm not sure it should be mainlined. I would personally find the feature cute, but - I think a note is an overkill for that; and - I also think that adding cruft to commit object header in a backward incompatible way, with the only possible escape-hatch being if you want to keep being compatible with other people, you can choose not to use this feature, is a slipperly slope to fragmentation we do not want to go nearby. Wouldn't it be sufficient to treat this in a similar way as references to tracker entries and references to other commit objects in the text of the commit message body are treated by gitk and friends? Just embed the information in the log text somewhere and teach the UI how they look like and what to do with them. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/17] gc_boundary(): move the check alloc = nr to caller
Michael Haggerty mhag...@alum.mit.edu writes: 1. The function name gc_boundary() suggests that it will do a garbage collection unconditionally. In fact, it might or might not depending on this test. At the caller, this made it look like a gc was happening each time through the loop, which made me nervous about the performance. The new version makes it clear at the caller that the gc is only happening occasionally. Perhaps. 2. Even assuming that gc_boundary() were renamed to maybe_gc_boundary(), the function has hopelessly little information on which to base its decision whether or not to gc, and the choice of policy can only be justified based on some implicit knowledge about how the array is grown and shrunk. But the growing/shrinking is done at the layer of the caller, and therefore the choice of gc policy also belongs at the layer of the caller. 3. As you point out, if the gc policy is ever to be made more intelligent, then gc_boundary() is unlikely to have enough information to implement the new policy (e.g., it would have no place to record low/high water marks). Separating concerns at the correct level would make a change like that easier. These two depend on how you look at the API hierarchy. You seem to think that the ideal end result is get_revision_internal() have an open coded when to gc logic in this function call object_array_filter() My suggestion was based on a different view, which is: get_revision_internal() call gc_boundary() gc_boundary() make decision on when and how to gc if decided to do so call object_array_fitler() You can obviously rename gc_boundary() to auto_gc_boundary() if that makes it easier to understand, but these two belong to the same codepath that deals with the object array used specifically for keeping track of boundary commits. I view who has what information as secondary---if the decision to gc is not the primary thing get_revision_internal() should be worrying about (and I do not think it is), it would be a better code structure to have a helper specific for doing so, i.e. gc_boundary(), and delegate that part of job to it. Obviously, the caller needs to supply sufficient information to that helper if the helper needs more than what it currently gets by adding parameters to it, but that goes without saying. At the moment I am not interested in improving the gc policy of this code. I am not either; the only effect we get from removing gc_boudnary() and calling directly to object_array_filter() is to lose the above abstraction and make it easy to let get_revision_internal() become more cluttered when somebody else later decides to improve the gc policy, it seems. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
Michael S. Tsirkin m...@redhat.com writes: Looks like push can't resolve tags to commits. Why is that? How else would you push a tag out? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: English/German terminology, git.git's de.po, and pro-git
* Ralf Thielow ralf.thie...@gmail.com [130522 17:17]: remote branch = Remote-Branch remote-tracking branch = Remote-Tracking-Branch upstream branch= Upstream-Branch Yes. What's the main reason for using Branch in the German text? Consistency with the commands, or assumed familiarity of the term within the target audience? Zweig is available. I think it's at the same level as Commit and a well known SCM-term. Users (even beginners) who know Commit and Tag do also know Branch. And I think it sounds better in combination with Remote-, Remote-Tracking- and Upstream- which are english words. Additionally Zweig might be a bit misleading. A branch is not part of the trees. It is called branch because in other VCSes the commits build a tree and a any commit outside of the main branch of that tree is part of exactly one different branch (so the head of that branch and the branch are synonymns). With git the commits are no longer a tree, so a git-branch is no branch and does not describe the whole branch of the tree of commits but is just a names pointer into the graph of commits. As it lost all meanings of the original word branch, translating it with a translation of the original English word might more confusion than helping anyone. (same for push). In other messages, the translation is in the same message as the command itself. I think it's OK when we just use fetch and push when the command is meant (as it's done for pull, e.g. in error messages), and the translation when the messages tell what the command is doing (e.g. help messages). So it would depends on the message whether we translate the word or not. This would apply to other terms that are commands, too, like clean or revert. I'd not call it OK. It's the only sane possibility. If you speak about the magic keyword you have to give the command line, you won't translate it, of course[1]. (The obvious interesting case is where the English text plays with the command name having a meaning as word itself. Here the translation will have to diverge to differentiate between both (or sacrifice one of them, where it is not important)). [1] Unlike you want to introduce a translated command line interface, like Depp anfordere Herkunft Original instead of git fetch origin master diff = Differenz delta = Differenz (or Delta) patch = Patch apply = anwenden diffstat = (leave it as it is) hunk = Bereich IMHO Kontext is better if you use a German word. Technically the context is something else, but in a German text IMHO it fits nicer when explaining to the user where he/she can select the n-th hunk. Not sure if German users would know what hunk means, in case we leave it untranslated. And I'm not sure if I would understand Kontext. I tend to leave it untranslated. Anyone found a German translation of the Patch manpage? Translating the English word-play here, I'd suggest Block or Patch-Block. paths = Pfade symbolic link = symbolische Verknüfung path = Pfad link = Verknüpfung In the filesystem a Link is a Verweis in Unix, not a Verknüpfung (that are usually the pseudo-links Windows supports). Bernhard R. Link -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
Junio C Hamano gits...@pobox.com writes: Users of full-output may want to be able to see the same thing. ... but I agree we may be able to cheat if we only want to support interactive, and we do want to find a way to cheat when we are running interactive without having to store much more information in each blame_entry. Imagine that your scoreboard originally has three blocks of text ... in that situation? (I snipped everything I said in the previous reply only for brevity but they are still relevant). I _think_ one way to cheat without storing too much information in each blame_entry is to first make a copy of all the original blame entries that share the suspect, see if any line in the line range represented by each of the blame entries ended up being blamed to an origin with a path different from that of the suspect. And print those original blame entries that fits the criterion as additional these are not the final blame as you are digging with the option to detect copy across files, but at this commit this line appeared anew as far as the origin-path is concerned output. So I think you were going in the right direction (in other words, the patch inserted new code to the right places), even though you may have got the details in what the new code should do wrong. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added guilt.reusebranch configuration option.
On Thu, May 23, 2013 at 03:22:50PM +0530, Ramkumar Ramachandra wrote:
Theodore Ts'o wrote:
Right now I do this just by being careful, but if there was an
automatic safety mechanism, it would save me a bit of work, since
otherwise I might not catch my mistake until I do the git push
publish, at which point I curse and then start consulting the reflog
to back the state of my tree out, and then reapplying the work I had
to the right tree.
My scenario is a bit different, and I think this safety feature is
highly overrated. It's not that I'll never rewind some branches, but
rewind other branches, but rather I might rewind anything at any
time, but I want immediate information so I can quickly inspect @{1}
to see if that was undesirable.
Spekaing of which, what I'd really appreciate is timestamps associated
with the reflog. That's because the most common time when I've
screwed something up is after doing a git rebase -i and so the
reflog has a *huge* number of entries on it, and figuring out which
entry in the reflog is the right one is painful. If could tell at a
glance when each entry of the reflog was created, it would make it a
lot easier to untangle a tree mangled by git rebase -i.
In practice, it means I waste five minutes carefully inspecting a few
dozen entries on the reflog, so it's not a disaster, although I'm
generally cursing the whole time while I'm trying to untangle the
whole mess.
This issue with reflogs not having timestamps isn't primarily about
rewind safety, BTW; it's just one of the things which make consulting
the reflog painful --- and it's much more likely happens after I screw
up a git rebase -i, generally because of what happens when there's a
merge conflict and then I accidentally fold two commits together
unintentionally. The times when I've screwed up a non-rewinding
branch and then needed to recover after discovering the problem when I
try to publish said branch are admittedly rare; maybe once or twice
times in the past twelve months.
So, do you still need this rewinding safety thing?
Meh; I don't *need* it. But then again, I'm an fairly experienced git
user. The fact that I use guilt without the guilt/master safety
feature and have never gotten bitten by it --- in fact I deliberately
publish rewindable branches with a guilt patch series applies speaks
to the fact that I'm pretty experienced at rewindable heads.
The only reason why I suggested it is because I believe it would be
useful for people with less experience, and perhaps it would help make
rewindable branches less scary, and less subject to a lot of the
fearmongering that you see on the blogosphere.
So what I do is something like this:
git push publish ; git push repo ; git push code
While we can definitely make the UI better for this (maybe push
--multiple?), there is no fundamental change: we have to re-initialize
all the refspecs, connect to the remote via the transport layer and
prepare a packfile to send. In other words, it's impossible to make
it any faster than what you get with the above.
Sure, and if I cared I'd make a git alias to automate this, instead of
depending on finger macros.
So you're a batched-push person. And the above makes it clear that
you don't want to explicitly differentiate between a push and push -f
(the +pu thing). And this assumes that you never create any new
branches (I branch out all the time), otherwise you'd have rules for
refs/heads/*.
I create new branches all the time. But they are for my own personal
testing purposes. So it's fairer to say that I rarely *publish* new
branches; I generally stick to the standard set of next, master,
maint, and pu. And part of that is that even publishing this number
of branches is enough to sometimes confuse the e2fsprogs developers
who are pulling from my tree.
So what I've done in the past is to create a whole bunch of feature
branches, and then merge them into the pu branch, and then only
publish the pu branch. And I try to get the feature branches cleaned
up as quickly as I have time, so they can appear on the maint or
master/next branches sooner rather than later.
Just out of curiosity, do you ever have ref-renaming
requirements (like push = refs/heads/*:refs/heads/tt/*)? We were
discussing that on another thread, but I haven't found an
implementation I'm happy with yet.
In general, no, I don't do that, for the reasons stated above --- even
publishing four branches gets to be confusing enough for people who
are looking at my tree.
I'm sure other people and other communities use git differently, so
please insert the standard disclaimer that there's more than one way
to skin a cat.
Regards,
- Ted
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added guilt.reusebranch configuration option.
Theodore Ts'o ty...@mit.edu writes:
On Wed, May 22, 2013 at 11:55:00AM -0700, Junio C Hamano wrote:
But in a triangular workflow, the way to make the result reach the
upstream is *not* by pushing there yourself. For developers at
the leaf level, it is to push to their own repository (often on
GitHub), which is different from where they (initially) clone from
in order to bootstrap themselves, and (subsequently) pull from in
order to keep them up-to-date. And then they request the published
work to be pulled by the upstream.
Yep, what I do personally is to call the destination of this publish, i.e.:
[remote publish]
url = ssh://gitol...@ra.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.g
push = +master:master
push = +origin:origin
push = +dev:dev
So my typical work flow when I am ready to submit to Linus is:
git tag -s ext4_for_linus
git push publish
wait for this to propagate from ra.kernel.org to git.kernel.org,
typically ~5 minutes
And at this point I presume that you wish this push automatically
pushed out ext4_for_linus, just like fetch by default grabs tags
that point into the history being fetched?
I think push --follow-tags in the upcoming 1.8.3 would work for
you if that is the case.
git request-pull
git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git origin
/tmp/pull
use /tmp/pull as the e-mail body to send to Linus, cc'ing
LKML and linux-e...@vger.kernel.org
But actually, it's much more common that I am doing a git push
publish so that (a) it can get picked up by the daily linux-next tree
(for integration testing even before Linus pulls it into his tree),
and (b) so other ext4 developers so they can either test or develop
against the ext4 tree in progress.
I suppose it would be convenient for git push to push to the
publish target, but I don't get confused about pushing to origin,
since semantically what I am doing is publishing the current state of
the ext4 tree so other people can see it. So git push publish makes
a lot of sense to me.
Noted.
Even in a triangular workflow, @{u} should still refer to the place
you integrate with, i.e. your upstream, not to the place you push
to publish the result of your work.
This branch.branch.rewindable safety however cannot be tied to
@{u}. The bottom boundary you want to be warned when you cross is
the change you pushed out to your publishing repository, and it may
not have reached remotes.origin.branch yet.
Indeed, and in fact for my use case what I promise people is that all
of the commits between origin..master are non-rewindable. It's the
commits betewen master..dev which are rewindable. So for me, I'd
still use the safety feature even for my rewindable branch, but
instead of using remotes/publish/dev the no-rewind point, I'd want
to use remotes/publish/master as the no-rewind point.
Sounds sensible.
Right now I do this just by being careful, but if there was an
automatic safety mechanism, it would save me a bit of work, since
otherwise I might not catch my mistake until I do the git push
publish, at which point I curse and then start consulting the reflog
to back the state of my tree out, and then reapplying the work I had
to the right tree.
Yes, exactly.
Yes, that would be convenient. BTW, one of the other things which I
do for e2fsprogs is that I use multiple publishing points, which is
mostly for historical reasons --- it used to be that repo.or.cz wasn't
all that reliable, and the 10-15 minute replication time from
ra.kernel.org to git.kernel.org got really old.
So what I do is something like this:
git push publish ; git push repo ; git push code
where
[remote publish]
url = ssh://gitol...@ra.kernel.org/pub/scm/fs/ext2/e2fsprogs.git
...
[remote repo]
...
I don't know if this is something you'd want git to encourage, or
support explicitly, but I thought I'd mention it.
I think you can have more than one destination URLs to a single
remote you are pushing as long as what are pushed and how are common
to them, that is, something like this:
[remote publish]
; where do we fetch/pull from and how?
url = ssh://gitol...@ra.kernel.org/pub/scm/fs/ext2/e2fsprogs.git
fetch = +refs/heads/*:refs/heads/*
; where do we push to and how?
pushurl = ssh://gitol...@ra.kernel.org/pub/scm/fs/ext2/e2fsprogs.git
pushurl = https://code.google.com/p/e2fsprogs/
pushurl = ssh://repo.or.cz/srv/git/e2fsprogs.git
push = next
push = master
push = maint
push = debian
push = +pu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added guilt.reusebranch configuration option.
Theodore Ts'o wrote: Spekaing of which, what I'd really appreciate is timestamps associated with the reflog. That's because the most common time when I've screwed something up is after doing a git rebase -i and so the reflog has a *huge* number of entries on it, and figuring out which entry in the reflog is the right one is painful. If could tell at a glance when each entry of the reflog was created, it would make it a lot easier to untangle a tree mangled by git rebase -i. Yeah, I completely agree with this one. I've wished for the reflog to be presented in a nicer ui, with humanized timestamps and colors. Meh; I don't *need* it. But then again, I'm an fairly experienced git user. The fact that I use guilt without the guilt/master safety feature and have never gotten bitten by it --- in fact I deliberately publish rewindable branches with a guilt patch series applies speaks to the fact that I'm pretty experienced at rewindable heads. Oh, and thanks for mentioning guilt: I just learnt about it. The only reason why I suggested it is because I believe it would be useful for people with less experience, and perhaps it would help make rewindable branches less scary, and less subject to a lot of the fearmongering that you see on the blogosphere. My message was a critique. I'm not denying that the feature may be useful; it's just that we should have a good rationalization of the usecase and design something carefully. Sure, and if I cared I'd make a git alias to automate this, instead of depending on finger macros. Yes. My comment was more of question: can --multiple be more than a for loop written in shell? If not, is it worth writing? Are there enough users? Junio mentioned pushurl in the other email: if they're perfect mirrors, won't pushurl suffice? I create new branches all the time. But they are for my own personal testing purposes. So it's fairer to say that I rarely *publish* new branches; I generally stick to the standard set of next, master, maint, and pu. And part of that is that even publishing this number of branches is enough to sometimes confuse the e2fsprogs developers who are pulling from my tree. Just for contrast: I never keep anything locally. I publish as much of my setup as humanly possible so that I'm not tied to one machine. In general, no, I don't do that, for the reasons stated above --- even publishing four branches gets to be confusing enough for people who are looking at my tree. Just publish different branches to different locations? Isn't that why we got triangular workflows? I'm sure other people and other communities use git differently, so please insert the standard disclaimer that there's more than one way to skin a cat. Ofcourse. I believe in being all-inclusive, and not dropping a single feature that has users. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Added guilt.reusebranch configuration option.
Theodore Ts'o ty...@mit.edu writes:
Spekaing of which, what I'd really appreciate is timestamps associated
with the reflog. That's because the most common time when I've
screwed something up is after doing a git rebase -i and so the
reflog has a *huge* number of entries on it, and figuring out which
entry in the reflog is the right one is painful. If could tell at a
glance when each entry of the reflog was created, it would make it a
lot easier to untangle a tree mangled by git rebase -i.
Do you mean you want to go back to one specific step in rebase -i,
or you mean you want to go back to the state before rebase -i?
If the latter, one nice thing to know may be that git log -g is
like git log -g HEAD@{0} and inspects the reflog associated with
HEAD, and you can view individual steps of rebase -i. On the
other hand, git log -g @{0} (or git log -g master@{0} if you are
on 'master' branch) will inspect the reflog associated with the
current branch, and rebase -i appears as a single event (i.e. the
tip before rewinding and replaying all the changes is replaced with
the tip after that whole series of replaying). So the latter is
what you want to use if you are interested in the state before the
whole rebase -i operation.
Also you can ask git log -g HEAD@{now} and git log -g @{now}. I
agree with you that git log --oneline -g @{now} is very handy, and
git log --oneline --relative-date -g @{now} is even better, as I
can clearly see where the flurry of recent activities ends and which
reflog entry is the one I was at 20 minutes ago before I started.
This issue with reflogs not having timestamps isn't primarily about
rewind safety,...
I think I may have answered this part with the above.
So what I've done in the past is to create a whole bunch of feature
branches, and then merge them into the pu branch, and then only
publish the pu branch. And I try to get the feature branches cleaned
up as quickly as I have time, so they can appear on the maint or
master/next branches sooner rather than later.
Sounds very similar to somebody else is doing ;-)
Just out of curiosity, do you ever have ref-renaming
requirements (like push = refs/heads/*:refs/heads/tt/*)? We were
discussing that on another thread, but I haven't found an
implementation I'm happy with yet.
In general, no, I don't do that, for the reasons stated above --- even
publishing four branches gets to be confusing enough for people who
are looking at my tree.
I'm sure other people and other communities use git differently, so
please insert the standard disclaimer that there's more than one way
to skin a cat.
Agreed to both counts. Thanks for comments.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, 23 May 2013 11:06:57 +, Andreas Krey wrote: ... ... Don't do that, then. Ouch, you're right. The problem is not actually in the pull; only the *last* pull into a feature branch that then get pushed back ff to master needs to be reversed. And at that time you don't know it's the last one - swap parents before the push if necessary. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current
On Thu, 23 May 2013 07:45:34 +, Junio C Hamano wrote: ... Even with 'mv', between the time the main in mv starts and the process finally issues rename(2) on the directory, you can start running what competes and interferes with it in another terminal, so it does not fundamentally change anything, I would have to say. Not fundamentally, it's just that the mv is fast enough you wouldn't even think of switch to another term/backgrounding it. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git hangs on pthread_join
On Thursday, May 23, 2013 07:01:43 am you wrote: I'm running a rather special configuration, basically i have a gerrit server pushing ... I have found git receive-packs that has been running for days/weeks without terminating ... Anyone that has any clues about what could be going wrong? -- Have you narrowed down whether this is a git client problem, or a server problem (gerrit in your case). Is this a repeatable issue. Try the same operation against a clone of the repo using just git. Check on the server side for .noz files in you repo (a jgit thing), -Martin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-send-email: fix handling of special characters
Michael S. Tsirkin m...@redhat.com writes:
When patch sender's name has special characters,
git send-email did not quote it before matching
against the author name.
As a result it would produce mail like this:
Date: Thu, 23 May 2013 16:36:00 +0300
From: Michael S. Tsirkin m...@redhat.com
To: qemu-de...@nongnu.org
Cc: Michael S. Tsirkin m...@redhat.com
Subject: [PATCH 0/9] virtio: switch to linux headers
Message-Id: 1369316169-20181-1-git-send-email-...@redhat.com
From: Michael S. Tsirkin m...@redhat.com
Fix by sanitizing before matching to patch author name.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
git-send-email.perl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index bd13cc8..c4dc438 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1400,7 +1400,8 @@ foreach my $t (@files) {
$subject = quote_subject($subject, $auto_8bit_encoding);
}
- if (defined $author and $author ne $sender) {
+ my $sanitized_sender = sanitize_address($sender);
+ if (defined $author and $author ne $sanitized_sender) {
$message = From: $author\n\n$message;
if (defined $author_encoding) {
if ($has_content_type) {
Is $author already sanitized at this point in the code? I see it
was unwrapped with unquote_rfc2047 after it was read from the From:
line; will it always be the same as sanitize_address($author) would
return, and if not, would you rather compare between sanitized
versions of sender and author, no?
Also, isn't the $sender the same during the whole outer loop that
iterates over @files? Do we need to apply sanitize_address() on it
over and over for each and every logical line in the @header?
This comment also applies to the other patch but they probably
should become a single patch anyway, I guess?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-send-email: fix handling of special characters
Junio C Hamano gits...@pobox.com writes:
+my $sanitized_sender = sanitize_address($sender);
+if (defined $author and $author ne $sanitized_sender) {
$message = From: $author\n\n$message;
if (defined $author_encoding) {
if ($has_content_type) {
...
Also, isn't the $sender the same during the whole outer loop that
iterates over @files? Do we need to apply sanitize_address() on it
over and over for each and every logical line in the @header?
Ahh, I think $sender is constant, but this is not for each @header
line, but done per @file, so it is a much lessor offence than what I
originally thought. The other one does do that inside the loop but
if we have a single copy of $sanitized_sender at the very beginning
that will become a non-issue.
This comment also applies to the other patch but they probably
should become a single patch anyway, I guess?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-remote-mediawiki: better error message when HTTP(S) access fails
My use-case is an invalid SSL certificate. Pulling from the wiki with a
recent version of libwww-perl fails, and git-remote-mediawiki gave no
clue about the reason. Give the mediawiki API detailed error message, and
since it is not so informative, hint the user about an invalid SSL
certificate on https:// urls.
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
contrib/mw-to-git/git-remote-mediawiki.perl | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl
b/contrib/mw-to-git/git-remote-mediawiki.perl
index 9c14c1f..5b6e833 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -292,7 +292,13 @@ sub get_mw_all_pages {
if (!defined($mw_pages)) {
print STDERR fatal: could not get the list of wiki pages.\n;
print STDERR fatal: '$url' does not appear to be a
mediawiki\n;
- print STDERR fatal: make sure '$url/api.php' is a valid
page.\n;
+ if ($url =~ /^https/) {
+ print STDERR fatal: make sure '$url/api.php' is a valid
page\n;
+ print STDERR fatal: and the SSL certificate is correct.\n;
+ } else {
+ print STDERR fatal: make sure '$url/api.php' is a valid
page.\n;
+ }
+ print STDERR error: . $mediawiki-{error}-{details} . \n;
exit 1;
}
foreach my $page (@{$mw_pages}) {
--
1.8.3.rc3.8.g5e49f30
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature Request: existence-only tracking
On Thu, 23 May 2013 12:01:43 +, Brett Trotter wrote: In my work, we have a lot of autogenerated files that need to exist so a script will replace their contents, but tracking the contents creates a lot of unnecessary conflicts. I would love to see an option for a different tracking method that just makes sure a file or directory exists. You could probably do this via the clean/smugde filters via .gitattributes. But really this loooks like suboptimal build design. You should change the generator to not require an (empty/) file beforehand, or change the build process to create those files before invoking the generator. (Likewise empty directories that are neeed in the build should be created by the build, not by the versioning system.) All IMHO. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
John Keeping j...@keeping.me.uk writes: I have to wonder how often git pull with no arguments actually does what users really want (even if they don't know it!) when it doesn't result in a fast-forward (and pull.rebase isn't configured). If you are in a totally centralized shared repository mindset without using topic branch workflow, --first-parent would not help you. In your history the second parent is more likely to be the mainline. So for them git pull that either fast-forward when it can, or makes a merge that records the then-current state of the central shared repository, is perfectly sensible. They will view gitk and see all the changes, git shortlog and git log --no-merges will give them what they expect. Hence my suggestion to error when git pull doesn't result in a fast-forward and no branch name is specified. We could give some advice like: Your local changes are not included in the local branch and you haven't told Git how to preserve them. If you want to rebase your changes onto the modified upstream branch, run: git pull --rebase I can parse the first paragraph above, but cannot make much sense out of it. Unless you are talking about local changes that are not committed yet, that is. But in that case I fail to see what it has to do with the current discussion, or suggestion to use rebase. But people need to realize that it is not solving the other half, a more fundamental problem some people have in their workflow. Yes, but some users don't realise that their workflow is broken, and perhaps we can nudge them in the right direction. I actually avoided mentioning that deliberately, because I think the flip the head when merging encourages people to (1) work directly on 'master' and (2) pull too often when they shouldn't. That is detrimental if your goal is to nudge them in the right direction. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
On Thu, May 23, 2013 at 11:10:32AM -0700, Junio C Hamano wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
On Thu, May 23, 2013 at 5:53 AM, Michael S. Tsirkin m...@redhat.com wrote:
Looks like push can't resolve tags to commits.
Why is that?
linux$ git push -f $PWD v3.10-rc2:refs/heads/vhost-next
Perhaps v3.10-rc2^{}. Yeah, totally and completely not-user-friendly,
More commonly v3.10-rc2^0:vhost-next, if you are truly pushing it
out to a remote repository, but then it invites a puzzlement What
do you plan to do next after pushing? The only reason v3.10-rc2 is
used is because there is not yet a local branch that will host the
vhost-next changes that is built on top of that tag (otherwise you
would be pushing that branch to vhost-next).
But in this particular case, you are force-pushing into the current
repository, and it is spelled much more commonly
git branch -f vhost-next v3.10-rc2
I would think.
That was just a bad example though, I really use it for
push to remove.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
Michael S. Tsirkin m...@redhat.com writes:
On Thu, May 23, 2013 at 11:10:32AM -0700, Junio C Hamano wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
On Thu, May 23, 2013 at 5:53 AM, Michael S. Tsirkin m...@redhat.com
wrote:
Looks like push can't resolve tags to commits.
Why is that?
linux$ git push -f $PWD v3.10-rc2:refs/heads/vhost-next
Perhaps v3.10-rc2^{}. Yeah, totally and completely not-user-friendly,
More commonly v3.10-rc2^0:vhost-next, if you are truly pushing it
out to a remote repository, but then it invites a puzzlement What
do you plan to do next after pushing? The only reason v3.10-rc2 is
used is because there is not yet a local branch that will host the
vhost-next changes that is built on top of that tag (otherwise you
would be pushing that branch to vhost-next).
But in this particular case, you are force-pushing into the current
repository, and it is spelled much more commonly
git branch -f vhost-next v3.10-rc2
I would think.
That was just a bad example though, I really use it for
push to remove.
Then it invites a puzzlement as you can see above.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
On Thu, May 23, 2013 at 11:05:00AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Looks like push can't resolve tags to commits. Why is that? How else would you push a tag out? Well my reaction is, it seems to figure out it needs a commit and then instead of just getting it, it errors out. Why not just DWIM? But at least I see the reason now, thanks. -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
On Thu, May 23, 2013 at 11:10:32AM -0700, Junio C Hamano wrote:
Felipe Contreras felipe.contre...@gmail.com writes:
On Thu, May 23, 2013 at 5:53 AM, Michael S. Tsirkin m...@redhat.com wrote:
Looks like push can't resolve tags to commits.
Why is that?
linux$ git push -f $PWD v3.10-rc2:refs/heads/vhost-next
Perhaps v3.10-rc2^{}. Yeah, totally and completely not-user-friendly,
More commonly v3.10-rc2^0:vhost-next, if you are truly pushing it
out to a remote repository, but then it invites a puzzlement What
do you plan to do next after pushing? The only reason v3.10-rc2 is
used is because there is not yet a local branch that will host the
vhost-next changes that is built on top of that tag (otherwise you
would be pushing that branch to vhost-next).
This was just a bad example.
The real reason is this:
I have a development branch, vhost-next. I have a tag there
like for_linus for things that are ready to go out.
I don't always point it at the tip of the branch
since I might want to rebase/amend X last commits.
It's a tag and not a branch since I need to sign the tag.
I push to a branch and not just the tag since this way
people can track it and do downstream development on top.
So pushing the tag to branch would save me some churn.
But in this particular case, you are force-pushing into the current
repository, and it is spelled much more commonly
git branch -f vhost-next v3.10-rc2
I would think.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-send-email: fix handling of special characters
On Thu, May 23, 2013 at 12:52:11PM -0700, Junio C Hamano wrote:
Michael S. Tsirkin m...@redhat.com writes:
When patch sender's name has special characters,
git send-email did not quote it before matching
against the author name.
As a result it would produce mail like this:
Date: Thu, 23 May 2013 16:36:00 +0300
From: Michael S. Tsirkin m...@redhat.com
To: qemu-de...@nongnu.org
Cc: Michael S. Tsirkin m...@redhat.com
Subject: [PATCH 0/9] virtio: switch to linux headers
Message-Id: 1369316169-20181-1-git-send-email-...@redhat.com
From: Michael S. Tsirkin m...@redhat.com
Fix by sanitizing before matching to patch author name.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
git-send-email.perl | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index bd13cc8..c4dc438 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1400,7 +1400,8 @@ foreach my $t (@files) {
$subject = quote_subject($subject, $auto_8bit_encoding);
}
- if (defined $author and $author ne $sender) {
+ my $sanitized_sender = sanitize_address($sender);
+ if (defined $author and $author ne $sanitized_sender) {
$message = From: $author\n\n$message;
if (defined $author_encoding) {
if ($has_content_type) {
Is $author already sanitized at this point in the code? I see it
was unwrapped with unquote_rfc2047 after it was read from the From:
line; will it always be the same as sanitize_address($author) would
return, and if not, would you rather compare between sanitized
versions of sender and author, no?
Yes. I'll have to look at the code more closely.
In my testing author here is Michael S. Tsirkin m...@redhat.com
so it matches the sanitized sender.
Of course that's because my name does not have non-ascii,
just a dot.
Also, isn't the $sender the same during the whole outer loop that
iterates over @files? Do we need to apply sanitize_address() on it
over and over for each and every logical line in the @header?
This comment also applies to the other patch but they probably
should become a single patch anyway, I guess?
OK so now you are ok with this last bit, right?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
Michael S. Tsirkin m...@redhat.com writes: On Thu, May 23, 2013 at 11:05:00AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Looks like push can't resolve tags to commits. Why is that? How else would you push a tag out? Well my reaction is, it seems to figure out it needs a commit and then instead of just getting it, it errors out. Why not just DWIM? Ahh, that one. The local branch name hierarchy refs/heads/ is special in that you cannot have a tag sitting at the tip, so when push decides to update something under refs/heads/ on the receiving end, it may not be a bad idea to peel it to a commit (and fail if it does not) before creating a pack and telling the other end what the value of the updated tip should be, and I do not think it will hurt anybody. Restriction in the other direction (i.e. if push does not go to refs/tags/, unconditionally unwrap) is a no-no, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
Michael S. Tsirkin m...@redhat.com writes: It's a tag and not a branch since I need to sign the tag. I push to a branch and not just the tag since this way people can track it and do downstream development on top. So pushing the tag to branch would save me some churn. It seems our messages crossed. Everything you said before the last sentence makes sense to me, and if the last sentence were pushing the commit that is tagged to branch, it also makes sense to me. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: push not resolving commit-ish?
On Thu, May 23, 2013 at 02:19:40PM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: On Thu, May 23, 2013 at 11:05:00AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Looks like push can't resolve tags to commits. Why is that? How else would you push a tag out? Well my reaction is, it seems to figure out it needs a commit and then instead of just getting it, it errors out. Why not just DWIM? Ahh, that one. The local branch name hierarchy refs/heads/ is special in that you cannot have a tag sitting at the tip, so when push decides to update something under refs/heads/ on the receiving end, it may not be a bad idea to peel it to a commit (and fail if it does not) before creating a pack and telling the other end what the value of the updated tip should be, and I do not think it will hurt anybody. Yes, that would help my case. Restriction in the other direction (i.e. if push does not go to refs/tags/, unconditionally unwrap) is a no-no, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-send-email: fix handling of special characters
Michael S. Tsirkin m...@redhat.com writes: Is $author already sanitized at this point in the code? I see it was unwrapped with unquote_rfc2047 after it was read from the From: line; will it always be the same as sanitize_address($author) would return, and if not, would you rather compare between sanitized versions of sender and author, no? Yes. I'll have to look at the code more closely. In my testing author here is Michael S. Tsirkin m...@redhat.com so it matches the sanitized sender. Of course that's because my name does not have non-ascii, just a dot. So the conclusion is that the logic to see if the names are the same needs a bit more work than what was posted, I think? Also, isn't the $sender the same during the whole outer loop that iterates over @files? Do we need to apply sanitize_address() on it over and over for each and every logical line in the @header? This comment also applies to the other patch but they probably should become a single patch anyway, I guess? OK so now you are ok with this last bit, right? Sorry, but I am not sure what you are asking. Do I think the assignment to $sanitized_sender can and should be done just once, not once per file, if the code inspection tells us that $sender is a constant inside the foreach (@files) loop? Do I think these two are solving pretty much the same thing and is better to be done in a single patch? I didn't really think them through when I responded, but now after you made me think, I would say the answers to both of them are yes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
On Thu, May 23, 2013 at 11:54 AM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Wed, May 22, 2013 at 11:07 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, May 22, 2013 at 7:08 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: IIRC, git-gui runs two blames, one without any -C and one with (I do not offhand recall how many -C it uses) to show both. 'git blame' is a very expensive operation, perhaps we should add another option so users don't need to run two blames to find this. Yes, you could add a struct origin *suspect_in_the_same_file next to the current struct origin *suspect to the blame_entry in order to represent the origin you would get if you were to stop at a move across files event, and keep digging, when you are operating under -C -C or higher mode. No, this is what I meant: But that would not print the right line numbers, so perhaps: Users of full-output may want to be able to see the same thing. I have a suspicion that you misread what assign_blame() does. The first inner loop to find one suspect is really what it says. It loops over blame entries in the scoreboard but it is not about finding one entry, but is to find one suspect. The ent found in the loop that you store in tmp_ent is no more special than any other ent that shares the same suspect as its origin. It is stored because it's the only structure that has the line numbers. If the blame is passed to another 'ent', the original line numbers are gone. We could manually copy the line numbers to three variables, or we could copy the whole thing to one variable. The rest of the 'ent' doesn't matter. Imagine that your scoreboard originally has three blocks of text (i.e. blame_entry) A, B, and C, and the current suspect for A and C are the same, while we already know what the final guilty party for B is (which may be some descendant of the suspect). I don't see how that's possible, but whatever. Once we find one suspect in the first inner loop, the call to pass_blame() does everything it can do to exonerate that suspect. It runs a single diff between the suspect and each of its parents to find lines in both A and C that can be blamed to one of these parents, and blame entries A and C are split into pieces, some of which still have the same suspect as their origin, which are where their lines originate from, while others are attributable to these parents or their ancestors. If you keep the original entry for A to do something special, like printing what the original range of A was before it was split by pass_blame(), wouldn't you need to do the same for C? tmp_ent is only used when there's no entry taken the guilt away from A. If the blame entry of A is split, and the result of the split has a different filename, found_guilty() would not be called, and thus we won't show the blame entry, and we want to show it. And yes, we would need to do the same for C, and we would once the turn of C comes along. If it's possible that A descendant from A takes the blame for C, then sure, we would need to do ensure C is printed before it's gone, but pass_blame(A) would not destroy C. We will not be visiting C later in the outer while(1) loop, as a single call to pass_blame() for suspect we did when we found it in A has already taken care of it. It took care of A's suspect, but not C. Isn't it possible that C is split further and creates another blame entry? Either way, in --incremental we want to print C as well, whether it's the guilty one or not. I am not sure what you are trying to achieve with that found-guilty logic, even if the save away in tmp_ent logic is changed to keep all the blame entries that have suspect we are looking at as their origin. When the current suspect is found to have passed all lines intact from its parents, we will see found-guilty left to be false. What? When a 'blame entry' passes the whole blame to a parent, found_guilty is false, so we print the entry, which is exactly what we want to do. How would it make the original blame_entry (perhaps halfway) guilty in that situation? I thought 'guilty' meant that it was guilty of adding those lines to the 'final' text, so it ends up in the non-incremental blame. In that sense those blame entries are not guilty. But they are still blame entries, and we want to print all blame entries in incremental, guilty or not. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-send-email: fix handling of special characters
On Thu, May 23, 2013 at 02:27:59PM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Is $author already sanitized at this point in the code? I see it was unwrapped with unquote_rfc2047 after it was read from the From: line; will it always be the same as sanitize_address($author) would return, and if not, would you rather compare between sanitized versions of sender and author, no? Yes. I'll have to look at the code more closely. In my testing author here is Michael S. Tsirkin m...@redhat.com so it matches the sanitized sender. Of course that's because my name does not have non-ascii, just a dot. So the conclusion is that the logic to see if the names are the same needs a bit more work than what was posted, I think? I think so. And a bit more testing with non-ASCII. Plan to look into this around Sunday if no one beats me to it. Also, isn't the $sender the same during the whole outer loop that iterates over @files? Do we need to apply sanitize_address() on it over and over for each and every logical line in the @header? This comment also applies to the other patch but they probably should become a single patch anyway, I guess? OK so now you are ok with this last bit, right? Sorry, but I am not sure what you are asking. Do I think the assignment to $sanitized_sender can and should be done just once, not once per file, if the code inspection tells us that $sender is a constant inside the foreach (@files) loop? Do I think these two are solving pretty much the same thing and is better to be done in a single patch? I didn't really think them through when I responded, but now after you made me think, I would say the answers to both of them are yes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
Felipe Contreras felipe.contre...@gmail.com writes: Imagine that your scoreboard originally has three blocks of text (i.e. blame_entry) A, B, and C, and the current suspect for A and C are the same, while we already know what the final guilty party for B is (which may be some descendant of the suspect). I don't see how that's possible, but whatever. The tree in your latest commit HEAD has a file with block of text A followed by block of text B followed by block of text C. The latest commit was the one that added B (perhaps new lines were inserted, or perhaps existing contiguous block of text was replaced, there is no difference between these two cases). You start digging the history of this file from HEAD. Your scoreboard begins with a single blame-entry that covers all three segments, with its suspect set to HEAD. Then pass_blame() discovers that top and bottom segments are older than this latest commit, and splits the originally-one blame entry into three blame entries. The first and the third blame entries cover the block A and the block C respectively, and their suspect fields are both set to HEAD^. The second blame entry covers the block B and its suspect does not change (it still is HEAD). Then it returns to the caller. The caller of pass_blame() looks at the scoreboard and finds these three blame entries. The second one's supect is still the original suspect the caller asked pass_blame() to exonerate blames for, and the suspect failed to do so for block B. The final blame for the middle block is now known to be HEAD. After all of the above, the next iteration of while(1) loop begins. That is how you arrive to the whatever situation. You have three blame entries, A, B and C, and suspect of A and C are the same, while B has a different suspect. Then in that next iteration, we pick blame entry for A and decide to see if HEAD^, which is the suspect for A, can be exonerated of blames for _any_ (not just A) blame entry it currently is suspected for. We call pass_blame() and it will find and process both A and C with a single git diff-tree, attempting to pass blame to HEAD^^ and its ancestors. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 02:01:39PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: I have to wonder how often git pull with no arguments actually does what users really want (even if they don't know it!) when it doesn't result in a fast-forward (and pull.rebase isn't configured). If you are in a totally centralized shared repository mindset without using topic branch workflow, --first-parent would not help you. In your history the second parent is more likely to be the mainline. So for them git pull that either fast-forward when it can, or makes a merge that records the then-current state of the central shared repository, is perfectly sensible. They will view gitk and see all the changes, git shortlog and git log --no-merges will give them what they expect. Yes, but for people used to a cleaner history it's confusing to see the mainline branch and one small change the wrong way round. When I see people doing this, it's normally something like: ... do some work for several hours... git commit -a git push # fails because it's not a fast forward git pull git push In this scenario, just adding --rebase to git pull actually results in a much more sensible history. Hence my suggestion to error when git pull doesn't result in a fast-forward and no branch name is specified. We could give some advice like: Your local changes are not included in the local branch and you haven't told Git how to preserve them. If you want to rebase your changes onto the modified upstream branch, run: git pull --rebase I can parse the first paragraph above, but cannot make much sense out of it. Unless you are talking about local changes that are not committed yet, that is. But in that case I fail to see what it has to do with the current discussion, or suggestion to use rebase. This isn't about swap parents, it's about helping people realise that just git pull isn't necessarily the best thing for them to do, and that they may want --rebase. So I was asking if it would be sensible (possibly in Git 2.0) to make git-pull pass --ff-only to git-merge by default. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
On Thu, May 23, 2013 at 4:52 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Imagine that your scoreboard originally has three blocks of text (i.e. blame_entry) A, B, and C, and the current suspect for A and C are the same, while we already know what the final guilty party for B is (which may be some descendant of the suspect). I don't see how that's possible, but whatever. The tree in your latest commit HEAD has a file with block of text A followed by block of text B followed by block of text C. The latest commit was the one that added B (perhaps new lines were inserted, or perhaps existing contiguous block of text was replaced, there is no difference between these two cases). You start digging the history of this file from HEAD. Your scoreboard begins with a single blame-entry that covers all three segments, with its suspect set to HEAD. Then pass_blame() discovers that top and bottom segments are older than this latest commit, and splits the originally-one blame entry into three blame entries. The first and the third blame entries cover the block A and the block C respectively, and their suspect fields are both set to HEAD^. The second blame entry covers the block B and its suspect does not change (it still is HEAD). Then it returns to the caller. The caller of pass_blame() looks at the scoreboard and finds these three blame entries. The second one's supect is still the original suspect the caller asked pass_blame() to exonerate blames for, and the suspect failed to do so for block B. The final blame for the middle block is now known to be HEAD. After all of the above, the next iteration of while(1) loop begins. That is how you arrive to the whatever situation. You have three blame entries, A, B and C, and suspect of A and C are the same, while B has a different suspect. Then in that next iteration, we pick blame entry for A and decide to see if HEAD^, which is the suspect for A, can be exonerated of blames for _any_ (not just A) blame entry it currently is suspected for. We call pass_blame() and it will find and process both A and C with a single git diff-tree, attempting to pass blame to HEAD^^ and its ancestors. All right, my code still works in that situation. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 4:55 PM, John Keeping j...@keeping.me.uk wrote: So I was asking if it would be sensible (possibly in Git 2.0) to make git-pull pass --ff-only to git-merge by default. Definitely yes. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git stash deletes/drops changes of
Jim Greenleaf james.a.greenl...@gmail.com writes:
Adeodato Simó dato at net.com.org.es writes:
I was unpleasantly surprised to discover yesterday that doing `git
stash` on a repository where I had previously run `git update-index
--assume-unchanged FOO` completely lost all changes I had in file FOO.
I just ran into this today.
Was a decision about this behavior reached in the intervening time?
When you mark a file assume-unchanged, git internally sets a flag that
this file should not be considered when doing cache refreshes -- the
file is always assumed to be up-to-date.
So while I haven't actually looked into all of the code, I imagine it
goes something like this:
* git-stash uses git update-index --all on all modified files. But it
doesn't show up as modified, because you promised it isn't.
* Later it calls git reset --hard, which blows away the existing state.
This would seem to ignore the assume-unchanged flag in this case, as
otherwise it wouldn't overwrite it.
Whether the last behavior is a bug is in the eye of the beholder. In
your case you apparently lost work. However, 'git reset --hard' in
itself should discard all uncommitted work without asking any further
questions (because it's --hard). So the bug is then in the sequence
ask about uncommitted work
save it elsewhere
git reset --hard
assuming that this actually makes sure nothing gets lost. But the only
thing that was lost was *files that you promised would not be changed*.
What's really unfortunate is that we caused this in the first place by
Pasky's 6259ac6 (Documentation: How to ignore local changes in tracked
files, 2008-07-18). It recommends exactly the --assume-unchanged
strategy to ignore changes to tracked files.
And it's hard to disagree with its commit message:
This is currently probably one of the top FAQs at #git and the
--assume-unchanged switch is not widely known
Except that now the corresponding FAQ is that we have to actively
dissuade people from using --assume-unchanged precisely because it keeps
biting people.
So maybe it would be time to first make up our minds as to what
--assume-unchanged should actually mean:
* Ignore changes to a tracked file, but treat them as valuable. In
this case we'd have to make sure that failures like git-stash's are
handled properly.
* Ignore changes to a tracked file, as in who cares if it was changed.
* A very specific optimization for users who know what they are doing.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
John Keeping j...@keeping.me.uk writes: This isn't about swap parents, it's about helping people realise that just git pull isn't necessarily the best thing for them to do, and that they may want --rebase. So I was asking if it would be sensible (possibly in Git 2.0) to make git-pull pass --ff-only to git-merge by default. Unless your primary user base is those who use Git as a deployment tool to always follow along the tip of some external repository without doing anything on your own on the branch you run your git pull on, defaulting it to --ff-only does not make much sense to me. If the proposal were to make pull.rebase the default at a major version bump and force all integrators and other people who are happy with how pull = fetch + merge (not fetch + rebase) works to say pull.rebase = false in their configuration, I think I can see why some people may think it makes sense, though. But neither is an easy sell, I would imagine. It is not about passing me, but about not hurting users like kernel folks we accumulated over 7-8 years. Also rebase of the branch you attempted to push out is sometimes a good solution (fixing just a small change on 'master' that was beaten by somebody else pushing first), but is a bad workaround (you had many changes on that branch, which would have been better if they were done on a topic branch, but you do not want to merge with the upstream because you worked on 'master') some other times, so I have this suspicion that 'pull.rebase' is not necessarily a good thing to encourage in the first place. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
Felipe Contreras felipe.contre...@gmail.com writes: Then in that next iteration, we pick blame entry for A and decide to see if HEAD^, which is the suspect for A, can be exonerated of blames for _any_ (not just A) blame entry it currently is suspected for. We call pass_blame() and it will find and process both A and C with a single git diff-tree, attempting to pass blame to HEAD^^ and its ancestors. All right, my code still works in that situation. When HEAD^ was processed, pass_blame() finds that the first line in A is attributable to HEAD^ (our current suspect) while the rest were copied from a different file in HEAD^^ . All lines in C are found to be copy from a different file in HEAD^^. Then your scoreboard would have: 1. a blame entry that failed to pass blame to parents of HEAD^ (the first line in A), which still has suspect == HEAD^ 2. a blame entry that covers the remainder of A, blaming HEAD^^. 3. a blame entry that covers all of B, whose final guilty party is known already. 4. a blame entry that covers all of C, blaming a different file in HEAD^^. Your Take responsibility loop goes over these blame entries, sets found_guilty to true because of the first blame entry (the first line of A), and calls print_guilty_entry() for blame entry 1, showing that HEAD^ is guilty for the first line of A. After the loop, your if we did not find anybody guilty logic does not kick in, and the original line range for block A you saved in tmp_ent is not used. You lost the fact that the second and remainder of A were in this file at the commit HEAD^ but not in HEAD^^ (i.e. these lines were moved by HEAD^ from elsewhere). The fact that HEAD^ touched _something_ is not lost, so if _all_ you care about is list all the commits, but I do not care about what line range was modified how, you can claim it working, but that is a very limited definition of working and is not very reusable or extensible in order to help those like gitk that currently have to run two separate blames. They need an output that lets them tell between - this is the earliest we saw these lines in the path (it may have been copied from another path, but this entry in the incremental output stream is not about that final blame); and - this is the final blame where these lines came from, possibly from different path and coalesce both kind of origin.. Also the fact that the entire C was copied from elsewhere by HEAD^ is lost but that is the same issue. The next round will not find any blame entry that is !ent-guity because the call to pass_blame() for HEAD^ already handled the final blame not just for blame entries 1 and 2, but also for 4 during this round. If the change in HEAD^ in the above example were to copy the whole A and C from a different file, then your !found_guilty logic would kick in and say all lines of A where copied from elsewhere in HEAD^, but again we would not learn the same information for C. I do not think I care only about a single bit per commit, i.e. if the commit touched the path; screw everybody else who wants to actually know where each line came from can be called working. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 5:11 PM, Junio C Hamano gits...@pobox.com wrote: John Keeping j...@keeping.me.uk writes: This isn't about swap parents, it's about helping people realise that just git pull isn't necessarily the best thing for them to do, and that they may want --rebase. So I was asking if it would be sensible (possibly in Git 2.0) to make git-pull pass --ff-only to git-merge by default. Unless your primary user base is those who use Git as a deployment tool to always follow along the tip of some external repository without doing anything on your own on the branch you run your git pull on, defaulting it to --ff-only does not make much sense to me. A lot of people do stuff, but the rebase it. If the proposal were to make pull.rebase the default at a major version bump and force all integrators and other people who are happy with how pull = fetch + merge (not fetch + rebase) works to say pull.rebase = false in their configuration, I think I can see why some people may think it makes sense, though. That makes perfect sense, because the people that are not familiar with Git more often than not end up making merges by mistake, and the ones that are familiar with it can easily configure it to do what they want, or just 'git pull --merge', or 'git fetch'+'git merge' (we should make merge.defaulttoupstream=true as well). But neither is an easy sell, I would imagine. It is not about passing me, but about not hurting users like kernel folks we accumulated over 7-8 years. I've worked in the Linux kernel, and in my experience the vast vast majority of kernel developers don't do merges; they send patches. It's only the lieutenants that might do that, and although there are a lot, they don't surpass the 200, and they most definitely know how to configure Git to do what they need. And even then, most of them don't do merges, but create a linear history for Linus to merge. So the only one who does really rely on merges is Linus, I think he would have no problems configuring Git. It is also my experience that most people don't do 'git pull', because it rarely does what one wants; 'upstream' is still too cumbersome for most people. Also rebase of the branch you attempted to push out is sometimes a good solution (fixing just a small change on 'master' that was beaten by somebody else pushing first), but is a bad workaround (you had many changes on that branch, which would have been better if they were done on a topic branch, but you do not want to merge with the upstream because you worked on 'master') some other times, so I have this suspicion that 'pull.rebase' is not necessarily a good thing to encourage in the first place. Too bad, that's what most people recommend; 'git fetch'+'git rebase'. That's the only way newcomers can avoid the ugliness of 'upstream', and avoid making atrocious merges. It's silly that the people familiar with Git has to explain this to each and every newcomer. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git stash deletes/drops changes of
Thomas Rast tr...@inf.ethz.ch writes: So maybe it would be time to first make up our minds as to what --assume-unchanged should actually mean: * Ignore changes to a tracked file, but treat them as valuable. In this case we'd have to make sure that failures like git-stash's are handled properly. * Ignore changes to a tracked file, as in who cares if it was changed. * A very specific optimization for users who know what they are doing. It has always been a promise the user makes to Git that the working tree files that are marked as such will be kept identical to what is in the index (hence there is no need for Git to check if they were modified). And by extension, Git is now free to choose reading from the working tree file when asked to read from blob object recorded in the index for that path, or vice versa, because of that promise. It is not --ignore-changes bit, and has never been. What are the workflows that are helped if we had such a bit? If we need to support them, I think you need a real --ignore-changes bit, not an abuse of --assume-unchanged. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
Felipe Contreras felipe.contre...@gmail.com writes: On Thu, May 23, 2013 at 5:11 PM, Junio C Hamano gits...@pobox.com wrote: John Keeping j...@keeping.me.uk writes: This isn't about swap parents, it's about helping people realise that just git pull isn't necessarily the best thing for them to do, and that they may want --rebase. So I was asking if it would be sensible (possibly in Git 2.0) to make git-pull pass --ff-only to git-merge by default. Unless your primary user base is those who use Git as a deployment tool to always follow along the tip of some external repository without doing anything on your own on the branch you run your git pull on, defaulting it to --ff-only does not make much sense to me. A lot of people do stuff, but the rebase it. If I am parsing the above properly, I think that is only saying that pull --rebase makes sense for people who do real work, which I am not disagreeing. If the proposal were to make pull.rebase the default at a major version bump and force all integrators and other people who are happy with how pull = fetch + merge (not fetch + rebase) works to say pull.rebase = false in their configuration, I think I can see why some people may think it makes sense, though. That makes perfect sense, because the people that are not familiar with Git more often than not end up making merges by mistake, and the ones that are familiar with it can easily configure it to do what they want Yes, in theory. The transition needs a major version bump, but it is doable (with unknown level of resistance). But neither is an easy sell, I would imagine. It is not about passing me, but about not hurting users like kernel folks we accumulated over 7-8 years. I've worked in the Linux kernel, and in my experience the vast vast majority of kernel developers don't do merges; they send patches. It's only the lieutenants that might do that, and although there are a lot, they don't surpass the 200, and they most definitely know how to configure Git to do what they need. And even then, most of them don't do merges, but create a linear history for Linus to merge. So the only one who does really rely on merges is Linus, I think he would have no problems configuring Git. That is not something I can agree or disagree without looping somebody whose judgement I can trust from the kernel circle ;-). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git stash deletes/drops changes of
Junio C Hamano gits...@pobox.com writes:
Thomas Rast tr...@inf.ethz.ch writes:
So maybe it would be time to first make up our minds as to what
--assume-unchanged should actually mean:
* Ignore changes to a tracked file, but treat them as valuable. In
this case we'd have to make sure that failures like git-stash's are
handled properly.
* Ignore changes to a tracked file, as in who cares if it was changed.
* A very specific optimization for users who know what they are doing.
It has always been a promise the user makes to Git that the working
tree files that are marked as such will be kept identical to what is
in the index (hence there is no need for Git to check if they were
modified). And by extension, Git is now free to choose reading from
the working tree file when asked to read from blob object recorded
in the index for that path, or vice versa, because of that promise.
It is not --ignore-changes bit, and has never been. What are the
workflows that are helped if we had such a bit? If we need to
support them, I think you need a real --ignore-changes bit, not
an abuse of --assume-unchanged.
I gather -- from #git -- that it's mostly used for config files, which
have an annoying habit of being different from the repository.
Which is wrong, really. But we still claim that --assume-unchanged is a
solution to it in git-update-index(1).
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] Add new git-related helper to contrib
On Thu, May 23, 2013 at 5:44 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Then in that next iteration, we pick blame entry for A and decide to see if HEAD^, which is the suspect for A, can be exonerated of blames for _any_ (not just A) blame entry it currently is suspected for. We call pass_blame() and it will find and process both A and C with a single git diff-tree, attempting to pass blame to HEAD^^ and its ancestors. All right, my code still works in that situation. When HEAD^ was processed, pass_blame() finds that the first line in A is attributable to HEAD^ (our current suspect) while the rest were copied from a different file in HEAD^^ . All lines in C are found to be copy from a different file in HEAD^^. Then your scoreboard would have: 1. a blame entry that failed to pass blame to parents of HEAD^ (the first line in A), which still has suspect == HEAD^ 2. a blame entry that covers the remainder of A, blaming HEAD^^. 3. a blame entry that covers all of B, whose final guilty party is known already. 4. a blame entry that covers all of C, blaming a different file in HEAD^^. Your Take responsibility loop goes over these blame entries, sets found_guilty to true because of the first blame entry (the first line of A), and calls print_guilty_entry() for blame entry 1, showing that HEAD^ is guilty for the first line of A. After the loop, your if we did not find anybody guilty logic does not kick in, and the original line range for block A you saved in tmp_ent is not used. You lost the fact that the second and remainder of A were in this file at the commit HEAD^ but not in HEAD^^ (i.e. these lines were moved by HEAD^ from elsewhere). The fact that HEAD^ touched _something_ is not lost, so if _all_ you care about is list all the commits, but I do not care about what line range was modified how, you can claim it working, The line range printed is correct. but that is a very limited definition of working and is not very reusable or extensible in order to help those like gitk that currently have to run two separate blames. They need an output that lets them tell between - this is the earliest we saw these lines in the path (it may have been copied from another path, but this entry in the incremental output stream is not about that final blame); and - this is the final blame where these lines came from, possibly from different path and coalesce both kind of origin.. They can already do that by looking at the lines; if they get replaced by another entry; they are not guilty. Or we can add a 'guilty' line to the incremental output, that's trivial. Also the fact that the entire C was copied from elsewhere by HEAD^ is lost but that is the same issue. The next round will not find any blame entry that is !ent-guity because the call to pass_blame() for HEAD^ already handled the final blame not just for blame entries 1 and 2, but also for 4 during this round. No, that's not true. If it's a different file the blame entry is not found guilty. If the change in HEAD^ in the above example were to copy the whole A and C from a different file, then your !found_guilty logic would kick in and say all lines of A where copied from elsewhere in HEAD^, but again we would not learn the same information for C. We would, when it's the turn for C, which is not guilty at this point. I do not think I care only about a single bit per commit, i.e. if the commit touched the path; screw everybody else who wants to actually know where each line came from can be called working. They can know where each line came from; the lines of each entry are printed properly; that's the whole point of 'tmp_ent'. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 5:54 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Thu, May 23, 2013 at 5:11 PM, Junio C Hamano gits...@pobox.com wrote: John Keeping j...@keeping.me.uk writes: This isn't about swap parents, it's about helping people realise that just git pull isn't necessarily the best thing for them to do, and that they may want --rebase. So I was asking if it would be sensible (possibly in Git 2.0) to make git-pull pass --ff-only to git-merge by default. Unless your primary user base is those who use Git as a deployment tool to always follow along the tip of some external repository without doing anything on your own on the branch you run your git pull on, defaulting it to --ff-only does not make much sense to me. A lot of people do stuff, but the rebase it. If I am parsing the above properly, I think that is only saying that pull --rebase makes sense for people who do real work, which I am not disagreeing. You claimed that 'git pull' (--ff-only) only makes sense if the primary user-base doesn't do any work on it, but that's not true; they can do a 'git rebase' after such pull (or a merge). We don't have to assume our primary user-base wants to do full fledged merges, in fact, such assumption would be wrong. If the proposal were to make pull.rebase the default at a major version bump and force all integrators and other people who are happy with how pull = fetch + merge (not fetch + rebase) works to say pull.rebase = false in their configuration, I think I can see why some people may think it makes sense, though. That makes perfect sense, because the people that are not familiar with Git more often than not end up making merges by mistake, and the ones that are familiar with it can easily configure it to do what they want Yes, in theory. The transition needs a major version bump, but it is doable (with unknown level of resistance). Isn't that what wer are discussing here? But neither is an easy sell, I would imagine. It is not about passing me, but about not hurting users like kernel folks we accumulated over 7-8 years. I've worked in the Linux kernel, and in my experience the vast vast majority of kernel developers don't do merges; they send patches. It's only the lieutenants that might do that, and although there are a lot, they don't surpass the 200, and they most definitely know how to configure Git to do what they need. And even then, most of them don't do merges, but create a linear history for Linus to merge. So the only one who does really rely on merges is Linus, I think he would have no problems configuring Git. That is not something I can agree or disagree without looping somebody whose judgement I can trust from the kernel circle ;-). See section 16) in Documentation/SubmittingPatches, notice how the whole section is written with Linus in mind. Some maintainers do have sub-maintainers that send pull requests to them, not Linus, but they are the minority. But most definitely pull requests are not for the general population (except in a few very rare exceptions maybe). -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git stash deletes/drops changes of
Thomas Rast tr...@inf.ethz.ch writes: What are the workflows that are helped if we had such a bit? If we need to support them, I think you need a real --ignore-changes bit, not an abuse of --assume-unchanged. I gather -- from #git -- that it's mostly used for config files, which have an annoying habit of being different from the repository. Which is wrong, really. But we still claim that --assume-unchanged is a solution to it in git-update-index(1). That is doubly wrong, then ;-) How would we want to proceed from here? The obvious first step would be to fix the documentation, but then what is next? Thinking aloud, ignoring that Which is wrong, really part in your message and assuming that we do want to support --ignore-changes Can the way we handle --ignore-changes files be a strict superset (or is it subset?) of what we currently do for --assume-unchanged? That is, if we fix^Wchange the behaviour of --assume-unchanged to be less aggressive in assuming that the user kept his promise, can we get --ignore-changes without losing much of the performance benefit of --assume-unchanged the people who originally wanted to have that feature have enjoyed for all these years? If you are working on a project with a large working tree, by marking paths in one directory you do not care about (and do not use) with the --assume-unchanged bit, checking out another branch can be done without inspecting if there are uncommitted changes in the part of the working tree that may be clobbered with the different version of the file in the other branch. That has to go for --ignore-changes, for example. Are there others that need to suffer? If so, these two have to be done as totally independent options, but if -ignore-changes can be just a slightly less agressive -assume-unchanged, we could fix --assume-unchanged, introduce --ignore-changes as a synonym and be done with it. I highly doubt that is doable. The only sensible way forward, it seems to me, is introduce a proper --ignore-changes that is independent from --assume-unchanged. What does --ignore-changes really mean? The end user does not want to see changes to a config file when he runs git status and git diff. I think git commit -a would ignore the local changes to the configuration file as a natural consequence if we teach git status to ignore paths marked with the --ignore-changes bit. But the same git diff (between the index and the working tree) logic is internally used to decide if a path has local changes when running git checkout to check out another branch, git rebase to see if there are local changes, etc. and the user do want to view the paths as modified. I am not so sure if there is a clear semantics other than an unactionable blanket statement ignore local changes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
Felipe Contreras felipe.contre...@gmail.com writes: Unless your primary user base is those who use Git as a deployment tool to always follow along the tip of some external repository without doing anything on your own on the branch you run your git pull on, defaulting it to --ff-only does not make much sense to me. A lot of people do stuff, but the rebase it. If I am parsing the above properly, I think that is only saying that pull --rebase makes sense for people who do real work, which I am not disagreeing. You claimed that 'git pull' (--ff-only) only makes sense if the primary user-base doesn't do any work on it, but that's not true; they can do a 'git rebase' after such pull (or a merge). Either you misread what I wrote or I was unclear. I really meant anything on your own *ON* THE BRANCH YOU RUN your git pull on. With git checkout frotz ; git pull --ff-only you do not do anything on frotz other than following along. You can of course commit, rebase and all others on other branches like xyzzy and push them out directly. But you cannot even do this once git checkout frotz; git merge xyzzy if you expect the next git checkout frotz; git pull --ff-only will keep working usefuly. We don't have to assume our primary user-base wants to do full fledged merges, in fact, such assumption would be wrong. I think we are in agreement on that point already. An assumption that people who do merges are somehow more well versed in Git and are more capable than others to configure their repository or they will not be annoyed if you asked them a single configuration change is also wrong, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--follow is ignored when used with --reverse
Hello! This [has been reported][1] to this list about half a year ago but with no response so I'm not even sure if it's been acknowledged as bug. [1]: http://marc.info/?l=gitm=135215709307126q=raw When I use `git log --follow file` all is OK, but once I add `--reverse` to it, it no longer follows the file beyond renames. This makes it hard to query for when the file was really added, which I was trying to achieve with $ git -1 --reverse --follow several_times_renamed_file Is this going to be fixed? Thanks, aL. -- Alois Mahdal -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 6:26 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Unless your primary user base is those who use Git as a deployment tool to always follow along the tip of some external repository without doing anything on your own on the branch you run your git pull on, defaulting it to --ff-only does not make much sense to me. A lot of people do stuff, but the rebase it. If I am parsing the above properly, I think that is only saying that pull --rebase makes sense for people who do real work, which I am not disagreeing. You claimed that 'git pull' (--ff-only) only makes sense if the primary user-base doesn't do any work on it, but that's not true; they can do a 'git rebase' after such pull (or a merge). Either you misread what I wrote or I was unclear. I really meant anything on your own *ON* THE BRANCH YOU RUN your git pull on. With git checkout frotz ; git pull --ff-only you do not do anything on frotz other than following along. You can of course commit, rebase and all others on other branches like xyzzy and push them out directly. But you cannot even do this once git checkout frotz; git merge xyzzy if you expect the next git checkout frotz; git pull --ff-only will keep working usefuly. Unless you rebase. We could of course have a 'branch.name.allow_merge' configuration that gets automatically turned on the first time a 'git merge' is executed, but I feel that creates more inconsistency. We don't have to assume our primary user-base wants to do full fledged merges, in fact, such assumption would be wrong. I think we are in agreement on that point already. An assumption that people who do merges are somehow more well versed in Git and are more capable than others to configure their repository or they will not be annoyed if you asked them a single configuration change is also wrong, though. s/people who do merges/people who should do merges/ And no, it's not wrong. People who do merges should know what they are doing. The alternatives are these: a) you annoy the vast majority of the user-base by making 'git pull' a dangerous operation that should be avoided, and replaced with 'git fetch'+'git rebase'. b) you annoy a minority of the user-base by making 'git pull' not do the merge the expected, so they have to do +'git merge' (which is already less of a change than a)), or configure the default (which they most likely are able to do, if they did intent to do a merge). b) is clearly superior. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 3:11 PM, Junio C Hamano gits...@pobox.com wrote: If the proposal were to make pull.rebase the default at a major version bump and force all integrators and other people who are happy with how pull = fetch + merge (not fetch + rebase) works to say pull.rebase = false in their configuration, I think I can see why some people may think it makes sense, though. But neither is an easy sell, I would imagine. It is not about passing me, but about not hurting users like kernel folks we accumulated over 7-8 years. It would be a *horrible* mistake to make rebase the default, because it's so much easier to screw things up that way. That said, making no-ff the default, and then if that fails, saying The pull was not a fast-forward pull, please say if you want to merge or rebase. Use either git pull --rebase git pull --merge You can also use git config pull.merge true or git config pull.rebase true to set this once for this project and forget about it. That way, people who want the existing behavior could just do that git config pull.merge true once, and they'd not even notice. Hmm? Better yet, make it per-branch. Linus -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
