* Re: Python extension commands in git - request for policy change
From: Guillaume DE BURE @ 2012-11-27 22:01 UTC (permalink / raw)
To: David Aguilar
Cc: Sitaram Chamarty, Felipe Contreras, esr, Nguyen Thai Ngoc Duy,
git, msysGit
In-Reply-To: <CAJDDKr7r5iP_LpXAT9Xz35GOfbDuDxSAKUvx=4dxa2LE_GLgrA@mail.gmail.com>
Le mardi 27 novembre 2012 02:51:04 David Aguilar a écrit :
> On Tue, Nov 27, 2012 at 1:17 AM, Sitaram Chamarty <sitaramc@gmail.com>
wrote:
> > On Tue, Nov 27, 2012 at 1:24 PM, David Aguilar <davvid@gmail.com> wrote:
> >> *cough* git-cola *cough*
> >>
> >> it runs everywhere. Yes, windows too. It's written in python.
> >> It's been actively maintained since 2007.
> >>
> >> It's "modern" and has features that don't exist anywhere else.
> >>
> >> It even has tests. It even comes with a building full of willing
> >> guinea-pigs^Wtesters that let me know right away when
> >> anything goes wrong.
> >>
> >> It uses Qt but that's really the whole point of Qt -> cross-platform.
> >> (not sure how that wiki page ended up saying Gnome/GTK?)
> >>
> >> The DAG aka git-dag (in its master branch, about to be released)
> >> is nicer looking then gitk IMO. gitk still has some features
> >> that are better too--there's no silver bullet, but the delta
> >> is pretty small.
> >
> > Gitk does a lot of things that people don't realise, since they're not
> > really documented and you have to scrounge around on the UI. The
> > thing is, it's just about the most awesome tool for code archeology I
> > have seen.
> >
> > I realise (from looking at the doc page) that git-cola helps you do
> > all sorts of things, but those are all things I am happier doing at
> > the command line.
>
> Ditto. There's actually a few small things I use it for,
> mainly for teasing apart commits. These days you can use git-gui
> for that, but in the old days it was the only way to interactively
> select individual lines and stage/unstage/revert them, etc.
> I don't think we can line-by-line revert in git-gui yet, though.
>
> Some other small things that I use: ctrl-g, type something
> for grep, hit enter twice and I'm in my editor on that
> (or any other selected) line. 'spacebar' does xdg-open,
> and 'enter' launches the editor in the status widget;
> small things. I, too, do most stuff on the command line.
>
> The grep thing is a good example. You have tons of output,
> you see the one line that you care about, and you want to jump
> there. Clicking on that line and hitting enter is the minimal
> effort to do that. You don't have to click because we also
> have keyboard navigation. I have a feeling that there's probably
> something I'm missing, though.. another way of working (emacs?)
> that would render all of this custom GUI stuff pointless.
>
> What I learned about users:
>
> The commit editor is the #1 thing that got my coworkers finally
> writing better commit messages. It forces the subject/description
> separation and shows yellow, red when the subject gets too long.
> It also auto-wraps. IMO it makes sense for git-gui to do
> the same these days.
>
> > Gitk does precisely those things which *require* a GUI, where the
> > amount of information presented overwhelms a text interface. The
> > display is concisely designed to give you the maximum information at a
> > minimum space use. For example, a little black square when a commit
> > has a note attached. Even hovering over the arrow-heads, on complex
> > trees where the line gets broken, does something meaningful.
> >
> > if I had to pin it down, the feature I use most often is "Show origin
> > of this line". Other features I use often are
> >
> > - review a commit file by file (f and b keys, also spacebar and 'd')
> > - search by SHA1 (4 digits appear to be enough, regardless of how
> >
> > big your repo is),
> >
> > - search for commits changing path/dir (while still showing all the
> >
> > commits; i.e., this is not 'git-dag -- README.txt' but within gitk you
> > search up and down for commits touching README.txt
> >
> > - and navigating the commit tree looking for stuff
> >
> > http://sitaramc.github.com/1-basic-usage/gitk.html is my attempt to
> > document some of the stuff I have found and use.
>
> Wow, this is awesome.
>
> > One final point: the DAG on the right wastes enormous amounts of
> > space. Purely subjectively, it is almost jarring on the senses. (If
> > you reduce it, it becomes unreadable).
> >
> > With all due respect, git-cola/dag isn't anywhere near what gitk does,
> > at least for people who are not afraid of the command line and only
> > need the GUI to visualise a truly complex tree.
>
> This is really great feedback.
> cc:ing Guillaume since he had similar ideas.
>
> thx,
Thanks David for cc:ing me. I'm sorry I was quite below the radar lately, had
too much work to get back on the dag. What I would really like for the dag is
to become as useful as the treeview in git extensions
(http://code.google.com/p/gitextensions/), where it is the central place for
checkout, merge, cherry picks...
My only complaint with git extensions is that it is poorly integrated on my
linux desktop, due to framework choice (let's not start a flame war here, it's
not the point). On the other hand, git-cola is quite easy to hack on thanks to
its python roots, well integrated on all OS thanks to Qt, so I thought it
would be great to make it at least on par.
Had an opportunity to rework the visuals on the dag, but not yet its
functionalities... As soon as I'm back on normal life, I'll pickup the subject
again :)
Cheers,
Guillaume
--
Skrooge, a free, Open Source, personal finances software for linux, Mac OS,
Windows
http://skrooge.org
^ permalink raw reply
* RE: Millisecond precision in timestamps?
From: Pyeron, Jason J CTR (US) @ 2012-11-27 21:44 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <20121127204828.577264065F@snark.thyrsus.com>
[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]
> -----Original Message-----
> From: Eric S. Raymond
> Sent: Tuesday, November 27, 2012 3:48 PM
>
> Because I do a lot of work on repository conversion tools, I've had
> to learn a lot of detail about ontological mismatches between
> version-control systems - especially places where you lose metadata
> moving between them.
>
> In general, git metadata can carry forward almost all the metadata in
> a Subversion repository. Among the handful of minor exceptions (empty
> directories, flow structure, certain kinds of mergeinfos) there is one
> that stands out because it seems to be an implementation detail rather
> than a consequence of fundamentally different design decisions.
>
> I refer to the one-second precision of git timestamps. Subversion
> stores its commit and property-change timestamps to microsecond
> precision; conversion tools have to throw the subsecond part of
> this information away.
>
> Has going to timestamps with the full precision of the system clock
> been considered and rejected, or am I the first to bring this up?
>
> If I were to write refactoring patches that treated "timestamp" as
> an ADT, with a view towards hiding the difference between int and
> float timestamps and eventually experimenting with float ones,
Do you really mean floating point numbers with approximate imprecise values?
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5615 bytes --]
^ permalink raw reply
* Re: Millisecond precision in timestamps?
From: Shawn Pearce @ 2012-11-27 21:41 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121127204828.577264065F@snark.thyrsus.com>
On Tue, Nov 27, 2012 at 12:48 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Because I do a lot of work on repository conversion tools, I've had
> to learn a lot of detail about ontological mismatches between
> version-control systems - especially places where you lose metadata
> moving between them.
>
> In general, git metadata can carry forward almost all the metadata in
> a Subversion repository. Among the handful of minor exceptions (empty
> directories, flow structure, certain kinds of mergeinfos) there is one
> that stands out because it seems to be an implementation detail rather
> than a consequence of fundamentally different design decisions.
>
> I refer to the one-second precision of git timestamps. Subversion
> stores its commit and property-change timestamps to microsecond
> precision; conversion tools have to throw the subsecond part of
> this information away.
>
> Has going to timestamps with the full precision of the system clock
> been considered and rejected, or am I the first to bring this up?
>
> If I were to write refactoring patches that treated "timestamp" as
> an ADT, with a view towards hiding the difference between int and
> float timestamps and eventually experimenting with float ones,
> would they be accepted?
JGit would fortunately ignore a floating point timestamp specification
if given in a commit, but I don't know about other Git
implementations... like say git. :-)
^ permalink raw reply
* Re: [PATCH 3/6] Fix grammar
From: Max Horn @ 2012-11-27 21:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8v9mn5k6.fsf@alter.siamese.dyndns.org>
On 27.11.2012, at 21:39, Junio C Hamano wrote:
> Max Horn <max@quendi.de> writes:
>
>> Subject: Re: [PATCH 3/6] Fix grammar
>
> Please run "git shortlog -200 --no-merges" from the tip of your
> topic branch before sending a series out, and see if you can
> immediately identify what area each of your patches affects.
First off: I apologize for the inconvenience my stumbling causes :-(, and I'll try to learn and do better next time I send this or another series.
In this particular case, I am not 100% sure if I understood you correctly, i.e. what exactly you are trying to tell me. Is it (in a nutshell) that the subject lines of my commits suck, as they don't identify which area of code they touch? At least that's the thing I notice when looking at that shortlog... Bad, of course...
If this is indeed it, would a commit message like
git-remote-helper.txt: minor grammar fix
be OK? If this is indeed it, I'll be happy to reroll and resubmit the series accordingly. If I am mistaken and something else / more is wrong, please be so kind and let me know, too.
Sorry once more and thank you very much for your feedback,
Max
>
>> Signed-off-by: Max Horn <max@quendi.de>
>> ---
>> Documentation/git-remote-helpers.txt | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
>> index db63541..7eb43d7 100644
>> --- a/Documentation/git-remote-helpers.txt
>> +++ b/Documentation/git-remote-helpers.txt
>> @@ -235,9 +235,9 @@ Commands are given by the caller on the helper's standard input, one per line.
>> 'capabilities'::
>> Lists the capabilities of the helper, one per line, ending
>> with a blank line. Each capability may be preceded with '*',
>> - which marks them mandatory for git version using the remote
>> - helper to understand (unknown mandatory capability is fatal
>> - error).
>> + which marks them mandatory for git versions using the remote
>> + helper to understand. Any unknown mandatory capability is a
>> + fatal error.
>>
>> 'list'::
>> Lists the refs, one per line, in the format "<value> <name>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH 4/5] diff --stat: move the "total count" logic to the last loop
From: Junio C Hamano @ 2012-11-27 21:21 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com>
The diffstat generation logic, with --stat-count limit, is
implemented as three loops.
- The first counts the width necessary to show stats up to
specified number of entries, and notes up to how many entries in
the data we need to iterate to show the graph;
- The second iterates that many times to draw the graph, adjusts
the number of "total modified files", and counts the total
added/deleted lines for the part that was shown in the graph;
- The third iterates over the remainder and only does the part to
count "total added/deleted lines" and to adjust "total modified
files" without drawing anything.
Move the logic to count added/deleted lines and modified files from
the second loop to the third loop.
This incidentally fixes a bug. The third loop was not filtering
binary changes (counted in bytes) from the total added/deleted as it
should. The second loop implemented this correctly, so if a binary
change appeared earlier than the --stat-count cutoff, the code
counted number of added/deleted lines correctly, but if it appeared
beyond the cutoff, the number of lines would have mixed with the
byte count in the buggy third loop.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 21 ++++++++++++---------
t/t4049-diff-stat-count.sh | 2 +-
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/diff.c b/diff.c
index e4e70e5..4105260 100644
--- a/diff.c
+++ b/diff.c
@@ -1498,7 +1498,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
if (max_change < change)
max_change = change;
}
- count = i; /* min(count, data->nr) */
+ count = i; /* where we can stop scanning in data->files[] */
/*
* We have width = stat_width or term_columns() columns total.
@@ -1592,10 +1592,9 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
uintmax_t deleted = file->deleted;
int name_len;
- if (!file->is_interesting && (added + deleted == 0)) {
- total_files--;
+ if (!file->is_interesting && (added + deleted == 0))
continue;
- }
+
/*
* "scale" the filename
*/
@@ -1640,8 +1639,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
*/
add = added;
del = deleted;
- adds += add;
- dels += del;
if (graph_width <= max_change) {
int total = add + del;
@@ -1667,7 +1664,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
show_graph(options->file, '-', del, del_c, reset);
fprintf(options->file, "\n");
}
- for (i = count; i < data->nr; i++) {
+
+ for (i = 0; i < data->nr; i++) {
struct diffstat_file *file = data->files[i];
uintmax_t added = file->added;
uintmax_t deleted = file->deleted;
@@ -1675,8 +1673,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
total_files--;
continue;
}
- adds += added;
- dels += deleted;
+
+ if (!file->is_binary && !file->is_unmerged) {
+ adds += added;
+ dels += deleted;
+ }
+ if (i < count)
+ continue;
if (!extra_shown)
fprintf(options->file, "%s ...\n", line_prefix);
extra_shown = 1;
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index e212b11..70ee073 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -28,7 +28,7 @@ test_expect_success 'limit output to 2 (simple)' '
test_i18ncmp expect actual
'
-test_expect_failure 'binary changes do not count in lines' '
+test_expect_success 'binary changes do not count in lines' '
git reset --hard &&
chmod +x c d &&
echo a >a &&
--
1.8.0.1.331.g808d2af
^ permalink raw reply related
* [PATCH 3/5] diff --stat: use "file" temporary variable to refer to data->files[i]
From: Junio C Hamano @ 2012-11-27 21:21 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com>
The generated code shouldn't change but it is easier to read.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/diff.c b/diff.c
index ce6baa4..e4e70e5 100644
--- a/diff.c
+++ b/diff.c
@@ -1470,8 +1470,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
for (i = 0; (i < count) && (i < data->nr); i++) {
struct diffstat_file *file = data->files[i];
uintmax_t change = file->added + file->deleted;
- if (!data->files[i]->is_interesting &&
- (change == 0)) {
+
+ if (!file->is_interesting && (change == 0)) {
count++; /* not shown == room for one more */
continue;
}
@@ -1586,13 +1586,13 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
*/
for (i = 0; i < count; i++) {
const char *prefix = "";
- char *name = data->files[i]->print_name;
- uintmax_t added = data->files[i]->added;
- uintmax_t deleted = data->files[i]->deleted;
+ struct diffstat_file *file = data->files[i];
+ char *name = file->print_name;
+ uintmax_t added = file->added;
+ uintmax_t deleted = file->deleted;
int name_len;
- if (!data->files[i]->is_interesting &&
- (added + deleted == 0)) {
+ if (!file->is_interesting && (added + deleted == 0)) {
total_files--;
continue;
}
@@ -1611,7 +1611,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
name = slash;
}
- if (data->files[i]->is_binary) {
+ if (file->is_binary) {
fprintf(options->file, "%s", line_prefix);
show_name(options->file, prefix, name, len);
fprintf(options->file, " %*s", number_width, "Bin");
@@ -1628,7 +1628,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
fprintf(options->file, "\n");
continue;
}
- else if (data->files[i]->is_unmerged) {
+ else if (file->is_unmerged) {
fprintf(options->file, "%s", line_prefix);
show_name(options->file, prefix, name, len);
fprintf(options->file, " Unmerged\n");
@@ -1668,10 +1668,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
fprintf(options->file, "\n");
}
for (i = count; i < data->nr; i++) {
- uintmax_t added = data->files[i]->added;
- uintmax_t deleted = data->files[i]->deleted;
- if (!data->files[i]->is_interesting &&
- (added + deleted == 0)) {
+ struct diffstat_file *file = data->files[i];
+ uintmax_t added = file->added;
+ uintmax_t deleted = file->deleted;
+ if (!file->is_interesting && (added + deleted == 0)) {
total_files--;
continue;
}
--
1.8.0.1.331.g808d2af
^ permalink raw reply related
* [PATCH 5/5] diff --stat: do not count "unmerged" entries
From: Junio C Hamano @ 2012-11-27 21:21 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com>
Even though we show a separate *UNMERGED* entry in the patch and
diffstat output (or in the --raw format, for that matter) in
addition to and separately from the diff against the specified stage
(defaulting to #2) for unmerged paths, they should not be counted in
the total number of files affected---that would lead to counting the
same path twice.
The separation done by the previous step makes this fix simple and
straightforward. Among the filepairs in diff_queue, paths that
weren't modified, and the extra "unmerged" entries do not count as
total number of files.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 6 ++++--
t/t4049-diff-stat-count.sh | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index 4105260..26ede82 100644
--- a/diff.c
+++ b/diff.c
@@ -1669,12 +1669,14 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
struct diffstat_file *file = data->files[i];
uintmax_t added = file->added;
uintmax_t deleted = file->deleted;
- if (!file->is_interesting && (added + deleted == 0)) {
+
+ if (file->is_unmerged ||
+ (!file->is_interesting && (added + deleted == 0))) {
total_files--;
continue;
}
- if (!file->is_binary && !file->is_unmerged) {
+ if (!file->is_binary) {
adds += added;
dels += deleted;
}
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 70ee073..37f50cd 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -44,7 +44,7 @@ test_expect_success 'binary changes do not count in lines' '
test_i18ncmp expect actual
'
-test_expect_failure 'exclude unmerged entries from total file count' '
+test_expect_success 'exclude unmerged entries from total file count' '
git reset --hard &&
echo a >a &&
echo b >b &&
--
1.8.0.1.331.g808d2af
^ permalink raw reply related
* [PATCH 2/5] diff --stat: status of unmodified pair in diff-q is not zero
From: Junio C Hamano @ 2012-11-27 21:21 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com>
It is spelled DIFF_STATUS_UNKNOWN these days, and is different from zero.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 95bbad6..ce6baa4 100644
--- a/diff.c
+++ b/diff.c
@@ -2411,7 +2411,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
}
data = diffstat_add(diffstat, name_a, name_b);
- data->is_interesting = p->status != 0;
+ data->is_interesting = p->status != DIFF_STATUS_UNKNOWN;
if (!one || !two) {
data->is_unmerged = 1;
--
1.8.0.1.331.g808d2af
^ permalink raw reply related
* [PATCH 1/5] test: add failing tests for "diff --stat" to t4049
From: Junio C Hamano @ 2012-11-27 21:21 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com>
There are a few problems in diff.c around --stat area, partially
caused by the recent 74faaa1 (Fix "git diff --stat" for interesting
- but empty - file changes, 2012-10-17), and largely caused by the
earlier change that introduced when --stat-count was added.
Add a few test pieces to t4049 to expose the issues.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t4049-diff-stat-count.sh | 46 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 7b3ef00..e212b11 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -4,12 +4,17 @@
test_description='diff --stat-count'
. ./test-lib.sh
-test_expect_success setup '
+test_expect_success 'setup' '
>a &&
>b &&
>c &&
>d &&
git add a b c d &&
+ git commit -m initial
+'
+
+test_expect_success 'limit output to 2 (simple)' '
+ git reset --hard &&
chmod +x c d &&
echo a >a &&
echo b >b &&
@@ -23,4 +28,43 @@ test_expect_success setup '
test_i18ncmp expect actual
'
+test_expect_failure 'binary changes do not count in lines' '
+ git reset --hard &&
+ chmod +x c d &&
+ echo a >a &&
+ echo b >b &&
+ cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
+ cat >expect <<-\EOF
+ a | 1 +
+ b | 1 +
+ ...
+ 4 files changed, 2 insertions(+)
+ EOF
+ git diff --stat --stat-count=2 >actual &&
+ test_i18ncmp expect actual
+'
+
+test_expect_failure 'exclude unmerged entries from total file count' '
+ git reset --hard &&
+ echo a >a &&
+ echo b >b &&
+ git ls-files -s a >x &&
+ git rm -f d &&
+ for stage in 1 2 3
+ do
+ sed -e "s/ 0 a/ $stage d/" x
+ done |
+ git update-index --index-info &&
+ echo d >d &&
+ chmod +x c d &&
+ cat >expect <<-\EOF
+ a | 1 +
+ b | 1 +
+ ...
+ 4 files changed, 3 insertions(+)
+ EOF
+ git diff --stat --stat-count=2 >actual &&
+ test_i18ncmp expect actual
+'
+
test_done
--
1.8.0.1.331.g808d2af
^ permalink raw reply related
* [PATCH 0/5] "diff --stat" counting fixes
From: Junio C Hamano @ 2012-11-27 21:21 UTC (permalink / raw)
To: git; +Cc: Antoine Pelisse
It turns out that there are at least two bugs in the diffstat
counting code. This series comes on top of the earlier 74faaa1 (Fix
"git diff --stat" for interesting - but empty - file changes,
2012-10-17) to fix them.
Junio C Hamano (5):
test: add failing tests for "diff --stat" to t4049
diff --stat: status of unmodified pair in diff-q is not zero
diff --stat: use "file" temporary variable to refer to data->files[i]
diff --stat: move the "total count" logic to the last loop
diff --stat: do not count "unmerged" entries
diff.c | 49 +++++++++++++++++++++++++---------------------
t/t4049-diff-stat-count.sh | 46 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 72 insertions(+), 23 deletions(-)
--
1.8.0.1.331.g808d2af
^ permalink raw reply
* Re: [PATCH v4 4/4] Hack fix for 'submodule update does not fetch already present commits'
From: W. Trevor King @ 2012-11-27 21:18 UTC (permalink / raw)
To: Heiko Voigt, Git
Cc: Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann,
Nahor
In-Reply-To: <20121127190105.GQ10656@odin.tremily.us>
[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]
On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
> On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> > On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote:
> > > Because you need to recurse through submodules for `update --branch`
> > > even if "$subsha1" == "$sha1", I had to amend the conditional
> > > controlling that block. This broke one of the existing tests, which I
> > > "fixed" in patch 4. I think a proper fix would involve rewriting
> > >
> > > (clear_local_git_env; cd "$sm_path" &&
> > > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
> > > test -z "$rev") || git-fetch)) ||
> > > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> > >
> > > but I'm not familiar enough with rev-list to want to dig into that
> > > yet. If feedback for the earlier three patches is positive, I'll work
> > > up a clean fix and resubmit.
> >
> > You probably need to separate your handling here. The comparison of the
> > currently checked out sha1 and the recorded sha1 is an optimization
> > which skips unnecessary fetching in case the submodules commits are
> > already correct. This code snippet checks whether the to be checked out
> > sha1 is already local and also skips the fetch if it is. We should not
> > break that.
>
> Agreed. However, determining if the target $sha1 is local should have
> nothing to do with the current checked out $subsha1.
Erm, I clearly wasn't getting enough sleep heading into yesterday,
because when I drop the hack patch #4, reinstall, and retest, I no
longer get the bad-fetch error. I'm not quite sure what was going on,
but please pretend I never mentioned it ;).
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: difftool -d symlinks, under what conditions
From: Matt McClure @ 2012-11-27 21:10 UTC (permalink / raw)
To: David Aguilar; +Cc: git@vger.kernel.org, Tim Henigan
In-Reply-To: <CAJDDKr4mTc8-FX7--pd7j0vUbdk_1+KU0YniKEhRdee6SaS-8Q@mail.gmail.com>
On Tuesday, November 27, 2012, David Aguilar wrote:
> It seems that there is an edge case here that we are not
> accounting for: unmodified worktree paths, when checked out
> into the temporary directory, can be edited by the tool when
> comparing against older commits. These edits will be lost.
Yes. That is exactly my desired use case. I want to make edits while
I'm reviewing the diff.
>
> When we can do that then we avoid needing to have a temporary
> directory altogether for any dir-diffs that involve the worktree.
I think the temporary directory is still a good thing. Without it, the
directory diff tool would have no way to distinguish a file added in
the diff from a file that was preexisting and unmodified.
--
Matt McClure
http://www.matthewlmcclure.com
http://www.mapmyfitness.com/profile/matthewlmcclure
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Sitaram Chamarty @ 2012-11-27 21:08 UTC (permalink / raw)
To: esr; +Cc: Magnus Bäck, Felipe Contreras, Michael Haggerty, git
In-Reply-To: <20121127183530.GB11845@thyrsus.com>
On Wed, Nov 28, 2012 at 12:05 AM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Magnus Bäck <baeck@google.com>:
>> While "constant traffic" probably overstates the issue, these are not
>> theoretical problems. I recall at least three cases in the last year
>> or so where Git has seen breakage with Solaris or Mac OS X because
>> of sed or tr incompatibilities, and I don't even read this list that
>> thoroughly.
>
> This is exactly the sort of of pain experience would lead me to
> expect.
>
> OK, this is where I assume the full Old Fart position (30-year
> old-school Unix guy, author of "The Art of Unix Programming", can
> remember the years before Perl and still has sh idioms in his
> fingertips) and say "Get with the 21st century, people! Or at least
> 1990..."
>
> As a general scripting language shell sucks *really badly* compared to
> anything new-school. Performance, portability, you name it, it's a
> mess. It's not so much the shell interpreters itself that are the
> portabilty problem, but (as Magnus implicitly points out) all those
> userland dependencies on sed and tr and awk and even variants of
> expr(!) that get dragged in the second you try to get any actual work
> done.
Not always. There are several situations where a shell script that
makes good use of grep, cut, etc., is definitely much cleaner and more
elegant than anything you can do in a "propah" programming language.
If the price of doing that is sticking to a base set of primitives,
it's a small price to pay, not much different from sticking to python
2.7 or perl 5.8 or whatever.
Shell *is* the universal scripting language, not perl (even though we
all know it is what God himself used to create the world -- see xkcd
224 if you don't believe me!), not python, not Ruby.
--
sitaram
^ permalink raw reply
* Millisecond precision in timestamps?
From: Eric S. Raymond @ 2012-11-27 20:48 UTC (permalink / raw)
To: git
Because I do a lot of work on repository conversion tools, I've had
to learn a lot of detail about ontological mismatches between
version-control systems - especially places where you lose metadata
moving between them.
In general, git metadata can carry forward almost all the metadata in
a Subversion repository. Among the handful of minor exceptions (empty
directories, flow structure, certain kinds of mergeinfos) there is one
that stands out because it seems to be an implementation detail rather
than a consequence of fundamentally different design decisions.
I refer to the one-second precision of git timestamps. Subversion
stores its commit and property-change timestamps to microsecond
precision; conversion tools have to throw the subsecond part of
this information away.
Has going to timestamps with the full precision of the system clock
been considered and rejected, or am I the first to bring this up?
If I were to write refactoring patches that treated "timestamp" as
an ADT, with a view towards hiding the difference between int and
float timestamps and eventually experimenting with float ones,
would they be accepted?
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
Every Communist must grasp the truth, 'Political power grows out of
the barrel of a gun.'
-- Mao Tse-tung, 1938, inadvertently endorsing the Second Amendment.
^ permalink raw reply
* Re: [PATCH 3/6] Fix grammar
From: Junio C Hamano @ 2012-11-27 20:39 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <1354038279-76475-4-git-send-email-max@quendi.de>
Max Horn <max@quendi.de> writes:
> Subject: Re: [PATCH 3/6] Fix grammar
Please run "git shortlog -200 --no-merges" from the tip of your
topic branch before sending a series out, and see if you can
immediately identify what area each of your patches affects.
> Signed-off-by: Max Horn <max@quendi.de>
> ---
> Documentation/git-remote-helpers.txt | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> index db63541..7eb43d7 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -235,9 +235,9 @@ Commands are given by the caller on the helper's standard input, one per line.
> 'capabilities'::
> Lists the capabilities of the helper, one per line, ending
> with a blank line. Each capability may be preceded with '*',
> - which marks them mandatory for git version using the remote
> - helper to understand (unknown mandatory capability is fatal
> - error).
> + which marks them mandatory for git versions using the remote
> + helper to understand. Any unknown mandatory capability is a
> + fatal error.
>
> 'list'::
> Lists the refs, one per line, in the format "<value> <name>
^ permalink raw reply
* Re: Operations on unborn branch
From: Martin von Zweigbergk @ 2012-11-27 20:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd2yyn685.fsf@alter.siamese.dyndns.org>
On Tue, Nov 27, 2012 at 12:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> simplify a lot of things (maybe I'm biased because of the things I
>> have happened to work on?)
>
> Yes. Do not waste time on it.
Yes, no way I would waste time on that; I was mostly just curious.
What I might spend time on is to fix cherry-pick.
^ permalink raw reply
* Re: git-gui: textconv not used on unstaged files
From: Peter Oberndorfer @ 2012-11-27 20:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk3t8qe4s.fsf@alter.siamese.dyndns.org>
On 2012-11-26 21:54, Junio C Hamano wrote:
> Peter Oberndorfer <kumbayo84@arcor.de> writes:
>
>> Does anybody have a idea which git command would output the diff
>> of a untracked file against /dev/null?
> The "--no-index" option is meant as a bolt-on to let you use various
> features of "git diff" that is missing from other people's "diff" in
> a context where git does not know anything about that file. It
> should be usable even outside a git repository.
>
> $ git diff --no-index /dev/null new-file.txt
>
> I do not know offhand (and didn't bother to check) if textconv
> applies, though. It does need access to a git repository as it
> reads from the $GIT_DIR/config to learn what to do.
Hi,
this seems to work when adding the --textconv option.
I will try to see if I can modify git gui to use this command
when clicking a unstaged file.
Thanks,
Greetings Peter
^ permalink raw reply
* RE: Interesting git-format-patch bug
From: Olsen, Alan R @ 2012-11-27 20:31 UTC (permalink / raw)
To: Perry Hutchison, gitster@pobox.com; +Cc: git@vger.kernel.org
In-Reply-To: <50b4304c.EwQy4JquPwsUyMfZ%perryh@pluto.rain.com>
[Sorry for the top posting. Outlook is crap.]
You are correct. I should only get one copy of the patch on branch A. Branch B was modified after the merge and git-format-patch includes the original patch from the merge and a duplicate copy with the changed comments. Note that this patch only has different comments. The body of the patch is exactly the same.
How gerrit mangles things is out of my control. I would prefer that they cherry-pick instead of merges. I have to live with the bad choices of both gerrit and developers in this case.
I guess I will have to diagram out a better example of what is happening here.
-----Original Message-----
From: Perry Hutchison [mailto:perryh@pluto.rain.com]
Sent: Monday, November 26, 2012 8:15 PM
To: gitster@pobox.com
Cc: git@vger.kernel.org; Olsen, Alan R
Subject: Re: Interesting git-format-patch bug
Junio C Hamano <gitster@pobox.com> wrote:
> "Olsen, Alan R" <alan.r.olsen@intel.com> writes:
> > I found an interesting bug in git-format-patch.
> >
> > Say you have a branch A. You create branch B and add a patch to it.
> > You then merge that patch into branch A. After the merge, some other
> > process (we will call it 'gerrit') uses annotate and changes the
> > comment on the patch that exists on branch B.
> >
> > Now someone runs git-format-patch for the last n patches on branch
> > A. You should just get the original patch that was merged over to
> > branch A. What you get is the patch that was merged to branch A
> > *and* the patch with the modified commit comment on branch B.
> > (Double the patches, double the
> > clean-up...)
>
> As you literally have patches that do essentially the same or similar
> things on two branches that was merged, you cannot expect to export
> each individual commit into a patch and not have conflicts among them.
> So I do not think there is no answer than "don't do that".
To me, this seems to miss Alan's point: only one patch was merged to branch A, so git-format-patch applied to branch A should find only one patch. It can be argued either way whether that one-patch report should include the gerrit annotations, but surely the application of gerrit on branch B, _after the merge to branch A has already been performed_, should not cause an additional patch to magically appear on branch A.
^ permalink raw reply
* Re: Operations on unborn branch
From: Junio C Hamano @ 2012-11-27 20:25 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <CANiSa6isDKAgxHWqh5XiQ-adT3-ASFtvAshp028DTcotjQxzmQ@mail.gmail.com>
Martin von Zweigbergk <martinvonz@gmail.com> writes:
> simplify a lot of things (maybe I'm biased because of the things I
> have happened to work on?)
Yes. Do not waste time on it.
^ permalink raw reply
* Re: [PATCH v4 3/4] git-submodule update: Add --branch option
From: W. Trevor King @ 2012-11-27 20:21 UTC (permalink / raw)
To: Heiko Voigt
Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121127185142.GB4185@book.hvoigt.net>
[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]
On Tue, Nov 27, 2012 at 07:51:42PM +0100, Heiko Voigt wrote:
> On Mon, Nov 26, 2012 at 04:00:18PM -0500, W. Trevor King wrote:
> > -b::
> > --branch::
> > - Branch of repository to add as submodule.
> > + When used with the add command, gives the branch of repository to
> > + add as submodule.
> > ++
> > +When used with the update command, checks out a branch named
> > +`submodule.<name>.branch` (as set by `--local-branch`) pointing at the
> > +current HEAD SHA-1. This is useful for commands like `update
> > +--rebase` that do not work on detached heads.
>
> Since you are reusing this option for update it further convinces me
> that reusing it for add makes sense and simplifies the logic for users.
>
> I think an optional argument for --branch would be nice in the update
> case:
>
> $ git submodule update --branch=master
>
> would then allow a user that has not configured anything (except the
> branch tracking info in the submodule of course) to pull all submodules
> master branches.
Sounds good to me. Remember that this is checking the branch and
pointing it at $sha1 (preparing for the pull), not pulling remote
branches. The pull happens in a later
$ git submodules foreach 'git pull'
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index c51b6ae..28eb4b1 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")"
> > die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")"
> > fi
> >
> > - if test "$subsha1" != "$sha1" -o -n "$force"
> > + if test "$subsha1" != "$sha1" -o -n "$force" -o "$update_module" = "branch"
>
> As said before I think separating your code from the current update
> logic will simplify the handling below.
This felt less invasive (it avoids duplicating the recursion logic),
but I don't mind breaking it into a separate function/block.
> > must_die_on_failure=
> > case "$update_module" in
> > rebase)
> > command="git rebase"
> > - die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
> > + die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
> > say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")"
> > - must_die_on_failure=yes
> > + must_die_on_failure=yes
>
> Please always cleanup whitespace changes.
Oops, sloppy me. Will fix.
> > then
> > - die_with_status 2 "$die_msg"
> > - else
> > - err="${err};$die_msg"
> > - continue
> > + if (clear_local_git_env; cd "$sm_path" &&
> > + git branch -f "$branch" "$sha1" &&
> > + git checkout "$branch")
>
> You wrote in earlier emails that you wanted to protect the user from
> non-fastforward changes. So I would expect a
>
> $ git pull --ff-only
I'm not pulling here, I'm doing a regular `submodule update`, and
after that's done I checkout the branch pointing at the $sha1 to which
the branch was just updated. All the submodule-state-clobbering
caveats of a usual `submodule update` still apply to this new
`submodule update --branch`, and I'm fine with that.
> BTW, I am more and more convinced that an automatically manufactured
> commit on update with --branch should be the default.
Again, there's nothing to update. The pull happens in a separate
step.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
From: W. Trevor King @ 2012-11-27 19:01 UTC (permalink / raw)
To: Heiko Voigt
Cc: Junio C Hamano, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121123175402.GH2806@odin.tremily.us>
[-- Attachment #1: Type: text/plain, Size: 4525 bytes --]
On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote:
> > From: "W. Trevor King" <wking@tremily.us>
> >
> > On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote:
> > > We could add
> > >
> > > $ git submodule update --branch
> > >
> > > to checkout the gitlinked SHA1 as submodule.<name>.branch in each of
> > > the submodules, leaving the submodules on the .gitmodules-configured
> > > branch. Effectively (for each submodule):
> > >
> > > $ git branch -f $branch $sha1
> > > $ git checkout $branch
> >
> > I haven't gotten any feedback on this as an idea, but perhaps someone
> > will comment on it as a patch series ;).
>
> I am not sure I understand you correctly. You are suggesting that the
> branch option as an alias for the registered SHA1 in the superproject?
>
> I though the goal of your series was that you want to track submodules
> branch which come from the remote side?
That's what I'd initially thought, but when I went to implement
`update --pull`, I realized that
$ git submodule foreach 'git checkout $(git config --file $toplevel/.gitmodules submodule.$name.branch) && …'
is using submodule.<name>.branch as the local branch name. The remote
branch name was actually setup in .git/modules/<name>/config during
the initial "clone -b <branch> …".
The v4 series leaves the remote branch amigious, but it helps you
point the local branch at the right hash so that future calls to
$ git submodule foreach 'git pull'
can use the branch's .git/modules/<name>/config settings.
> I would think more of some convention like:
>
> $ git checkout -t origin/$branch
>
> when first initialising the submodule with e.g.
>
> $ git submodule update --init --branch
>
> Then later calls of
>
> $ git submodule update --branch
>
> would have a branch configured to pull from. I imagine that results in
> a similar behavior gerrit is doing on the server side?
That sounds like it's doing pretty much the same thing. Can you think
of a test that would distinguish it from my current v4 implementation?
> > Changes since v3:
> >
> > * --record=??? is now --local-branch=???
> > * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation)
> > * Added local git-config overrides of .gitmodules' submodule.<name>.branch
> > * Added `submodule update --branch`
>
> I would prefer if we could squash all these commits together into one
> since it seems to me one logical step, using the new variable for update
> belongs together with its configuration on initialization.
>
> How about reusing the -b|--branch option for add? Since we only change
> the behavior when submodule.$name.update is set to branch it seems
> reasonable to me. Opinions?
That was the approach I used in v1, but people were concerned that we
would be stomping on previously unclaimed config space. Since noone
has pointed out other uses besides Gerrit's very similar case, I'm not
sure if that is still an issue.
> > Because you need to recurse through submodules for `update --branch`
> > even if "$subsha1" == "$sha1", I had to amend the conditional
> > controlling that block. This broke one of the existing tests, which I
> > "fixed" in patch 4. I think a proper fix would involve rewriting
> >
> > (clear_local_git_env; cd "$sm_path" &&
> > ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
> > test -z "$rev") || git-fetch)) ||
> > die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> >
> > but I'm not familiar enough with rev-list to want to dig into that
> > yet. If feedback for the earlier three patches is positive, I'll work
> > up a clean fix and resubmit.
>
> You probably need to separate your handling here. The comparison of the
> currently checked out sha1 and the recorded sha1 is an optimization
> which skips unnecessary fetching in case the submodules commits are
> already correct. This code snippet checks whether the to be checked out
> sha1 is already local and also skips the fetch if it is. We should not
> break that.
Agreed. However, determining if the target $sha1 is local should have
nothing to do with the current checked out $subsha1.
Thanks for the feedback!
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
From: Heiko Voigt @ 2012-11-27 19:16 UTC (permalink / raw)
To: W. Trevor King
Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121127183125.GA4185@book.hvoigt.net>
Hi,
I just realized that I gave you an confusing suggestion.
On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> if test "$subsha1" != "$sha1"
> then
> handle_on_demand_fetch_update ...
> else
> handle_tracked_branch_update ...
> fi
That obviously does not work. Here I meant of course something like:
if test "$update_module" = "branch"
then
handle_tracked_branch_update ...
else
handle_on_demand_fetch_update ...
fi
Cheers Heiko
^ permalink raw reply
* Re: [PATCH] diff: Fixes shortstat number of files
From: Junio C Hamano @ 2012-11-27 19:14 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git, Linus Torvalds
In-Reply-To: <CALWbr2z+9fJGg34q7zp3knZVWgaTwtvzFBvFHRSmXfH+hc-jNw@mail.gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> Indeed stat seems to be broken on master by commit 74faaa16 from Linus Torvalds
>
> There are three separated issues here:
> - unmerged files are marked as "interesting" in stat and probably
> shouldn't, with some patch like this:
>
> data->is_interesting = p->status != 0;
>
> if (!one || !two) {
> data->is_unmerged = 1;
> + data->is_interesting = 0;
> return;
> }
>
> By the way, I don't get the point of this code then:
>
> else if (data->files[i]->is_unmerged) {
> fprintf(options->file, "%s", line_prefix);
> show_name(options->file, prefix, name, len);
> fprintf(options->file, " Unmerged\n");
> continue;
> }
>
> and
>
> if (file->is_unmerged) {
> /* "Unmerged" is 8 characters */
> bin_width = bin_width < 8 ? 8 : bin_width;
> continue;
> }
>
> Are we ever supposed to print that ? I feel like it could be removed.
Yes, we have been showing two entries in --stat output:
file | Unmerged
file | 4 ++++
and that is not going to change.
There are a few problems in diff.c around --stat area, partially
caused by Linus's patch (like s/is_rename/is_interesting/ change
started ignoring unmerged entries in the diff queue and made the
existing loop not go into the codepath we see above), and largely
caused by the earlier change that introduced when --stat-count was
added (the second loop that decrements total_files does so only for
the paths within the "count" horizon determined by the first loop;
total_files must be adjusted for _all_ uninteresting and unchanged
filepairs and exclude unmerged entries).
Also the initialization of data->is_interesting is wrong. These
days, p->status is never zero.
I'll send out a fix later today.
^ permalink raw reply
* Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
From: W. Trevor King @ 2012-11-27 19:04 UTC (permalink / raw)
To: Heiko Voigt
Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121127183125.GA4185@book.hvoigt.net>
[-- Attachment #1: Type: text/plain, Size: 704 bytes --]
On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> I would prefer if we could squash all these commits together into
> one since it seems to me one logical step, using the new variable
> for update belongs together with its configuration on
> initialization.
Works for me. I could also try to rework the patch boundaries if a
monolithic patch is not acceptable. I agree that the current
documentation assignments are fairly arbitrary. If I don't hear from
anyone in favor of keeping them separate, v5 will be monolithic.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/4] git-submodule update: Add --branch option
From: Heiko Voigt @ 2012-11-27 18:51 UTC (permalink / raw)
To: W. Trevor King
Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <95edff1c97c513c555652014f9c2bbf61c8e7560.1353962698.git.wking@tremily.us>
On Mon, Nov 26, 2012 at 04:00:18PM -0500, W. Trevor King wrote:
> From: "W. Trevor King" <wking@tremily.us>
>
> This allows users to checkout the current
> superproject-recorded-submodule-sha as a branch, avoiding the detached
> head state that the standard submodule update creates. This may be
> useful for the existing --rebase/--merge workflows which already avoid
> detached heads.
>
> It is also useful if you want easy tracking of upstream branches. The
> particular upstream branch to be tracked is configured locally with
> .git/modules/<name>/config. With the new option Ævar's suggested
>
> $ git submodule foreach 'git checkout $(git config --file $toplevel/.gitm
> odules submodule.$name.branch) && git pull'
>
> reduces to a
>
> $ git submodule update --branch
>
> after each supermodule .gitmodules edit, and a
>
> $ git submodule foreach 'git pull'
>
> whenever you feel like updating the submodules. Your still on you're
> own to commit (or not) the updated submodule hashes in the
> superproject's .gitmodules.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> Documentation/git-submodule.txt | 20 +++++++++++------
> git-submodule.sh | 48 +++++++++++++++++++++++++++++----------
> t/t7406-submodule-update.sh | 50 ++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 98 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index d0b4436..34392a1 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
> [-f|--force] [--reference <repository>] [--] <repository> [<path>]
> 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
> 'git submodule' [--quiet] init [--] [<path>...]
> -'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> +'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--branch] [--rebase]
> [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
> [commit] [--] [<path>...]
> @@ -136,11 +136,11 @@ init::
>
> update::
> Update the registered submodules, i.e. clone missing submodules and
> - checkout the commit specified in the index of the containing repository.
> - This will make the submodules HEAD be detached unless `--rebase` or
> - `--merge` is specified or the key `submodule.$name.update` is set to
> - `rebase`, `merge` or `none`. `none` can be overridden by specifying
> - `--checkout`.
> + checkout the commit specified in the index of the containing
> + repository. This will make the submodules HEAD be detached unless
> + `--branch`, `--rebase`, `--merge` is specified or the key
> + `submodule.$name.update` is set to `branch`, `rebase`, `merge` or
> + `none`. `none` can be overridden by specifying `--checkout`.
> +
> If the submodule is not yet initialized, and you just want to use the
> setting as stored in .gitmodules, you can automatically initialize the
> @@ -207,7 +207,13 @@ OPTIONS
>
> -b::
> --branch::
> - Branch of repository to add as submodule.
> + When used with the add command, gives the branch of repository to
> + add as submodule.
> ++
> +When used with the update command, checks out a branch named
> +`submodule.<name>.branch` (as set by `--local-branch`) pointing at the
> +current HEAD SHA-1. This is useful for commands like `update
> +--rebase` that do not work on detached heads.
Since you are reusing this option for update it further convinces me
that reusing it for add makes sense and simplifies the logic for users.
I think an optional argument for --branch would be nice in the update
case:
$ git submodule update --branch=master
would then allow a user that has not configured anything (except the
branch tracking info in the submodule of course) to pull all submodules
master branches.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index c51b6ae..28eb4b1 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -627,7 +631,7 @@ Maybe you want to use 'update --init'?")"
> die "$(eval_gettext "Unable to find current revision in submodule path '\$sm_path'")"
> fi
>
> - if test "$subsha1" != "$sha1" -o -n "$force"
> + if test "$subsha1" != "$sha1" -o -n "$force" -o "$update_module" = "branch"
As said before I think separating your code from the current update
logic will simplify the handling below.
> then
> subforce=$force
> # If we don't already have a -f flag and the submodule has never been checked out
> @@ -650,16 +654,21 @@ Maybe you want to use 'update --init'?")"
> case ";$cloned_modules;" in
> *";$name;"*)
> # then there is no local change to integrate
> - update_module= ;;
> + case "$update_module" in
> + rebase|merge)
> + update_module=
> + ;;
> + esac
> + ;;
> esac
>
> must_die_on_failure=
> case "$update_module" in
> rebase)
> command="git rebase"
> - die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
> + die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$sm_path'")"
> say_msg="$(eval_gettext "Submodule path '\$sm_path': rebased into '\$sha1'")"
> - must_die_on_failure=yes
> + must_die_on_failure=yes
Please always cleanup whitespace changes.
> ;;
> merge)
> command="git merge"
> @@ -674,15 +683,30 @@ Maybe you want to use 'update --init'?")"
> ;;
> esac
>
> then
> - die_with_status 2 "$die_msg"
> - else
> - err="${err};$die_msg"
> - continue
> + if (clear_local_git_env; cd "$sm_path" &&
> + git branch -f "$branch" "$sha1" &&
> + git checkout "$branch")
You wrote in earlier emails that you wanted to protect the user from
non-fastforward changes. So I would expect a
$ git pull --ff-only
here and the setup of that in the initialization of the submodule.
BTW, I am more and more convinced that an automatically manufactured
commit on update with --branch should be the default. What do other
think? Sascha raised a concern that he would not want this, but as far as
I understood he let the CI-server do that so I see no downside to
natively adding that to git. People who want to manually craft those
commits can still amend the generated commit. Since this is all about
helping people keeping their submodules updated why not go the full way?
Cheers Heiko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox