* Re: [PATCH 1/2] rebase -i --root: simplify code
From: Junio C Hamano @ 2009-01-26 5:54 UTC (permalink / raw)
To: Thomas Rast; +Cc: Johannes Schindelin, git
In-Reply-To: <200901260053.06315.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Thomas Rast wrote:
>> test ! -s "$DOTEST"/upstream && REBASE_ROOT=t
>
> Actually, I think that test never worked (and it's clearly my fault).
>
> The corresponding 'echo $UPSTREAM > "$DOTEST"/upstream' just expanded
> to 'echo > ...', resulting in a file containing a single newline, but
> never a zero-length file. Duh.
Since you never use the value stored in "$DOTEST/upstream" for anything
else anyway, how about doing something like this instead? It would make
the meaning of the file used as a state variable much clearer.
It may break hooks and outside scripts that look at $DOTEST/upstream. I
didn't check.
The hunk in the middle is to protect you against an environment variable
UPSTREAM the user may have before starting "rebase -i". There could be
other state variables you added in recent commit d911d14 (rebase -i: learn
to rebase root commit, 2009-01-02) that needs similar protection. Please
check.
git-rebase--interactive.sh | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git c/git-rebase--interactive.sh w/git-rebase--interactive.sh
index 21ac20c..17cf0e5 100755
--- c/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -456,7 +456,7 @@ get_saved_options () {
test -d "$REWRITTEN" && PRESERVE_MERGES=t
test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
test -f "$DOTEST"/verbose && VERBOSE=t
- test ! -s "$DOTEST"/upstream && REBASE_ROOT=t
+ test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
}
while test $# != 0
@@ -585,6 +585,7 @@ first and then run 'git rebase --continue' again."
test -z "$ONTO" && ONTO=$UPSTREAM
shift
else
+ UPSTREAM=
UPSTREAM_ARG=--root
test -z "$ONTO" &&
die "You must specify --onto when using --root"
@@ -611,7 +612,12 @@ first and then run 'git rebase --continue' again."
echo "detached HEAD" > "$DOTEST"/head-name
echo $HEAD > "$DOTEST"/head
- echo $UPSTREAM > "$DOTEST"/upstream
+ case "$REBASE_ROOT" in
+ '')
+ rm -f "$DOTEST"/rebase-root ;;
+ *)
+ : >"$DOTEST"/rebase-root ;;
+ esac
echo $ONTO > "$DOTEST"/onto
test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
test t = "$VERBOSE" && : > "$DOTEST"/verbose
^ permalink raw reply related
* Re: [PATCH 1/3] Documentation: simplify refspec format description
From: Junio C Hamano @ 2009-01-26 6:24 UTC (permalink / raw)
To: Anders Melchiorsen; +Cc: git
In-Reply-To: <1232927133-30377-2-git-send-email-mail@cup.kalibalik.dk>
Anders Melchiorsen <mail@cup.kalibalik.dk> writes:
> diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt
> index ebdd948..820c140 100644
> --- a/Documentation/pull-fetch-param.txt
> +++ b/Documentation/pull-fetch-param.txt
> @@ -5,10 +5,10 @@
> of a remote (see the section <<REMOTES,REMOTES>> below).
>
> <refspec>::
> - The canonical format of a <refspec> parameter is
> - `+?<src>:<dst>`; that is, an optional plus `{plus}`, followed
> - by the source ref, followed by a colon `:`, followed by
> - the destination ref.
> + The format of a <refspec> parameter is an optional plus
> + `{plus}`, followed by the source ref <src>, followed
> + by a colon `:`, followed by the destination ref <dst>.
> + Find various forms of refspecs in examples section.
> +
> The remote ref that matches <src>
> is fetched, and if <dst> is not empty string, the local
I think this is *much* nicer, but I do not think git-fetch.txt has
examples to fall back on.
The patch to git-push.txt would not have this issue; the exmaple is there
in the page itself.
But I think it might be even better to briefly describe what it means,
like this patch on top of yours does to git-push.txt. The fetch/pull side
already has the corresponding description immediately after that, so I'd
suggest just removing the reference to non-existing examples section.
I found your 2/3 and 3/3 good improvements.
Documentation/git-push.txt | 3 ++-
Documentation/pull-fetch-param.txt | 3 +--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git i/Documentation/git-push.txt w/Documentation/git-push.txt
index 3fd4bbb..ea45935 100644
--- i/Documentation/git-push.txt
+++ w/Documentation/git-push.txt
@@ -36,7 +36,8 @@ OPTIONS
The format of a <refspec> parameter is an optional plus
`{plus}`, followed by the source ref <src>, followed
by a colon `:`, followed by the destination ref <dst>.
- Find various forms of refspecs in examples section.
+ It is used to specify with what <src> object the <dst> ref
+ in the remote repository is to be updated.
+
The <src> side represents the source branch (or arbitrary
"SHA1 expression", such as `master~4` (four parents before the
diff --git i/Documentation/pull-fetch-param.txt w/Documentation/pull-fetch-param.txt
index 820c140..f9811f2 100644
--- i/Documentation/pull-fetch-param.txt
+++ w/Documentation/pull-fetch-param.txt
@@ -8,12 +8,11 @@
The format of a <refspec> parameter is an optional plus
`{plus}`, followed by the source ref <src>, followed
by a colon `:`, followed by the destination ref <dst>.
- Find various forms of refspecs in examples section.
+
The remote ref that matches <src>
is fetched, and if <dst> is not empty string, the local
ref that matches it is fast forwarded using <src>.
-Again, if the optional plus `+` is used, the local ref
+If the optional plus `+` is used, the local ref
is updated even if it does not result in a fast forward
update.
+
^ permalink raw reply related
* Re: [PATCH] git-svn: add --ignore-paths option for fetching
From: Junio C Hamano @ 2009-01-26 6:28 UTC (permalink / raw)
To: Eric Wong; +Cc: public_vi, Thomas Rast, git
In-Reply-To: <20090126011847.GA8703@dcvr.yhbt.net>
Eric Wong <normalperson@yhbt.net> writes:
> Thanks Vitaly, acked and pushed out with minor fixes to
> git://git.bogomips.org/git-svn.git
Pulled; thanks.
^ permalink raw reply
* Re: [PATCH 08/10] run test suite without dashed git-commands in PATH
From: Matthew Ogilvie @ 2009-01-26 6:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901250255250.14855@racer>
Hi,
On Sun, Jan 25, 2009 at 02:59:53AM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Sat, 24 Jan 2009, Matthew Ogilvie wrote:
>
> > .gitignore | 1 +
> > Makefile | 42 +++++++++++++++++++++++++++++++-----------
> > t/test-lib.sh | 14 +++++++++++++-
> > test-bin-wrapper.sh | 12 ++++++++++++
> > 4 files changed, 57 insertions(+), 12 deletions(-)
> > create mode 100644 test-bin-wrapper.sh
>
> I am strongly opposed to a patch this big, just for something as 3rd class
> as CVS server faking. We already have a big fallout from all that bending
> over for Windows support, and I do not like it at all.
>
> Note: I do not even have to look further than the diffstat to see that it
> is wrong.
>
> The point is: if cvsserver wants to pretend that it is in a fake bin where
> almost none of the other Git programs are, fine, let's do that _in the
> test for cvsserver_.
>
> Let's not fsck up the whole test suite just for one user.
>
> Ciao,
> Dscho
Since by default git is installed such that most of the dashed-form
commands are not in a user's default PATH, my thought was that
it would make sense for the test suite to mimick that environment
as much as possible. This could detect regressions in any
installed/tested git script that erroneously assumes the dashed
form commands are in the PATH, not just git-cvsserver.
I can think of ways it could be made smaller, but they seem uglier than
the current patch to me:
1. Perhaps just list the executables for the fake bin directory
separately, but then it is all too easy for the list to get out of sync
with what the final install environment will be.
2. Perhaps strip off the $X's (.exe on windows; currently empty
elsewhere) from the words of the existing variables, in the rule
for building the "fake bin" directory. But generally I'ld rather
not assume that pattern-matching replacement of
$X's will never conflict with a part of a script name.
3. Perhaps just use symlinks or hardlinks instead of a wrapper
script. This might have some promise, except that links are more
likely to fail on windows, and the wrappers generally give you
more flexibility for testing odd scenarios.
4. The test-bin-wrapper.sh script does not actually need to
set environment variables (GIT_EXEC_DIT and templates) for
purposes of this patch. But my thought was that in this form
you could run things straight out of the test-bin directory
to manually try out new code without needing to actually install
a build or mess with the environment variables yourself. It could
also be extended to handle other global wrapper needs
relatively easily, such as valgrind.
Any other ideas?
--
Matthew Ogilvie [mmogilvi_git@miniinfo.net]
^ permalink raw reply
* Re: bug: transform a binary file into a symlink in one commit => invalid binary patch
From: Junio C Hamano @ 2009-01-26 7:37 UTC (permalink / raw)
To: Jeff King; +Cc: Pixel, git
In-Reply-To: <20090126003556.GA19368@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Jan 23, 2009 at 01:25:30PM +0100, Pixel wrote:
>
>> i hit a bug (git 1.6.1): when you transform a binary file into a
>> symlink in one commit, the binary patch can't be used in "git apply".
>> Is it a known issue?
>
> Not that I know of.
>
> Below is a patch against the test suite that fairly neatly displays the
> problem. I didn't get a chance to look into actually fixing it, though
> (I'm not even sure the problem is in apply, and not in the generated
> patch).
The generated diff is wrong.
A filepair that changes type must be split into deletion followed by
creation, which means the "index" line should say 0{40} on the right hand
side for the first half and then 0{40} on the left hand side for the
second half. The patch generated by this step:
> +test_expect_success 'create patch' '
> + git diff-tree --binary HEAD^ HEAD >patch
> +'
However says the blob contents change from "\0" to "file" on both.
This is because diff.c::run_diff() computes "index" only once and reuses
it for both halves.
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #06; Sat, 24)
From: Johannes Sixt @ 2009-01-26 7:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Steffen Prohaska
In-Reply-To: <7v8wp0kmj4.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> * sp/runtime-prefix (Sun Jan 18 13:00:15 2009 +0100) 7 commits
> - Windows: Revert to default paths and convert them by
> RUNTIME_PREFIX
> - Compute prefix at runtime if RUNTIME_PREFIX is set
> - Modify setup_path() to only add git_exec_path() to PATH
> - Add calls to git_extract_argv0_path() in programs that call
> git_config_*
> - git_extract_argv0_path(): Move check for valid argv0 from caller
> to callee
> - Refactor git_set_argv0_path() to git_extract_argv0_path()
> - Move computation of absolute paths from Makefile to runtime (in
> preparation for RUNTIME_PREFIX)
>
> We should move this to 'next' soon with J6t's blessing.
I've been using this series for a few days now without problems:
Acked-by: Johannes Sixt <j6t@kdbg.org>
-- Hannes
^ permalink raw reply
* Re: [PATCH] http-push: refactor request url creation
From: Daniel Stenberg @ 2009-01-26 8:02 UTC (permalink / raw)
To: Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0901251752p5b34c053pb24dce8a35b06fce@mail.gmail.com>
On Mon, 26 Jan 2009, Ray Chuan wrote:
>>> - curl_easy_setopt(slot->curl, CURLOPT_URL, url);
>>> + curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
>>
>> The original code gave a separate "url" to setop() but this gives the same
>> string. Does curl_easy_setop() copies the given string away? IOW is this
>> change safe?
>
> curl strdup's it, so this is safe.
I'm not sure what the oldest possibly libcurl version git can deal with, but
here's a related quote from the curl_easy_setopt man page:
NOTE: before 7.17.0 strings were not copied. Instead the user was
forced keep them available until libcurl no longer needed them.
--
/ daniel.haxx.se
^ permalink raw reply
* [PATCH (v2)] Mention "local convention" rule in the CodingGuidelines
From: Nanako Shiraishi @ 2009-01-26 8:32 UTC (permalink / raw)
To: git; +Cc: Sverre Rabbelier
The document suggests to imitate the existing code, but didn't
say which existing code it should imitate. This clarifies.
Signed-off-by: しらいしななこ <nanako3@lavabit.com>
---
Quoting Junio C Hamano <gitster@pobox.com>:
> It is always preferable to match the _local_ convention. I'd expect a new
> script added to git suite to match my preference (the one I showed you in
> my comments to you that is used in git-am, which is what you suggested
> above), but I'd expect a modification to mergetool to match the style
> mergetool already uses.
Sverre fixed some typo for me.
Documentation/CodingGuidelines | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index f628c1f..664c6c2 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -21,8 +21,13 @@ code. For git in general, three rough rules are:
As for more concrete guidelines, just imitate the existing code
(this is a good guideline, no matter which project you are
-contributing to). But if you must have a list of rules,
-here they are.
+contributing to). It is always preferable to match the _local_
+convention. New code added to git suite is expected to match
+the overall style of existing code. Modifications to existing
+code is expected to match the style the surrounding code already
+uses (even if it doesn't match the overall style of existing code).
+
+But if you must have a list of rules, here they are.
For shell scripts specifically (not exhaustive):
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply related
* [PATCH] diff.c: output correct index lines for a split diff
From: Junio C Hamano @ 2009-01-26 8:33 UTC (permalink / raw)
To: Jeff King; +Cc: Pixel, git
In-Reply-To: <7vy6wy8qmm.fsf@gitster.siamese.dyndns.org>
A patch that changes the filetype (e.g. regular file to symlink) of a path
must be split into a deletion event followed by a creation event, which
means that we need to have two independent metainfo lines for each.
However, the code reused the single set of metainfo lines.
As the blob object names recorded on the index lines are usually not used
nor validated on the receiving end, this is not an issue with normal use
of the resulting patch. However, when accepting a binary patch to delete
a blob, git-apply verified that the postimage blob object name on the
index line is 0{40}, hence a patch that deletes a regular file blob that
records binary contents to create a blob with different filetype (e.g. a
symbolic link) failed to apply. "git am -3" also uses the blob object
names recorded on the index line, so it would also misbehave when
synthesizing a preimage tree.
This moves the code to generate metainfo lines around, so that two
independent sets of metainfo lines are used for the split halves.
The fix revealed that one test expected the incorrect behaviour, which is
also fixed here.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
> ...
>> Below is a patch against the test suite that fairly neatly displays the
>> problem. I didn't get a chance to look into actually fixing it, though
>> (I'm not even sure the problem is in apply, and not in the generated
>> patch).
>
> The generated diff is wrong.
>
> A filepair that changes type must be split into deletion followed by
> creation, which means the "index" line should say 0{40} on the right hand
> side for the first half and then 0{40} on the left hand side for the
> second half. The patch generated by this step:
>
>> +test_expect_success 'create patch' '
>> + git diff-tree --binary HEAD^ HEAD >patch
>> +'
>
> However says the blob contents change from "\0" to "file" on both.
>
> This is because diff.c::run_diff() computes "index" only once and reuses
> it for both halves.
I did not include your new test script here; perhaps we can add it to an
existing typechange diff/apply test, like t4114?
diff.c | 146 +++++++++++++++++++++++++---------------------
t/t4030-diff-textconv.sh | 2 +-
2 files changed, 81 insertions(+), 67 deletions(-)
diff --git a/diff.c b/diff.c
index 972b3da..7e18364 100644
--- a/diff.c
+++ b/diff.c
@@ -2081,16 +2081,86 @@ static void run_external_diff(const char *pgm,
}
}
+static int similarity_index(struct diff_filepair *p)
+{
+ return p->score * 100 / MAX_SCORE;
+}
+
+static void fill_metainfo(struct strbuf *msg,
+ const char *name,
+ const char *other,
+ struct diff_filespec *one,
+ struct diff_filespec *two,
+ struct diff_options *o,
+ struct diff_filepair *p)
+{
+ strbuf_init(msg, PATH_MAX * 2 + 300);
+ switch (p->status) {
+ case DIFF_STATUS_COPIED:
+ strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
+ strbuf_addstr(msg, "\ncopy from ");
+ quote_c_style(name, msg, NULL, 0);
+ strbuf_addstr(msg, "\ncopy to ");
+ quote_c_style(other, msg, NULL, 0);
+ strbuf_addch(msg, '\n');
+ break;
+ case DIFF_STATUS_RENAMED:
+ strbuf_addf(msg, "similarity index %d%%", similarity_index(p));
+ strbuf_addstr(msg, "\nrename from ");
+ quote_c_style(name, msg, NULL, 0);
+ strbuf_addstr(msg, "\nrename to ");
+ quote_c_style(other, msg, NULL, 0);
+ strbuf_addch(msg, '\n');
+ break;
+ case DIFF_STATUS_MODIFIED:
+ if (p->score) {
+ strbuf_addf(msg, "dissimilarity index %d%%\n",
+ similarity_index(p));
+ break;
+ }
+ /* fallthru */
+ default:
+ /* nothing */
+ ;
+ }
+ if (one && two && hashcmp(one->sha1, two->sha1)) {
+ int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
+
+ if (DIFF_OPT_TST(o, BINARY)) {
+ mmfile_t mf;
+ if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
+ (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
+ abbrev = 40;
+ }
+ strbuf_addf(msg, "index %.*s..%.*s",
+ abbrev, sha1_to_hex(one->sha1),
+ abbrev, sha1_to_hex(two->sha1));
+ if (one->mode == two->mode)
+ strbuf_addf(msg, " %06o", one->mode);
+ strbuf_addch(msg, '\n');
+ }
+ if (msg->len)
+ strbuf_setlen(msg, msg->len - 1);
+}
+
static void run_diff_cmd(const char *pgm,
const char *name,
const char *other,
const char *attr_path,
struct diff_filespec *one,
struct diff_filespec *two,
- const char *xfrm_msg,
+ struct strbuf *msg,
struct diff_options *o,
- int complete_rewrite)
+ struct diff_filepair *p)
{
+ const char *xfrm_msg = NULL;
+ int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
+
+ if (msg) {
+ fill_metainfo(msg, name, other, one, two, o, p);
+ xfrm_msg = msg->len ? msg->buf : NULL;
+ }
+
if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
pgm = NULL;
else {
@@ -2130,11 +2200,6 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
hashclr(one->sha1);
}
-static int similarity_index(struct diff_filepair *p)
-{
- return p->score * 100 / MAX_SCORE;
-}
-
static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
{
/* Strip the prefix but do not molest /dev/null and absolute paths */
@@ -2148,13 +2213,11 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
{
const char *pgm = external_diff();
struct strbuf msg;
- char *xfrm_msg;
struct diff_filespec *one = p->one;
struct diff_filespec *two = p->two;
const char *name;
const char *other;
const char *attr_path;
- int complete_rewrite = 0;
name = p->one->path;
other = (strcmp(name, p->two->path) ? p->two->path : NULL);
@@ -2164,83 +2227,34 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
if (DIFF_PAIR_UNMERGED(p)) {
run_diff_cmd(pgm, name, NULL, attr_path,
- NULL, NULL, NULL, o, 0);
+ NULL, NULL, NULL, o, p);
return;
}
diff_fill_sha1_info(one);
diff_fill_sha1_info(two);
- strbuf_init(&msg, PATH_MAX * 2 + 300);
- switch (p->status) {
- case DIFF_STATUS_COPIED:
- strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
- strbuf_addstr(&msg, "\ncopy from ");
- quote_c_style(name, &msg, NULL, 0);
- strbuf_addstr(&msg, "\ncopy to ");
- quote_c_style(other, &msg, NULL, 0);
- strbuf_addch(&msg, '\n');
- break;
- case DIFF_STATUS_RENAMED:
- strbuf_addf(&msg, "similarity index %d%%", similarity_index(p));
- strbuf_addstr(&msg, "\nrename from ");
- quote_c_style(name, &msg, NULL, 0);
- strbuf_addstr(&msg, "\nrename to ");
- quote_c_style(other, &msg, NULL, 0);
- strbuf_addch(&msg, '\n');
- break;
- case DIFF_STATUS_MODIFIED:
- if (p->score) {
- strbuf_addf(&msg, "dissimilarity index %d%%\n",
- similarity_index(p));
- complete_rewrite = 1;
- break;
- }
- /* fallthru */
- default:
- /* nothing */
- ;
- }
-
- if (hashcmp(one->sha1, two->sha1)) {
- int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
-
- if (DIFF_OPT_TST(o, BINARY)) {
- mmfile_t mf;
- if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
- (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
- abbrev = 40;
- }
- strbuf_addf(&msg, "index %.*s..%.*s",
- abbrev, sha1_to_hex(one->sha1),
- abbrev, sha1_to_hex(two->sha1));
- if (one->mode == two->mode)
- strbuf_addf(&msg, " %06o", one->mode);
- strbuf_addch(&msg, '\n');
- }
-
- if (msg.len)
- strbuf_setlen(&msg, msg.len - 1);
- xfrm_msg = msg.len ? msg.buf : NULL;
-
if (!pgm &&
DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
(S_IFMT & one->mode) != (S_IFMT & two->mode)) {
- /* a filepair that changes between file and symlink
+ /*
+ * a filepair that changes between file and symlink
* needs to be split into deletion and creation.
*/
struct diff_filespec *null = alloc_filespec(two->path);
run_diff_cmd(NULL, name, other, attr_path,
- one, null, xfrm_msg, o, 0);
+ one, null, &msg, o, p);
free(null);
+ strbuf_release(&msg);
+
null = alloc_filespec(one->path);
run_diff_cmd(NULL, name, other, attr_path,
- null, two, xfrm_msg, o, 0);
+ null, two, &msg, o, p);
free(null);
}
else
run_diff_cmd(pgm, name, other, attr_path,
- one, two, xfrm_msg, o, complete_rewrite);
+ one, two, &msg, o, p);
strbuf_release(&msg);
}
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 2f27a0b..a3f0897 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -104,7 +104,7 @@ cat >expect.typechange <<'EOF'
-1
diff --git a/file b/file
new file mode 120000
-index ad8b3d2..67be421
+index 0000000..67be421
--- /dev/null
+++ b/file
@@ -0,0 +1 @@
--
1.6.1.1.248.g7f6d2
^ permalink raw reply related
* [PATCH] Reintegrate script
From: Junio C Hamano @ 2009-01-26 9:03 UTC (permalink / raw)
To: git
In a workflow that uses topic branches heavily, you would need to keep
updating test integration branch(es) all the time. If they are managed
like my 'next' by merging the tips of topics that have grown since the
last integration, it is not so difficult. You only need to review output
from "git branch --no-merged next" to see if there are topics that can and
needs to be merged to 'next'.
But sometimes it is easier to rebuild a test integration branch from
scratch all the time, especially if you do not publish it for others to
build on.
I've been using this script for some time to rebuild jch and pu branches
in my workflow. jch's tip is supposed to always match 'next', but it is
rebuilt from scratch on top of 'master' by merging the same topics that
are in 'next'. You can use the same script in your work.
To use it, you give a commit range base..tip to the script, and you will
see a shell script that uses a series of 'git-merge'. "base" is the more
stable branch that you rebuild your test integration branch on top (in my
case, 'master'), and "tip" is where the tip of the test integration branch
is from the last round (in my case, 'jch' or 'pu').
Then you can run the resulting script, fix conflicted merge and use
'git-commit', and then repeat until all the branches are re-integrated on
top of the base branch.
$ Meta/Reintegrate master..jch >/var/tmp/redo-jch.sh
$ cat /var/tmp/redo-jch.sh
#!/bin/sh
while read branch eh
do
case "$eh" in
"") git merge "$branch" || break ;;
?*) echo >&2 "Eh? $branch $eh"; break ;;
esac
done <<EOF
jc/blame
js/notes
ks/maint-mailinfo-folded~3
tr/previous-branch
EOF
$ git checkout jch
$ git reset --hard master
$ /var/tmp/redo-jch.sh
... if there is conflict, resolve, "git commit" here ...
$ /var/tmp/redo-jch.sh
... repeat until everything is applied.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is taken from my 'todo' branch, which I keep a checkout in Meta/
directory.
Reintegrate | 42 ++++++++++++++++++++++++++++++++++++++++++
1 files changed, 42 insertions(+), 0 deletions(-)
create mode 100755 Reintegrate
diff --git a/Reintegrate b/Reintegrate
new file mode 100755
index 0000000..dfdb73e
--- /dev/null
+++ b/Reintegrate
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+merge_msg="Merge branch '\(.*\)'"
+x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+x40="$x40$x40$x40$x40$x40$x40$x40$x40"
+LF='
+'
+
+echo '#!/bin/sh
+while read branch eh
+do
+ case "$eh" in
+ "") git merge "$branch" || break ;;
+ ?*) echo >&2 "Eh? $branch $eh"; break ;;
+ esac
+done <<EOF'
+
+git log --pretty=oneline --first-parent "$1" |
+{
+ series=
+ while read commit msg
+ do
+ other=$(git rev-parse --verify "$commit^2") &&
+ branch=$(expr "$msg" : "$merge_msg") &&
+ tip=$(git rev-parse --verify "refs/heads/$branch" 2>/dev/null) &&
+ merged=$(git name-rev --refs="refs/heads/$branch" "$other" 2>/dev/null) &&
+ merged=$(expr "$merged" : "$x40 \(.*\)") &&
+ test "$merged" != undefined || {
+ other=$(git log -1 --pretty='format:%s' $other) &&
+ merged="$branch :rebased? $other"
+ }
+ if test -z "$series"
+ then
+ series="$merged"
+ else
+ series="$merged$LF$series"
+ fi
+ done
+ echo "$series"
+}
+
+echo 'EOF'
--
1.6.1.1.248.g7f6d2
^ permalink raw reply related
* [PATCH] rebase -i: correctly remember --root flag across --continue
From: Thomas Rast @ 2009-01-26 9:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin, Junio C Hamano
In-Reply-To: <7vtz7ma9z1.fsf@gitster.siamese.dyndns.org>
From: Junio C Hamano <gitster@pobox.com>
d911d14 (rebase -i: learn to rebase root commit, 2009-01-02) tried to
remember the --root flag across a merge conflict in a broken way.
Introduce a flag file $DOTEST/rebase-root to fix and clarify.
While at it, also make sure $UPSTREAM is always initialized to guard
against existing values in the environment.
[tr: added tests]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Junio C Hamano wrote:
> Since you never use the value stored in "$DOTEST/upstream" for anything
> else anyway, how about doing something like this instead? It would make
> the meaning of the file used as a state variable much clearer.
Yes, thanks, a patch precisely "like this" is in fact the right fix.
I came up with some tests that try a conflicted --root rebase of each
flavour, to guard against the problem in the future. I wasn't
entirely sure how to shape this into a patch, but here's a version
that forges patch message and sign-off in your name.
Dscho, with that confusion cleared, you can add my Ack to your 1/2
(unchanged, though I'm afraid you'll get a textual conflict).
git-rebase--interactive.sh | 10 ++++-
t/t3412-rebase-root.sh | 88 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 21ac20c..17cf0e5 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -456,7 +456,7 @@ get_saved_options () {
test -d "$REWRITTEN" && PRESERVE_MERGES=t
test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
test -f "$DOTEST"/verbose && VERBOSE=t
- test ! -s "$DOTEST"/upstream && REBASE_ROOT=t
+ test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
}
while test $# != 0
@@ -585,6 +585,7 @@ first and then run 'git rebase --continue' again."
test -z "$ONTO" && ONTO=$UPSTREAM
shift
else
+ UPSTREAM=
UPSTREAM_ARG=--root
test -z "$ONTO" &&
die "You must specify --onto when using --root"
@@ -611,7 +612,12 @@ first and then run 'git rebase --continue' again."
echo "detached HEAD" > "$DOTEST"/head-name
echo $HEAD > "$DOTEST"/head
- echo $UPSTREAM > "$DOTEST"/upstream
+ case "$REBASE_ROOT" in
+ '')
+ rm -f "$DOTEST"/rebase-root ;;
+ *)
+ : >"$DOTEST"/rebase-root ;;
+ esac
echo $ONTO > "$DOTEST"/onto
test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
test t = "$VERBOSE" && : > "$DOTEST"/verbose
diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
index 6359580..29bb6d0 100755
--- a/t/t3412-rebase-root.sh
+++ b/t/t3412-rebase-root.sh
@@ -184,4 +184,92 @@ test_expect_success 'pre-rebase hook stops rebase -i' '
test 0 = $(git rev-list other...stops2 | wc -l)
'
+test_expect_success 'remove pre-rebase hook' '
+ rm -f .git/hooks/pre-rebase
+'
+
+test_expect_success 'set up a conflict' '
+ git checkout master &&
+ echo conflict > B &&
+ git add B &&
+ git commit -m conflict
+'
+
+test_expect_success 'rebase --root with conflict (first part)' '
+ git checkout -b conflict1 other &&
+ test_must_fail git rebase --root --onto master &&
+ git ls-files -u | grep "B$"
+'
+
+test_expect_success 'fix the conflict' '
+ echo 3 > B &&
+ git add B
+'
+
+cat > expect-conflict <<EOF
+6
+5
+4
+3
+conflict
+2
+1
+EOF
+
+test_expect_success 'rebase --root with conflict (second part)' '
+ git rebase --continue &&
+ git log --pretty=tformat:"%s" > conflict1 &&
+ test_cmp expect-conflict conflict1
+'
+
+test_expect_success 'rebase -i --root with conflict (first part)' '
+ git checkout -b conflict2 other &&
+ GIT_EDITOR=: test_must_fail git rebase -i --root --onto master &&
+ git ls-files -u | grep "B$"
+'
+
+test_expect_success 'fix the conflict' '
+ echo 3 > B &&
+ git add B
+'
+
+test_expect_success 'rebase -i --root with conflict (second part)' '
+ git rebase --continue &&
+ git log --pretty=tformat:"%s" > conflict2 &&
+ test_cmp expect-conflict conflict2
+'
+
+sed 's/#/ /g' > expect-conflict-p <<'EOF'
+* Merge branch 'third' into other
+|\##
+| * 6
+* | Merge branch 'side' into other
+|\ \##
+| * | 5
+* | | 4
+|/ /##
+* | 3
+|/##
+* conflict
+* 2
+* 1
+EOF
+
+test_expect_success 'rebase -i -p --root with conflict (first part)' '
+ git checkout -b conflict3 other &&
+ GIT_EDITOR=: test_must_fail git rebase -i -p --root --onto master &&
+ git ls-files -u | grep "B$"
+'
+
+test_expect_success 'fix the conflict' '
+ echo 3 > B &&
+ git add B
+'
+
+test_expect_success 'rebase -i -p --root with conflict (second part)' '
+ git rebase --continue &&
+ git log --graph --topo-order --pretty=tformat:"%s" > conflict3 &&
+ test_cmp expect-conflict-p conflict3
+'
+
test_done
--
1.6.1.469.g6f3d5
^ permalink raw reply related
* Re: [PATCH] diff.c: output correct index lines for a split diff
From: Jeff King @ 2009-01-26 9:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pixel, git
In-Reply-To: <7vhc3m8o0b.fsf_-_@gitster.siamese.dyndns.org>
On Mon, Jan 26, 2009 at 12:33:56AM -0800, Junio C Hamano wrote:
> This moves the code to generate metainfo lines around, so that two
> independent sets of metainfo lines are used for the split halves.
The patch and the generated output look correct to me, so
Acked-by: Jeff King <peff@peff.net>
> I did not include your new test script here; perhaps we can add it to an
> existing typechange diff/apply test, like t4114?
I think it makes sense to add to t4114. Please squash in the test below.
One think to note, though: test 8 (binary -> symlink) shows breakage
with the current master, but test 9 (symlink -> binary) does not.
However, if you run the test under "-d" you can see that the diff has
bogus metainfo. It's just that "apply" doesn't care.
Your test fixes the test failure, and I verified manually that the diff
output is sensible. I don't think an additional test is worth it, but we
could add one that explicitly checks the metainfo if you want to be
extra paranoid.
---
diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh
index 5533492..0f185ca 100755
--- a/t/t4114-apply-typechange.sh
+++ b/t/t4114-apply-typechange.sh
@@ -25,6 +25,10 @@ test_expect_success 'setup repository and commits' '
git update-index foo &&
git commit -m "foo back to file" &&
git branch foo-back-to-file &&
+ printf "\0" > foo &&
+ git update-index foo &&
+ git commit -m "foo becomes binary" &&
+ git branch foo-becomes-binary &&
rm -f foo &&
git update-index --remove foo &&
mkdir foo &&
@@ -85,6 +89,20 @@ test_expect_success 'symlink becomes file' '
'
test_debug 'cat patch'
+test_expect_success 'binary file becomes symlink' '
+ git checkout -f foo-becomes-binary &&
+ git diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&
+ git apply --index < patch
+ '
+test_debug 'cat patch'
+
+test_expect_success 'symlink becomes binary file' '
+ git checkout -f foo-symlinked-to-bar &&
+ git diff-tree -p --binary HEAD foo-becomes-binary > patch &&
+ git apply --index < patch
+ '
+test_debug 'cat patch'
+
test_expect_success 'symlink becomes directory' '
git checkout -f foo-symlinked-to-bar &&
^ permalink raw reply related
* Re: Translations in Git release?
From: Johannes Gilger @ 2009-01-26 9:54 UTC (permalink / raw)
To: git
In-Reply-To: <60646ee10901250941s34f7accem1b74fc201e895a41@mail.gmail.com>
On 2009-01-25, Dill <sarpulhu@gmail.com> wrote:
> Is there a plan to include translations of the Documentation within
> Git or should they exist outside of the project?
My oppinion on localization of software (and its documentation) is
generally a negative one.
- People who use software like git are (in my experience) people who
have a solid foundation of english, especially when it comes to
computer-topics.
- The effort that goes into translating the vast git documentation and
keeping it up-to-date isn't small, energy better spent in other areas.
- Translating a lot of technical terms into a language like german is
really ugly and not fun to read. I always prefer reading english
documentation and using non-localized versions of programs as it enables
me to easier partake in discussions about it and also enables me to
google for error messages without trying every different language the
message could be in ;)
But thats just me, if you want to start a translation effort knock
yourself out
Greetings,
Jojo
--
Johannes Gilger <heipei@hackvalue.de>
http://hackvalue.de/heipei/
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81 882C 58EE B178 42F6 DE81
^ permalink raw reply
* Re: [PATCH] cygwin: Convert paths for html help from posix to windows
From: Steffen Jaeckel @ 2009-01-26 10:05 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Björn Steinbrink, Junio C Hamano, git
In-Reply-To: <497B632B.1060801@ramsay1.demon.co.uk>
-----Original Message-----
From: Ramsay Jones [mailto:ramsay@ramsay1.demon.co.uk]
> Hi Björn,
> I had the same problem. However, rather than modifying git, I created a
> firefox wrapper script (in ~/bin) which used the cygpath command line
> tool to do the path conversion. Also, if you use "git instaweb", you
> also need to filter out http URLs and pass them through un-molested
> by cygpath (it turns http://localhost into http:\localhost).
> My script is clearly a "quick hack" just to get something working for
> me, but you may find it useful as a starting point for your own
> ("proper" ;-) script, so I've included it below.
> HTH,
> Ramsay Jones
Hi Ramsay,
thanks for this idea. I solved the problem by using the bash script
included below.
Cheerz,
Steffen
>sj
#!/bin/sh
#
ff="/cygdrive/c/Programme/Mozilla Firefox/firefox.exe"
while test $# != 0
do
case "$1" in
-version)
echo $("$ff" "-version")
exit 0
;;
-new-tab)
p=
case "$2" in
http*)
p="$2"
;;
*)
# check if file exists
if [ -e "$2" ]
then
p="$(cygpath -w "$2")"
fi
;;
esac
# check if $p has been set, otherwise exit with error
if [ "$p" ]
then
$("$ff" "$p")
exit 0
else
exit 1
fi
;;
esac
shift
done
exit 1
--
Steffen Jaeckel
Steinbeis-Transferzentrum/Steinbeis-Innovationszentrum
Embedded Design und Networking
an der Berufsakademie Lörrach
Poststraße 35, 79423 Heitersheim
Leiter: Prof. Dr.-Ing. Axel Sikora
Phone: +49 7634 6949341
Mob : +49 170 2328968
Fax : +49 7634 5049886
www.stzedn.de
HINWEIS
Das Steinbeis Transferzentrum Embedded Design und Networking (stzedn)
an der Dualen Hochschule Baden-Württemberg/Berufsakademie Lörrach wird
vom 3.-5.3.2009 auf der Embedded World 2009 in Nürnberg mit einem Stand
vertreten sein. Bitte besuchen Sie uns in Halle 12 Stand 322h.
Zentrale:
Steinbeis GmbH & Co. KG für Technologietransfer
Willi-Bleicher-Straße 19, 70174 Stuttgart
Registergericht Stuttgart HRA 12 480
Komplementär: Steinbeis-Verwaltung-GmbH, Registergericht Stuttgart HRB 18715
Geschäftsführer: Prof. Dr. Heinz Trasch, Prof. Dr. Michael Auer
Der Inhalt dieser E-Mail einschließlich aller Anhänge ist vertraulich und
ausschließlich für den bezeichneten Adressaten bestimmt. Wenn Sie nicht der
vorgesehene Adressat dieser E-Mail oder dessen Vertreter sein sollten, so
beachten Sie bitte, dass jede Form der Kenntnisnahme, Veröffentlichung,
Vervielfältigung oder Weitergabe des Inhalts dieser E-Mail unzulässig ist.
Wir bitten Sie, sich in diesem Fall mit dem Absender der E-Mail in Verbindung
zu setzen, sowie die Originalnachricht zu löschen und alle Kopien hiervon zu
vernichten.
This e-mail message including any attachments is for the sole use of the
intended recipient(s) and may contain privileged or confidential information.
Any unauthorized review, use, disclosure or distribution is prohibited. If you
are not the intended recipient, please immediately contact the sender by reply
e-mail and delete the original message and destroy all copies thereof.
^ permalink raw reply
* Re: Translations in Git release?
From: Peter Krefting @ 2009-01-26 10:07 UTC (permalink / raw)
To: Johannes Gilger; +Cc: git
In-Reply-To: <glk19g$2f5$1@ger.gmane.org>
Johannes Gilger:
> My oppinion on localization of software (and its documentation) is
> generally a negative one.
That's the normal response from tech-savvy people. They usually dislike
translations because they think it cannot convey the same ideas as the
original.
However, for a lot of less techy people, having to use software and
read documentation in a non-native language *is* a big hurdle for using
computers. That is especially true when it comes to complex software,
such as Git.
I would very much like to see the core git commands translated. The
command-line svn client already talks Swedish to me (cvs does not,
though), and I would be very happy to teach git the same. I already did
translate git-gui and gitk, which was as much for my own benefit as
others.
--
\\// Peter - http://www.softwolves.pp.se/
^ permalink raw reply
* Re: [PATCH] http-push: refactor request url creation
From: Johannes Schindelin @ 2009-01-26 10:41 UTC (permalink / raw)
To: Ray Chuan; +Cc: Junio C Hamano, git
In-Reply-To: <be6fef0d0901251752p5b34c053pb24dce8a35b06fce@mail.gmail.com>
Hi,
On Mon, 26 Jan 2009, Ray Chuan wrote:
> On Mon, Jan 26, 2009 at 4:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> >> @@ -304,17 +312,7 @@ static void start_fetch_loose(struct
> >> transfer_request *request)
> >>
> >> git_SHA1_Init(&request->c);
> >>
> >> - url = xmalloc(strlen(remote->url) + 50);
> >> ...
> >> - strcpy(request->url, url);
> >> + request->url = get_remote_object_url(remote->url, hex, 0);
> >> ...
> >> - curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> >> + curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
> >
> > The original code gave a separate "url" to setop() but this gives the same
> > string. Does curl_easy_setop() copies the given string away? IOW is this
> > change safe?
> >
>
> curl strdup's it, so this is safe.
I might have mentioned that things like this _need_ to go into the commit
message.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Johannes Schindelin @ 2009-01-26 10:54 UTC (permalink / raw)
To: Jeff King; +Cc: Keith Cascio, Junio C Hamano, git
In-Reply-To: <20090126031206.GB14277@sigill.intra.peff.net>
Hi,
On Sun, 25 Jan 2009, Jeff King wrote:
> So if you just want this from the command line, then I think it is safe
> to have "git diff" always respect "diff.primer", and scripts shouldn't
> be impacted.
But as Keith made clear, he wanted to use it from _git-gui_. Which
-- naturally -- _has_ to use plumbing, to guarantee a stable interface.
So "fixing" this in "git diff" is the wrong place; anything else than
teaching "git gui" to remember user-defined diff options and to use them
would be a complicator's glove.
Ciao,
Dscho
^ permalink raw reply
* backwards compatibility, was Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Johannes Schindelin @ 2009-01-26 10:59 UTC (permalink / raw)
To: Jeff King; +Cc: Keith Cascio, Junio C Hamano, git
In-Reply-To: <20090126031206.GB14277@sigill.intra.peff.net>
Hi,
On Sun, 25 Jan 2009, Jeff King wrote:
> 1. Sometimes we blur the line of plumbing and porcelain, where
> functionality is available only through plumbing. For example,
> gitweb until recently called "git diff" because there is no other
> way to diff two arbitrary blobs. But the solution there is, I
> think, to make that functionality available through plumbing. Not
> to disallow enhancements to porcelain.
Just a reminder: we are very conservative when it comes to breaking
backwards compatibility. For example, people running (but not upgrading)
gitweb who want to upgrade Git may rightfully expect their setups not to
be broken for a long time, if ever.
So your mentioning gitweb using "git diff" precludes all kind of cute
games, methinks.
And please no "anybody who would do this and that would be nuts" excuses:
if you want to change something fundamental like this, _you_ have to
defend it.
It is not acceptable to just shout out what you want and expect those
affected negatively to do the impact analysis for you.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 08/10] run test suite without dashed git-commands in PATH
From: Johannes Schindelin @ 2009-01-26 11:06 UTC (permalink / raw)
To: Matthew Ogilvie; +Cc: git
In-Reply-To: <20090126064004.GA3004@comcast.net>
Hi,
On Sun, 25 Jan 2009, Matthew Ogilvie wrote:
> On Sun, Jan 25, 2009 at 02:59:53AM +0100, Johannes Schindelin wrote:
>
> > On Sat, 24 Jan 2009, Matthew Ogilvie wrote:
> >
> > > .gitignore | 1 +
> > > Makefile | 42 +++++++++++++++++++++++++++++++-----------
> > > t/test-lib.sh | 14 +++++++++++++-
> > > test-bin-wrapper.sh | 12 ++++++++++++
> > > 4 files changed, 57 insertions(+), 12 deletions(-)
> > > create mode 100644 test-bin-wrapper.sh
> >
> > I am strongly opposed to a patch this big, just for something as 3rd
> > class as CVS server faking. We already have a big fallout from all
> > that bending over for Windows support, and I do not like it at all.
> >
> > Note: I do not even have to look further than the diffstat to see that
> > it is wrong.
> >
> > The point is: if cvsserver wants to pretend that it is in a fake bin
> > where almost none of the other Git programs are, fine, let's do that
> > _in the test for cvsserver_.
>
> Since by default git is installed such that most of the dashed-form
> commands are not in a user's default PATH, my thought was that it would
> make sense for the test suite to mimick that environment as much as
> possible.
This sounds very generic, but you hid it in cvsserver-specific patch
series.
So maybe I was wrong to assume that this is cvsserver specific, but then,
you made that mistake rather easy to make.
> This could detect regressions in any installed/tested git script that
> erroneously assumes the dashed form commands are in the PATH, not just
> git-cvsserver.
The major point is that these scripts _will_ run if you call _them_ using
the dash-less form, as GIT_EXEC_PATH will be added to the PATH by the Git
wrapper.
> 3. Perhaps just use symlinks or hardlinks instead of a wrapper
> script. This might have some promise, except that links are more
> likely to fail on windows, and the wrappers generally give you
> more flexibility for testing odd scenarios.
Not likely. Sure as hell.
> 4. The test-bin-wrapper.sh script does not actually need to set
> environment variables (GIT_EXEC_DIT and templates) for purposes
> of this patch. But my thought was that in this form you could
> run things straight out of the test-bin directory to manually try
> out new code without needing to actually install a build or mess
> with the environment variables yourself. It could also be
> extended to handle other global wrapper needs relatively easily,
> such as valgrind.
Umm.
You missed the valgrind patch series.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-26 11:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Keith Cascio, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901261152320.14855@racer>
On Mon, Jan 26, 2009 at 11:54:26AM +0100, Johannes Schindelin wrote:
> > So if you just want this from the command line, then I think it is safe
> > to have "git diff" always respect "diff.primer", and scripts shouldn't
> > be impacted.
>
> But as Keith made clear, he wanted to use it from _git-gui_. Which
> -- naturally -- _has_ to use plumbing, to guarantee a stable interface.
>
> So "fixing" this in "git diff" is the wrong place; anything else than
> teaching "git gui" to remember user-defined diff options and to use them
> would be a complicator's glove.
I think what you are missing here is that he specifically mentioned the
command line, and I was responding to those comments. There are two
separate problems: default options for command line usage and the
mechanism by which one can set options for things like git-gui.
In this case he was asking specifically about "git diff" from the
command line, so fixing it in there _is_ the place to fix it (the only
other alternative being to make a wrapper or alias).
For the other problem, it _may_ benefit from tool support that would
help porcelains respect a "diff.primer" variable like Keith proposed.
But that has already been discussed elsewhere in the thread, so I'm not
going to repeat it here.
-Peff
^ permalink raw reply
* Re: backwards compatibility, was Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Jeff King @ 2009-01-26 11:16 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Keith Cascio, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901261154330.14855@racer>
On Mon, Jan 26, 2009 at 11:59:46AM +0100, Johannes Schindelin wrote:
> Just a reminder: we are very conservative when it comes to breaking
> backwards compatibility. For example, people running (but not upgrading)
> gitweb who want to upgrade Git may rightfully expect their setups not to
> be broken for a long time, if ever.
>
> So your mentioning gitweb using "git diff" precludes all kind of cute
> games, methinks.
Are you aware that gitweb no longer calls "git diff", exactly because
of problems caused by calling a porcelain from a script?
I don't want to break existing setups, either. But at some point you
have to say "this is porcelain, so don't rely on there not being any
user-triggered effects in its behavior". If porcelain is cast in stone,
then what is the point in differentiating plumbing from porcelain?
And when the line is blurred (as I think it is in several places), then
it has to be dealt with on a case-by-case basis. What is the benefit,
and what is the likelihood and extent of harm?
> And please no "anybody who would do this and that would be nuts" excuses:
> if you want to change something fundamental like this, _you_ have to
> defend it.
>
> It is not acceptable to just shout out what you want and expect those
> affected negatively to do the impact analysis for you.
This message is addressed to me, but I don't know exactly what you think
I'm proposing, failing to defend, or failing to do an impact analysis
for. Or are you speaking generally of the "you" who submit patches?
-Peff
^ permalink raw reply
* Re: [PATCH] Reintegrate script
From: Johannes Schindelin @ 2009-01-26 11:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4ea8mnf.fsf@gitster.siamese.dyndns.org>
Hi,
On Mon, 26 Jan 2009, Junio C Hamano wrote:
> In a workflow that uses topic branches heavily, you would need to keep
> updating test integration branch(es) all the time. If they are managed
> like my 'next' by merging the tips of topics that have grown since the
> last integration, it is not so difficult. You only need to review
> output from "git branch --no-merged next" to see if there are topics
> that can and needs to be merged to 'next'.
>
> But sometimes it is easier to rebuild a test integration branch from
> scratch all the time, especially if you do not publish it for others to
> build on.
FWIW that is exactly what I am aiming at with my rebase -i -p work.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] gitweb: check if-modified-since for feeds
From: Giuseppe Bilotta @ 2009-01-26 11:25 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200901260318.05301.jnareb@gmail.com>
On Mon, Jan 26, 2009 at 3:18 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> Should be "[PATCH 2/2]" or similar, just in case.
Yeah, but I was just hacking up ideas as they came. I'm planning a
resend of the whole series, properly numbered and all.
> On Sun, 25 Jun 2009, Giuseppe Bilotta wrote:
>
>> Offering Last-modified header
>
> And skipping generating the body if client uses 'HEAD' request to
> get only Last-Modified header.
>
>> for feeds is only half the work: we should
>> also check that same date against If-modified-since, and bail out early
>> with 304 Not Modified.
>
> Lacks signoff.
Oh yeah.
>> - %latest_date = parse_date($latest_commit{'committer_epoch'});
>> + my $latest_epoch = $latest_commit{'committer_epoch'};
>> + %latest_date = parse_date($latest_epoch);
>> + my $if_modified = $cgi->http('IF_MODIFIED_SINCE');
>> + if (defined $if_modified) {
>> + my $since;
>> + if (eval { require HTTP::Date; 1; }) {
>> + $since = HTTP::Date::str2time($if_modified);
>> + } elsif (eval { require Time::ParseDate; 1; }) {
>> + $since = Time::ParseDate::parsedate($if_modified, GMT => 1);
>> + }
>
> I'd really like to fallback on hand-parsing, as we have to parse date
> in well defined HTTP-date format (RFC-1123, update to RFC-822), which
> I think is what we send in Last-Modified header (or is it RFC-2822?).
>
> But that might be too much work. I like the checking for modules,
> and the fallback cascade, but could you explain why in this order?
Of course, if we have our own parsing code, we don't need the other
modules. I'm way too lazy to write the parsing code myself, although a
copypaste from existing GPL code would do it.
(BTW, I asked on #perl and they think gitweb non-reliance on CPAN
makes for some very horrible code. Of course, IMO the real problem is
that perl's stdlib is way too limited, but that is likely to causes a
language war so I refrained from discussing the thing.)
The order is almost casual, but I suspect that HTTP::Date, from
libwww-perl, is more likely to be available on a webserver than the
other.
>> + if (defined $since && $latest_epoch <= $since) {
>> + print $cgi->header(
>> + -type => $content_type,
>> + -charset => 'utf-8',
>> + -last_modified => $latest_date{'rfc2822'},
>> + -status => 304);
>
> I think we spell HTTP status messages in full (even if it is hidden
> in die_error subroutine), i.e.
>
> + -status => '304 Not Modified');
Can do that.
> P.S. It would be nice to have this mechanism (responding to
> cache-control headers such as If-Modified-Since) for all of gitweb,
> but I guess it is most critical for feeds, which are _polled_.
I thought so too, but then again I couldn't see where last-modified
was used. (Its usage could be added, of course.)
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: backwards compatibility, was Re: [PATCH v1 1/3] Introduce config variable "diff.primer"
From: Johannes Schindelin @ 2009-01-26 11:28 UTC (permalink / raw)
To: Jeff King; +Cc: Keith Cascio, Junio C Hamano, git
In-Reply-To: <20090126111605.GB19993@coredump.intra.peff.net>
Hi,
On Mon, 26 Jan 2009, Jeff King wrote:
> On Mon, Jan 26, 2009 at 11:59:46AM +0100, Johannes Schindelin wrote:
>
> > Just a reminder: we are very conservative when it comes to breaking
> > backwards compatibility. For example, people running (but not upgrading)
> > gitweb who want to upgrade Git may rightfully expect their setups not to
> > be broken for a long time, if ever.
>
> Are you aware that gitweb no longer calls "git diff", exactly because
> of problems caused by calling a porcelain from a script?
As I said: do you really expect people not to forget to upgrade gitweb
manually when they do "sudo make install" with a new Git version?
> I don't want to break existing setups, either. But at some point you
> have to say "this is porcelain, so don't rely on there not being any
> user-triggered effects in its behavior". If porcelain is cast in stone,
> then what is the point in differentiating plumbing from porcelain?
Two points there:
- with gitweb, we were the offenders ourselves. So we should give the
users of gitweb at least _some_ slack.
- Concretely for the "porcelain" git diff: This workflow
git diff > my-patch
<attach and send to somebody>
is probably pretty wide spread. And it is okay, a user is not a script,
they are very much allowed to use porcelain. And we _would_ break
expectations there.
Now, I have another two, fundamental problems with the diff options
defaults: you are restricting the thing to _one_ set of options, and when
somebody wants to run without those options, she has to actively _undo_
them.
Remember, sometimes you need another set of options. Like, when I send
mail to a Git user, I want "-M -C -C", when I send mail to a non-Git user,
I do not want any additional options (and try to undo "-M -C -C" on the
command line, good luck), and sometimes it is much easier to see what
happened with a word diff.
So what I need are three different sets of diff options.
Guess how well that works with aliases -- we are talking command line
here after all, right?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] gitweb: ensure the default stylesheet is accessible
From: Giuseppe Bilotta @ 2009-01-26 11:35 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Junio C Hamano
In-Reply-To: <200901260248.22120.jnareb@gmail.com>
On Mon, Jan 26, 2009 at 2:48 AM, Jakub Narebski <jnareb@gmail.com> wrote:
> On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:
>
>> On some installations the CSS fails to be linked correctly when
>> path_info is enabled, since the link refers to "gitweb.css", whereas it
>> should be "${my_uri}/gitweb.css". Fix by setting the appropriate default
>> in the Makefile.
>
> Why "on some installations"? What does "some" mean? I don't think it
> is something indeterministic: please spell when one can have problems
> with linking CSS file.
The truth is, I haven't the slightest idea. It works fine on my
machine, it doesn't without the patch on ruby-rbot.org, but I really
don't know why.
> Wouldn't it be simpler to deal with problem of base URL when using
> path_info gitweb URLs to add BASE element to HTML head if we use
> path_info? Something like:
>
> if ($ENV{'PATH_INFO'}) { # $path_info is unfortunately stripped
> print qq(<base href="$my_uri">\n);
> }
>
> somewhere in git_header_html() subroutine?
Ah, this might work. I'll test it.
> It is not the same case for git-logo.png and git-favicon.png as for
> gitweb.css? If it is not, please explain why in commit message.
> If it is, then your patch is only partial solution to path_info
> problem.
Oh, interesting, true, I hadn't noticed.
I'll look into the base thing.
--
Giuseppe "Oblomov" Bilotta
^ 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