Re: git-mergetool reverse file ordering
Am 17.08.2016 um 08:46 schrieb David Aguilar: The only thing that using diff-files doesn't address is the rerere support in mergetool where it processes the files in the order specified by "git rerere remaining". This is why I initially thought we needed a generic sort-like command. I see. This is actually an important code path. How about this code structure: if test $# -eq 0 then cd_to_toplevel if test -e "$GIT_DIR/MERGE_RR" then set -- $(git rerere remaining) fi fi files=$(git diff-files --name-only --diff-filter=U -- "$@") This does not require an enhancement of rerere-remaining and still captures all three cases that currently go through separate branches. (Throw in some version of --ignore-submodules= if necessary, but I guess it is not.) We do have a problem if there are file names with spaces, but it is not a new problem. The patches could then be: 1. switch to diff-files, add tests, and document how diff.orderFile affects mergetool. 2. Teach mergetool about the "-O" flag so that it can override the configuration, and add tests. It could be argued that this should be squashed into (1). 3. (optional) teach "rerere remaining" to honor the -O flag and teach mergetool to supply the option. Sound good? Sure, except that 3. won't be necessary. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path
Junio C Hamano writes: > Remi Galan Alfonso > writes: > >> Johannes Schindelin writes: >>> Hi Rémi, >>> >>> On Tue, 16 Aug 2016, Remi Galan Alfonso wrote: >>> >>> > Johannes Schindelin writes: >>> > > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh >>> > > index 5e3fb3a..f1f9aee 100755 >>> > > --- a/t/t1350-config-hooks-path.sh >>> > > +++ b/t/t1350-config-hooks-path.sh >>> > > @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of >>> > > specifying core.hooksPath work' >>> > > test_cmp expect actual >>> > > ' >>> > > >>> > > +test_expect_success 'git rev-parse --git-path hooks' ' >>> > > +git config core.hooksPath .git/custom-hooks && >>> > >>> > Any reason to not use 'test_config' here? >>> >>> Yes: consistency. The rest of the script uses `git config`, not >>> `test_config`. >> >> Fine by me, then. Sorry for the noise. > > No, thanks for reviewing. I'll take Dscho's patch as-is but once it > hits 'next', it probably is a good idea to do a separate clean-up > patch on top to use test_config where necessary. > > Having said that, this entire script is about constantly changing > the value of that single configuration variable and see how the code > performs, so any new test added after existing ones is expected to > ignore left-over values in the variable and set it to a value of its > own liking. So I suspect there is no existing "git config" call in > this script, with or without Dscho's patch, that would benefit from > getting converted to test_config. Thanks for checking the ones in this file, considering the lack of benefits it might not be worth it to change it for now. I tried to see if the `git config` in other tests were in the same case or not but the sheer amount made me reconsider. However taking a look at a couple of random ones, there are some scripts that would benefit from the conversion. For example in t3200-branch there is: test_expect_success 'git branch with column.*' ' git config column.ui column && git config column.branch "dense" && COLUMNS=80 git branch >actual && git config --unset column.branch && git config --unset column.ui && cat >expected <<\EOF && a/b/c bam foo l * mastern o/p r abc bar j/k m/m master2 o/o q EOF test_cmp expected actual ' The conversion would drop 2 lines in this particular case and would avoid bleeding the config should `git branch` fail (not sure how possible that is...). (I can make a patch for t3200 but that won't be before days/weeks, so if someone else wants to take care of it I won't mind) If I have some time to kill, I'll try looking at a few others but I won't expect that any time soon. Thanks, Rémi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits
Hi,
On Tue, 16 Aug 2016, Remi Galan Alfonso wrote:
> Stephen Morton writes:
> > +if (multiple_commits)
> > + advise(_("after resolving the conflicts,
> > mark the corrected paths with 'git add ' or 'git rm '\n"
> > +"then continue with 'git %s
> > --continue'\n"
> > +"or cancel with 'git %s
> > --abort'" ), action_name(opts), action_name(opts));
> > +else
> > +advise(_("after resolving the
> > conflicts, mark the corrected paths\n"
> > +"with 'git add ' or 'git
> > rm '\n"
> > +"and commit the result with
> > 'git commit'"));
>
> In both cases (multiple_commits or not), the beginning of the advise
> is nearly the same, with only a '\n' in the middle being the
> difference:
>
> multiple_commits:
> "after resolving the conflicts, mark the corrected paths with 'git
> add ' or 'git rm '\n"
>
> !multiple_commits:
> "after resolving the conflicts, mark the corrected paths\n with 'git
> add ' or 'git rm '\n"
> ~~~^
>
> In 'multiple_commits' case the advise is more than 80 characters long,
> did you forget the '\n' in that case?
>
> If you end up using the same beginning of advice, maybe it's possible
> to give it before the 'if(multiple_commits)' and avoid duplication of
> the lines.
I concur with this, and also with Christian's advice to append the
parameter consistently as last one, and also with renaming it to
"last_commit".
Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Working with zip files
On Tue, 16 Aug 2016, Nikolaus Rath wrote: On Aug 16 2016, David Lang wrote: On Tue, 16 Aug 2016, Nikolaus Rath wrote: I would like to store Simulink models in a Git repository. Unfortunately, the file format is binary. But luckily, the binary format happens to be a zipfile containing nicely formatted XML files. Is there a way to teach Git to take advantage of this when storing, diff-ing and merging these files? you should be able to use clean/smudge to have git store the files uncompressed, which will help a lot. Having looked at that, I'm not sure if this really helps: As I understand, the smudge command is run on checkout to convert the blob in the repository to the format that is desired in the working tree. But this is the opposite of what I need: on checkout, I need to convert the text data in the repository to a blob in the working tree. Furthermore, I need to convert multiple text files into one blob, will smudge/clean seem to do just 1:1 conversions. Am I missing something? Are there any other options? so the smudge command would zip the file and the clean command would unzip the file (assuming it's a single file, if the zip is multiple files, you will have to add something to combine them) you want the working tree to have a zip file and the repository to have text. David Lang -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git Rev News edition 18
Hi everyone, I'm happy announce that the 18th edition of Git Rev News is now published: https://git.github.io/rev_news/2016/08/17/edition-18/ Thanks a lot to all the contributors and helpers, especially Lars, Dscho, Jakub, Roberto and Josh! Enjoy, Christian and Thomas. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #06; Sun, 14)
On Mon, Aug 15, 2016 at 5:46 AM, Junio C Hamano wrote: > * dt/index-helper (2016-07-06) 21 commits > > A new "index-helper" daemon has been introduced to give newly > spawned Git process a quicker access to the data in the index, and > optionally interface with the watchman daemon to further reduce the > refresh cost. > > Not quite ready yet, it seems. > cf. > cf. David I can take back this series if you are busy or no longer interested in it. If so, Junio, since I may try some slightly different direction first, it may take a while before I resubmit, feel free to drop it if it adds work to you. > * nd/shallow-deepen (2016-06-13) 27 commits > > The existing "git fetch --depth=" option was hard to use > correctly when making the history of an existing shallow clone > deeper. A new option, "--deepen=", has been added to make this > easier to use. "git clone" also learned "--shallow-since=" > and "--shallow-exclude=" options to make it easier to specify > "I am interested only in the recent N months worth of history" and > "Give me only the history since that version". > > Needs review. > > Rerolled. What this topic attempts to achieve is worthwhile, I > would think. I guess I can't do anything to help speed this up? -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] document how to reference previous commits
Hi, On Thu, Jul 28, 2016 at 02:55:14PM +0200, Heiko Voigt wrote: > To reference previous commits people used to put just the abbreviated > SHA-1 into commit messages. This is what has evolved as a more > stable format for referencing commits. So lets document it for everyone > to lookup when needed. A quick ping about this patch. Maybe you missed to include it Junio? I can not find any reference to it in the cooking mails and in your repository. Cheers Heiko -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Do not lock temporary files via child processes on Windows
This issue was originally reported and fixed in https://github.com/git-for-windows/git/pull/755 The problem is that file handles to temporary files (such as index.lock) were inherited by spawned processes. If those spawned processes do not exit before the parent process wants to delete or rename them, we are in big trouble. The original use case triggering the bug is a merge driver that does not quit, but listen to subsequent merge requests. However, the same issue turned up in Lars Schneider's work on making clean/smudge filters batchable (i.e. more efficient by avoiding possibly thousands of child processes, one per file). Ben Wijen (2): t6026-merge-attr: child processes must not inherit index.lock handles mingw: ensure temporary file handles are not inherited by child processes compat/mingw.h| 4 t/t6026-merge-attr.sh | 13 + tempfile.c| 2 +- 3 files changed, 18 insertions(+), 1 deletion(-) Published-As: https://github.com/dscho/git/releases/tag/mingw-index-lock-v1 Fetch-It-Via: git fetch https://github.com/dscho/git mingw-index-lock-v1 -- 2.9.2.691.g78954f3 base-commit: 07c92928f2b782330df6e78dd9d019e984d820a7 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t6026-merge-attr: child processes must not inherit index.lock handles
From: Ben Wijen On Windows, files cannot be removed unless all file handles to it have been released. Hence it is particularly important to close handles when spawning children (which would probably not even know that they hold on to those handles). The example chosen for this test is a custom merge driver that indeed has no idea that it blocks the deletion of index.lock. The full use case is a daemon that lives on after the merge, with subsequent invocations handing off to the daemon, thereby avoiding hefty start-up costs. We simulate this behavior by simply sleeping one second. Note that the test only fails on Windows, due to the file locking issue. Since we have no way to say "expect failure with MINGW, success otherwise", we simply skip this test on Windows for now. Signed-off-by: Ben Wijen Signed-off-by: Johannes Schindelin --- t/t6026-merge-attr.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh index ef0cbce..3d28c78 100755 --- a/t/t6026-merge-attr.sh +++ b/t/t6026-merge-attr.sh @@ -181,4 +181,17 @@ test_expect_success 'up-to-date merge without common ancestor' ' ) ' +test_expect_success !MINGW 'custom merge does not lock index' ' + git reset --hard anchor && + write_script sleep-one-second.sh <<-\EOF && + sleep 1 & + EOF + + test_write_lines >.gitattributes \ + "* merge=ours" "text merge=sleep-one-second" && + test_config merge.ours.driver true && + test_config merge.sleep-one-second.driver ./sleep-one-second.sh && + git merge master +' + test_done -- 2.9.2.691.g78954f3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes
From: Ben Wijen
When the index is locked and child processes inherit the handle to
said lock and the parent process wants to remove the lock before the
child process exits, on Windows there is a problem: it won't work
because files cannot be deleted if a process holds a handle on them.
The symptom:
Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed.
Should I try again? (y/n)
Spawning child processes with bInheritHandles==FALSE would not work
because no file handles would be inherited, not even the hStdXxx
handles in STARTUPINFO (stdin/stdout/stderr).
Opening every file with O_NOINHERIT does not work, either, as e.g.
git-upload-pack expects inherited file handles.
This leaves us with the only way out: creating temp files with the
O_NOINHERIT flag. This flag is Windows-specific, however. For our
purposes, it is equivalent our purposes) to O_CLOEXEC (which does not
exist on Windows), so let's just open temporary files with the
O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows.
This fixes the test that we just introduced to demonstrate the problem.
Signed-off-by: Ben Wijen
Signed-off-by: Johannes Schindelin
---
compat/mingw.h| 4
t/t6026-merge-attr.sh | 2 +-
tempfile.c| 2 +-
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 95e128f..753e641 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -67,6 +67,10 @@ typedef int pid_t;
#define F_SETFD 2
#define FD_CLOEXEC 0x1
+#if !defined O_CLOEXEC && defined O_NOINHERIT
+#define O_CLOEXEC O_NOINHERIT
+#endif
+
#ifndef EAFNOSUPPORT
#define EAFNOSUPPORT WSAEAFNOSUPPORT
#endif
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 3d28c78..dd8f88d 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -181,7 +181,7 @@ test_expect_success 'up-to-date merge without common
ancestor' '
)
'
-test_expect_success !MINGW 'custom merge does not lock index' '
+test_expect_success 'custom merge does not lock index' '
git reset --hard anchor &&
write_script sleep-one-second.sh <<-\EOF &&
sleep 1 &
diff --git a/tempfile.c b/tempfile.c
index 0af7ebf..db3981d 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -120,7 +120,7 @@ int create_tempfile(struct tempfile *tempfile, const char
*path)
prepare_tempfile_object(tempfile);
strbuf_add_absolute_path(&tempfile->filename, path);
- tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL,
0666);
+ tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL |
O_CLOEXEC, 0666);
if (tempfile->fd < 0) {
strbuf_reset(&tempfile->filename);
return -1;
--
2.9.2.691.g78954f3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/1] convert: Correct NNO tests and missing `LF will be replaced by CRLF`
Hi Torsten, On Sat, 13 Aug 2016, tbo...@web.de wrote: > From: Torsten Bögershausen > > Change since v1: > - The changes done in 1/2 in t0027 needed to be reverted in 2/2. > Put both changes for convert.c and t0027 into a single commit > Thanks to Johannes Sixt. > Torsten Bögershausen (1): > convert: Correct NNO tests and missing `LF will be replaced by CRLF` Thank you so much for the fix! Ciao, Dscho
Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes
On Wed, Aug 17, 2016 at 8:41 AM, Johannes Schindelin wrote: > When the index is locked and child processes inherit the handle to > said lock and the parent process wants to remove the lock before the > child process exits, on Windows there is a problem: it won't work > because files cannot be deleted if a process holds a handle on them. > The symptom: > > Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed. > Should I try again? (y/n) > > Spawning child processes with bInheritHandles==FALSE would not work > because no file handles would be inherited, not even the hStdXxx > handles in STARTUPINFO (stdin/stdout/stderr). > > Opening every file with O_NOINHERIT does not work, either, as e.g. > git-upload-pack expects inherited file handles. > > This leaves us with the only way out: creating temp files with the > O_NOINHERIT flag. This flag is Windows-specific, however. For our > purposes, it is equivalent our purposes) to O_CLOEXEC (which does not s/our purposes)// > exist on Windows), so let's just open temporary files with the > O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows. > > This fixes the test that we just introduced to demonstrate the problem. > > Signed-off-by: Ben Wijen > Signed-off-by: Johannes Schindelin -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t3404: become resilient to GETTEXT_POISON
Hi Vasco, On Fri, 12 Aug 2016, Vasco Almeida wrote: > The concerned test greps the output of exit_with_patch() in > git-rebase--interactive.sh script. > > Signed-off-by: Vasco Almeida Thank you for keeping an eye out for these issues. I have to admit that I am a bit confused when to use i18ngrep and when not, so it is good to know that my mistakes won't survive for long! Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #06; Sun, 14)
On Wed, 2016-08-17 at 17:49 +0700, Duy Nguyen wrote: > On Mon, Aug 15, 2016 at 5:46 AM, Junio C Hamano wrote: > > * dt/index-helper (2016-07-06) 21 commits > > > > A new "index-helper" daemon has been introduced to give newly > > spawned Git process a quicker access to the data in the index, and > > optionally interface with the watchman daemon to further reduce the > > refresh cost. > > > > Not quite ready yet, it seems. > > cf. > > cf. > > David I can take back this series if you are busy or no longer interested in > it. > > If so, Junio, since I may try some slightly different direction first, > it may take a while before I resubmit, feel free to drop it if it adds > work to you. Unfortunately, I am pretty busy. So please do feel free to take over. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes
> On 17 Aug 2016, at 14:41, Johannes Schindelin > wrote: > > From: Ben Wijen > > When the index is locked and child processes inherit the handle to > said lock and the parent process wants to remove the lock before the > child process exits, on Windows there is a problem: it won't work > because files cannot be deleted if a process holds a handle on them. > The symptom: > >Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed. >Should I try again? (y/n) > > Spawning child processes with bInheritHandles==FALSE would not work > because no file handles would be inherited, not even the hStdXxx > handles in STARTUPINFO (stdin/stdout/stderr). > > Opening every file with O_NOINHERIT does not work, either, as e.g. > git-upload-pack expects inherited file handles. > > This leaves us with the only way out: creating temp files with the > O_NOINHERIT flag. This flag is Windows-specific, however. For our > purposes, it is equivalent our purposes) to O_CLOEXEC (which does not > exist on Windows), so let's just open temporary files with the > O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows. > > This fixes the test that we just introduced to demonstrate the problem. > > Signed-off-by: Ben Wijen > Signed-off-by: Johannes Schindelin > --- > compat/mingw.h| 4 > t/t6026-merge-attr.sh | 2 +- > tempfile.c| 2 +- > 3 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/compat/mingw.h b/compat/mingw.h > index 95e128f..753e641 100644 > --- a/compat/mingw.h > +++ b/compat/mingw.h > @@ -67,6 +67,10 @@ typedef int pid_t; > #define F_SETFD 2 > #define FD_CLOEXEC 0x1 > > +#if !defined O_CLOEXEC && defined O_NOINHERIT > +#define O_CLOEXECO_NOINHERIT > +#endif > + > #ifndef EAFNOSUPPORT > #define EAFNOSUPPORT WSAEAFNOSUPPORT > #endif > diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh > index 3d28c78..dd8f88d 100755 > --- a/t/t6026-merge-attr.sh > +++ b/t/t6026-merge-attr.sh > @@ -181,7 +181,7 @@ test_expect_success 'up-to-date merge without common > ancestor' ' > ) > ' > > -test_expect_success !MINGW 'custom merge does not lock index' ' > +test_expect_success 'custom merge does not lock index' ' > git reset --hard anchor && > write_script sleep-one-second.sh <<-\EOF && > sleep 1 & > diff --git a/tempfile.c b/tempfile.c > index 0af7ebf..db3981d 100644 > --- a/tempfile.c > +++ b/tempfile.c > @@ -120,7 +120,7 @@ int create_tempfile(struct tempfile *tempfile, const char > *path) > prepare_tempfile_object(tempfile); > > strbuf_add_absolute_path(&tempfile->filename, path); > - tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL, > 0666); > + tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL | > O_CLOEXEC, 0666); This solution works great for me. I struggled with a similar problem in my Git filter protocol series: http://public-inbox.org/git/20160810130411.12419-16-larsxschnei...@gmail.com/ I also tried selectively disabling inheritance for file handles but it looks like as this is not easily possible right now: https://github.com/git-for-windows/git/issues/770#issuecomment-238816331 - Lars-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits
Responding to a few comments...
On 2016-08-14 7:44 AM, Christian Couder wrote:
multiple_commits)
... but here multiple_commits is the last argument.
It would be better if it was more consistent.
(Johannes made the same comment.)
Yes. Will do.
multiple_commits = (todo_list->next) != NULL;
Why not "last_commit" instead of "multiple_commits"?
Because it *isn't*. You can see that in pick_commits(), I set
multiple_commits outside of the `for todo_list` loop. It is not
re-evaluated at every iteration of the loop. As per my comment when
emailing the patch "I intentionally print the '--continue' hint even in
the case where it's last of n commits that fails. " I think it makes
much more sense that "this is the message you always get when
cherry-picking multiple commits as opposed to "this is the message you
sometimes get, except when it's the last one". (Yes, the careful
observer will realize that if when cherry-picking multiple commits,
there are conflicts in the second-last and last then the --continue
from the second-last will result in multiple_commits being set to 0. I
can live with that.)
On 2016-08-16 4:44 AM, Remi Galan Alfonso wrote:
Hi Stephen,
Stephen Morton writes:
+if (multiple_commits)
+ advise(_("after resolving the conflicts,
mark the corrected paths with 'git add ' or 'git rm '\n"
+"then continue with 'git %s
--continue'\n"
+"or cancel with 'git %s
--abort'" ), action_name(opts), action_name(opts));
+else
+advise(_("after resolving the
conflicts, mark the corrected paths\n"
+"with 'git add ' or 'git
rm '\n"
+"and commit the result with
'git commit'"));
In both cases (multiple_commits or not), the beginning of the advise
is nearly the same, with only a '\n' in the middle being the
difference:
multiple_commits:
"after resolving the conflicts, mark the corrected paths with 'git
add ' or 'git rm '\n"
!multiple_commits:
"after resolving the conflicts, mark the corrected paths\n with 'git
add ' or 'git rm '\n"
~~~^
In 'multiple_commits' case the advise is more than 80 characters long,
did you forget the '\n' in that case?
A previous comment had indicated that having 4 lines was too many. And I
tend to agree. So I tried to squash it into 3. Back in xterm days, 80
characters was sacrosanct, but is it really a big deal to exceed it now?
On 2016-08-14 7:44 AM, Christian Couder wrote:
...but please try to send a real patch.
There is https://github.com/git/git/blob/master/Documentation/SubmittingPatches
and also SubmitGit that can help you do that.
Agreed. I just want to send a patch that stands a reasonable chance of
getting accepted.
Stephen
--
Stephen Morton, 7750 SR Product Group, SW Development Tools/DevOps
w: +1-613-784-6026 (int: 2-825-6026) m: +1-613-302-2589 | EST Time Zone
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git for Windows 2.9.3
Hi Junio, On Sat, 13 Aug 2016, Junio C Hamano wrote: > Johannes Schindelin writes: > > > New Features > > > > • Comes with Git 2.9.3. > > For future reference, what time (in UTC) of the day is convenient > for you to see an upstream tarball? Heh... I don't do tarballs anymore, I now use this newfangled tool to manage source code... "gyt" or something like that, it is called. :-) Given that between you and me there is currently a time zone difference of 9h (except for four weeks, two in spring, when it is only 8h, and two in fall, when it is 10h), I believe we cannot find a time that is convenient for both of us. But I also think it is fine, when I discover a new upstream Git version in the morning, I can spend all day on fixing any problems and on packaging the result ;-) > > • Sports a new --smudge option for git cat-file that lets it pass > > blob contents through smudge filters configured for the specified > > path. > > Perhaps we want to upstream this, together with a new "--clean" option > for git hash-object? No question about that. I just needed this in a hurry and short-circuited it into Git for Windows before submitting it upstream. > And after writing all of the above, I noticed that hash-object by > default uses the clean machinery and that can be turned off by giving > the "--no-filters" option. The reason why the option is not called > "--no-clean" is because it is not just about the clean filter but is > about using the entirety of convert_to_git() filter chain. Right, as is the --smudge option (it is about the entirety of convert_to_worktree()). > We probably should teach "hash-objects" to take "--filters" for > consistency. I actually thought about that, too. Which was one of the reasons I did not submit the patch to the Git mailing list first, as I expect several iterations to be necessary to get everything into `master`. > And then your "git cat-file" patch can be upstreamed with the option > renamed to (or with an additional synonym) "--filters", which would make > things consistent. Right. I would like to ask for a `--smudge` synonym nevertheless, just because I already use this. On the other hand, it is early enough to tell everybody who knows about this feature to change their invocation (anybody who would know about `--smudge` would be in that 1% of users that have read the release notes, so most likely would read the next release notes, too). Ciao, Dscho
Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits
Stephen Morton writes:
> [snip]
> On 2016-08-16 4:44 AM, Remi Galan Alfonso wrote:
>> Hi Stephen,
>>
>> Stephen Morton writes:
>>> +if (multiple_commits)
>>> + advise(_("after resolving the conflicts,
>>> mark the corrected paths with 'git add ' or 'git rm '\n"
>>> +"then continue with 'git %s
>>> --continue'\n"
>>> +"or cancel with 'git %s
>>> --abort'" ), action_name(opts), action_name(opts));
>>> +else
>>> +advise(_("after resolving the
>>> conflicts, mark the corrected paths\n"
>>> +"with 'git add ' or 'git
>>> rm '\n"
>>> +"and commit the result with
>>> 'git commit'"));
>> In both cases (multiple_commits or not), the beginning of the advise
>> is nearly the same, with only a '\n' in the middle being the
>> difference:
>>
>> multiple_commits:
>> "after resolving the conflicts, mark the corrected paths with 'git
>> add ' or 'git rm '\n"
>>
>> !multiple_commits:
>> "after resolving the conflicts, mark the corrected paths\n with 'git
>> add ' or 'git rm '\n"
>>~~~^
>>
>> In 'multiple_commits' case the advise is more than 80 characters long,
>> did you forget the '\n' in that case?
> A previous comment had indicated that having 4 lines was too many. And I
> tend to agree. So I tried to squash it into 3. Back in xterm days, 80
> characters was sacrosanct, but is it really a big deal to exceed it now?
Either way (3 or 4 lines) I find it strange to have both advices
start in the same way except that one is split and not the other.
I cannot tell if it's a big deal or not to exceed 80 characters
but FWIW most of my stuff (terminal and emacs) is 80 columns
long, and I haven't known the "xterm days".
Thanks,
Rémi
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git for Windows 2.9.3
Johannes Schindelin writes: >> And then your "git cat-file" patch can be upstreamed with the option >> renamed to (or with an additional synonym) "--filters", which would make >> things consistent. > > Right. I would like to ask for a `--smudge` synonym nevertheless, just > because I already use this. On the other hand, it is early enough to tell > everybody who knows about this feature to change their invocation (anybody > who would know about `--smudge` would be in that 1% of users that have > read the release notes, so most likely would read the next release notes, > too). It is OK if it were your private edition, but you end up hurting your users if you need to redo the feature differently. That's the price of your using open source and taking a short-cut. Adding a "--smudge" synonym is spreading the same hurt to outside your fork. Let's see if we can avoid doing that. Perhaps mark that "--smudge" as experimental-and-subject-to-change and re-announce? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] diff-highlight: add support for --graph output.
Signed-off-by: Brian Henderson
---
contrib/diff-highlight/diff-highlight | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/contrib/diff-highlight/diff-highlight
b/contrib/diff-highlight/diff-highlight
index ffefc31..9364423 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -21,6 +21,10 @@ my $RESET = "\x1b[m";
my $COLOR = qr/\x1b\[[0-9;]*m/;
my $BORING = qr/$COLOR|\s/;
+# The patch portion of git log -p --graph should only ever have preceding | and
+# not / or \ as merge history only shows up on the commit line.
+my $GRAPH = qr/$COLOR?\|$COLOR?\s+/;
+
my @removed;
my @added;
my $in_hunk;
@@ -32,12 +36,12 @@ $SIG{PIPE} = 'DEFAULT';
while (<>) {
if (!$in_hunk) {
print;
- $in_hunk = /^$COLOR*\@/;
+ $in_hunk = /^$GRAPH*$COLOR*\@/;
}
- elsif (/^$COLOR*-/) {
+ elsif (/^$GRAPH*$COLOR*-/) {
push @removed, $_;
}
- elsif (/^$COLOR*\+/) {
+ elsif (/^$GRAPH*$COLOR*\+/) {
push @added, $_;
}
else {
@@ -46,7 +50,7 @@ while (<>) {
@added = ();
print;
- $in_hunk = /^$COLOR*[\@ ]/;
+ $in_hunk = /^$GRAPH*$COLOR*[\@ ]/;
}
# Most of the time there is enough output to keep things streaming,
@@ -211,8 +215,8 @@ sub is_pair_interesting {
my $suffix_a = join('', @$a[($sa+1)..$#$a]);
my $suffix_b = join('', @$b[($sb+1)..$#$b]);
- return $prefix_a !~ /^$COLOR*-$BORING*$/ ||
- $prefix_b !~ /^$COLOR*\+$BORING*$/ ||
+ return $prefix_a !~ /^$GRAPH*$COLOR*-$BORING*$/ ||
+ $prefix_b !~ /^$GRAPH*$COLOR*\+$BORING*$/ ||
$suffix_a !~ /^$BORING*$/ ||
$suffix_b !~ /^$BORING*$/;
}
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] diff-highlight: add support for git log --graph output.
Changes made per Eric.
On Wed, Aug 10, 2016 at 08:56:35AM +, Eric Wong wrote:
> Brian Henderson wrote:
>
> Hi Brian,
>
> A few minor portability/style nits below, but contrib/ probably
> (still?) has laxer rules than the rest of git...
>
> I think we still require Signed-off-by lines in contrib,
> though...
added
>
> > +++ b/contrib/diff-highlight/t/Makefile
> > @@ -0,0 +1,19 @@
> > +-include ../../../config.mak.autogen
> > +-include ../../../config.mak
> > +
> > +T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
> > +
> > +all: test
> > +test: $(T)
> > +
> > +.PHONY: help clean all test $(T)
> > +
> > +help:
> > + @echo 'Run "$(MAKE) test" to launch test scripts'
> > + @echo 'Run "$(MAKE) clean" to remove trash folders'
> > +
> > +$(T):
> > + @echo "*** $@ ***"; sh $@ $(GIT_TEST_OPTS)
>
> Probably:
>
> @echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
>
> like we do in t/Makefile in case we need to override 'sh'.
>
done, although I basically had to copy the SHELL_PATH_SQ logic from t/Makefile
> > +
> > +clean:
> > + $(RM) -r 'trash directory'.*
> > diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh
> > b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> > new file mode 100644
> > index 000..ca7605f
> > --- /dev/null
> > +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> > @@ -0,0 +1,63 @@
> > +#!/bin/sh
> > +#
> > +# Copyright (C) 2016
>
> IANAL, but I think your name (or who you represent) needs to be
> in the copyright.
diff-highlight doesn't have a copyright, so I just removed it. ok?
>
> > +
> > +test_description='Test diff-highlight'
> > +
> > +. ./test-diff-highlight.sh
> > +. $TEST_DIRECTORY/test-lib.sh
>
> TEST_DIRECTORY ought to be quoted since it could contain
> shell-unfriendly chars (we intentionally name 'trash directory'
> this way to trigger errors).
done
>
> > +
> > +# PERL is required, but assumed to be present, although not necessarily
> > modern
> > +# some tests require 5.8
> > +
> > +test_expect_success 'diff-highlight highlightes the beginning of a line' '
>
> You can declare a prereq for PERL::
>
> test_expect_success PERL 'name' 'true'
done
>
> And spelling: "highlights" (there's other instances of this, too)
oops, thanks
>
> > + dh_test \
> > +"aaa\nbbb\nccc\n" \
> > +"aaa\n0bb\nccc\n" \
> > +"
>
> We use tabs for shell indentation.
done
> > +
> > +dh_diff_test() {
> > + local a="$1" b="$2"
>
> "local" is not a portable construct. It's common for
> Debian/Ubuntu systems to use dash as /bin/sh instead of bash;
> (dash is faster, and mostly sticks to POSIX)
>
> The "devscripts" package in Debian/Ubuntu-based systems has a
> handy "checkbashisms" tool for checking portability of shell
> scripts.
checkbashisms didn't output anything, and I found other instances of local in
some tests. but I removed anyway.
>
> > +
> > + printf "$a" > file
> > + git add file
> > +
> > + printf "$b" > file
> > +
> > + git diff file > diff.raw
>
> Commands should be '&&'-chained here since any of them could fail
> ("git add"/printf/etc could run out of space or fail on disk errors)
I wasn't totally sure what to do here. I added some checks for empty files and
&& chained them with the last command to verify I wasn't diffing 2 empty files.
Hope that is sufficient.
>
> Also, our redirect style is: command >file
> without a space between '>' and destination.
done
>
> See Documentation/CodingGuidelines for more details.
> Unfortunately, the reasoning is not explained for '>NOSPACE'
> and I'm not sure exactly why, either...
>
> > + if test "$#" = 3
done
>
> Better to use -eq for numeric comparisons: test $# -eq 3
> Quoting $# doesn't seem necessary unless your shell is
> hopelessly buggy :)
>
> > + then
> > +# remove last newline
> > +head -n5 diff.raw | head -c -1 > diff.act
>
> "head -c" isn't portable, fortunately Jeff hoisted it out for
> use as test_copy_bytes in commit 48860819e8026
> https://public-inbox.org/git/20160630090753.ga17...@sigill.intra.peff.net/
It didn't look like his version supported a negative number, so I rolled my own.
>
> > +printf "$3" >> diff.act
> > + else
> > +cat diff.raw > diff.act
> > + fi
> > +
> > + < diff.raw $CMD > diff.exp
>
> $CMD probably needs to be quoted. However, by the time I got
> here I had to ask myself: What is $CMD again?
> A: Oh, look up at the top!
>
> *shrug* My attention span is tiny and my fonts are gigantic.
>
> Perhaps using:
>
> DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
>
> Would be more-readable?
good, done.
>
> > +
> > + diff diff.exp diff.act
>
> Maybe use test_cmp to avoid depending external diff.
> (or "git diff -b --no-index" in your later test)
>
> Same comments for the rest of the series, I think.
>
> Typically, we expect a reroll in a few days, and I guess there's
> no rush (so you can squash your comment patch in addressing
> Junio's concern into 3/3).
>
> Thanks.
thanks for t
Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits
Remi Galan Alfonso writes: > Either way (3 or 4 lines) I find it strange to have both advices > start in the same way except that one is split and not the other. True. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] diff-highlight: add some tests.
Signed-off-by: Brian Henderson
---
contrib/diff-highlight/Makefile | 5 ++
contrib/diff-highlight/t/Makefile| 22
contrib/diff-highlight/t/t9400-diff-highlight.sh | 62 +
contrib/diff-highlight/t/test-diff-highlight.sh | 69
4 files changed, 158 insertions(+)
create mode 100644 contrib/diff-highlight/Makefile
create mode 100644 contrib/diff-highlight/t/Makefile
create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh
create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh
diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
new file mode 100644
index 000..b866259
--- /dev/null
+++ b/contrib/diff-highlight/Makefile
@@ -0,0 +1,5 @@
+# nothing to build
+all:;
+
+test:
+ $(MAKE) -C t
diff --git a/contrib/diff-highlight/t/Makefile
b/contrib/diff-highlight/t/Makefile
new file mode 100644
index 000..5ff5275
--- /dev/null
+++ b/contrib/diff-highlight/t/Makefile
@@ -0,0 +1,22 @@
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+# copied from ../../t/Makefile
+SHELL_PATH ?= $(SHELL)
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+
+all: test
+test: $(T)
+
+.PHONY: help clean all test $(T)
+
+help:
+ @echo 'Run "$(MAKE) test" to launch test scripts'
+ @echo 'Run "$(MAKE) clean" to remove trash folders'
+
+$(T):
+ @echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+
+clean:
+ $(RM) -r 'trash directory'.*
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
new file mode 100755
index 000..8eff178
--- /dev/null
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='Test diff-highlight'
+
+. ./test-diff-highlight.sh
+. "$TEST_DIRECTORY"/test-lib.sh
+
+# PERL is required, but assumed to be present, although not necessarily modern
+# some tests require 5.8
+test_expect_success PERL 'name' 'true'
+
+test_expect_success 'diff-highlight highlights the beginning of a line' '
+ dh_test \
+ "aaa\nbbb\nccc\n" \
+ "aaa\n0bb\nccc\n" \
+"
+ aaa
+-${CW}b${CR}bb
++${CW}0${CR}bb
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight highlights the end of a line' '
+ dh_test \
+ "aaa\nbbb\nccc\n" \
+ "aaa\nbb0\nccc\n" \
+"
+ aaa
+-bb${CW}b${CR}
++bb${CW}0${CR}
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight highlights the middle of a line' '
+ dh_test \
+ "aaa\nbbb\nccc\n" \
+ "aaa\nb0b\nccc\n" \
+"
+ aaa
+-b${CW}b${CR}b
++b${CW}0${CR}b
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight does not highlight whole line' '
+ dh_test \
+ "aaa\nbbb\nccc\n" \
+ "aaa\n000\nccc\n"
+'
+
+test_expect_success 'diff-highlight does not highlight mismatched hunk size' '
+ dh_test \
+ "aaa\nbbb\n" \
+ "aaa\nb0b\nccc\n"
+'
+
+# TODO add multi-byte test
+
+test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh
b/contrib/diff-highlight/t/test-diff-highlight.sh
new file mode 100644
index 000..38323e8
--- /dev/null
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+CURR_DIR=$(pwd)
+TEST_OUTPUT_DIRECTORY=$(pwd)
+TEST_DIRECTORY="$CURR_DIR"/../../../t
+DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
+
+CW="\033[7m"
+CR="\033[27m"
+
+export TEST_OUTPUT_DIRECTORY TEST_DIRECTORY CW CR
+
+dh_test() {
+ dh_diff_test "$@" &&
+ dh_commit_test "$@"
+}
+
+dh_diff_test() {
+ a="$1" b="$2"
+
+ printf "$a" >file
+ git add file
+
+ printf "$b" >file
+ git diff file >diff.raw
+
+ if test $# -eq 3
+ then
+ # remove last newline
+ head -n5 diff.raw | test_chomp_eof >diff.exp
+ printf "$3" >>diff.exp
+ else
+ cat diff.raw >diff.exp
+ fi
+
+ diff.act &&
+ test -s diff.act &&
+ diff diff.exp diff.act
+}
+
+dh_commit_test() {
+ a="$1" b="$2"
+
+ printf "$a" >file
+ git add file
+ git commit -m"Add a file" >/dev/null
+
+ printf "$b" >file
+ git commit -am"Update a file" >/dev/null
+
+ git show >commit.raw
+
+ if test "$#" = 3
+ then
+ # remove last newline
+ head -n11 commit.raw | test_chomp_eof >commit.exp
+ printf "$3" >>commit.exp
+ else
+ cat commit.raw >commit.exp
+ fi
+
+ commit.act &&
+ test -s commit.act &&
+ test_cmp commit.exp commit.act
+}
+
+test_chomp_eof() {
+ perl -pe 'chomp if eof'
+}
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] diff-highlight: add failing test for handling --graph output.
Signed-off-by: Brian Henderson
---
contrib/diff-highlight/t/t9400-diff-highlight.sh | 13 +++
contrib/diff-highlight/t/test-diff-highlight.sh | 43
2 files changed, 56 insertions(+)
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 8eff178..39707c6 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -59,4 +59,17 @@ test_expect_success 'diff-highlight does not highlight
mismatched hunk size' '
# TODO add multi-byte test
+test_expect_success 'diff-highlight highlights the beginning of a line' '
+ dh_graph_test \
+ "aaa\nbbb\nccc\n" \
+ "aaa\n0bb\nccc\n" \
+ "aaa\nb0b\nccc\n" \
+"
+ aaa
+-${CW}b${CR}bb
++${CW}0${CR}bb
+ ccc
+"
+'
+
test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh
b/contrib/diff-highlight/t/test-diff-highlight.sh
index 38323e8..67f742c 100644
--- a/contrib/diff-highlight/t/test-diff-highlight.sh
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -64,6 +64,49 @@ dh_commit_test() {
test_cmp commit.exp commit.act
}
+dh_graph_test() {
+ a="$1" b="$2" c="$3"
+
+ {
+ printf "$a" >file
+ git add file
+ git commit -m"Add a file"
+
+ printf "$b" >file
+ git commit -am"Update a file"
+
+ git checkout -b branch
+ printf "$c" >file
+ git commit -am"Update a file on branch"
+
+ git checkout master
+ printf "$a" >file
+ git commit -am"Update a file again"
+
+ git checkout branch
+ printf "$b" >file
+ git commit -am"Update a file similar to master"
+
+ git merge master
+ git checkout master
+ git merge branch --no-ff
+ } >/dev/null 2>&1
+
+ git log -p --graph --no-merges >graph.raw
+
+ # git log --graph orders the commits different than git log so we hack
it by
+ # using sed to remove the graph part. We know from other tests, that
DIFF_HIGHLIGHT
+ # works without the graph, so there should be no diff when running it
with
+ # and without.
+ graph.exp
+ graph.act &&
+
+ test -s graph.act &&
+ # ignore whitespace since we're using a hacky sed command to remove the
graph
+ # parts.
+ git diff -b --no-index graph.exp graph.act
+}
+
test_chomp_eof() {
perl -pe 'chomp if eof'
}
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.10.0-rc0
Torsten Bögershausen writes: > Is this `text=auto` correct ? Thanks for spotting a typo. Definitely. > I think it should be > >used to have the same effect as >$ echo "* text eol=crlf" >.gitattributes Thanks. > # In other words, the `auto` was ignored, as explained here: > + $ git config core.eol crlf > + i.e. declaring all files are text; the combination now is > + equivalent to doing > + $ git config core.autocrlf true > + -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path
Remi Galan Alfonso writes: > I tried to see if the `git config` in other tests were in the > same case or not but the sheer amount made me reconsider. However > taking a look at a couple of random ones, there are some scripts > that would benefit from the conversion. Yes, that is why I said "it is a good idea to do this where necessary, but the test Dscho touched here does not need it". It is a given that there are tons of "git config" users as we have a lot more tests that were written before test_config was invented. It is good to occasionally modernise ancient tests; we usually do so when we need to make some other changes to them (e.g. I added a feature, and wanted to add a couple of new tests, but I found it unreadable and unmaintainable because it kept the ancient style, so I am updating the existing tests to more modern style first before doing my changes--and then do my changes on top of the cleaned up codebase). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.10.0-rc0
Junio C Hamano writes: > Torsten Bögershausen writes: > >> Is this `text=auto` correct ? > > Thanks for spotting a typo. Definitely. > >> I think it should be >> >>used to have the same effect as >>$ echo "* text eol=crlf" >.gitattributes > > Thanks. > >> # In other words, the `auto` was ignored, as explained here: >> + $ git config core.eol crlf >> + i.e. declaring all files are text; the combination now is >> + equivalent to doing >> + $ git config core.autocrlf true >> + Just to make sure I got you right, how does this look? Documentation/RelNotes/2.10.0.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/RelNotes/2.10.0.txt b/Documentation/RelNotes/2.10.0.txt index 179e575..ccf5f64 100644 --- a/Documentation/RelNotes/2.10.0.txt +++ b/Documentation/RelNotes/2.10.0.txt @@ -235,13 +235,13 @@ Performance, Internal Implementation, Development Support etc. * The API to iterate over all the refs (i.e. for_each_ref(), etc.) has been revamped. - * The handling of the "text = auto" attribute has been updated. + * The handling of the "text=auto" attribute has been corrected. $ echo "* text=auto eol=crlf" >.gitattributes used to have the same effect as - $ echo "* text=auto eol=crlf" >.gitattributes + $ echo "* text eol=crlf" >.gitattributes $ git config core.eol crlf - i.e. declaring all files are text; the combination now is - equivalent to doing + i.e. declaring all files are text (ignoring "auto"). The + combination has been fixed to be equivalent to doing $ git config core.autocrlf true * A few tests that specifically target "git rebase -i" have been -- 2.10.0-rc0-144-gf47a5f1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2016, #06; Sun, 14)
Duy Nguyen writes: > On Mon, Aug 15, 2016 at 5:46 AM, Junio C Hamano wrote: >> * dt/index-helper (2016-07-06) 21 commits >> >> A new "index-helper" daemon has been introduced to give newly >> spawned Git process a quicker access to the data in the index, and >> optionally interface with the watchman daemon to further reduce the >> refresh cost. >> >> Not quite ready yet, it seems. >> cf. >> cf. > > David I can take back this series if you are busy or no longer interested in > it. > > If so, Junio, since I may try some slightly different direction first, > it may take a while before I resubmit, feel free to drop it if it adds > work to you. OK; thanks, both. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] document how to reference previous commits
Heiko Voigt writes: > On Thu, Jul 28, 2016 at 02:55:14PM +0200, Heiko Voigt wrote: >> To reference previous commits people used to put just the abbreviated >> SHA-1 into commit messages. This is what has evolved as a more >> stable format for referencing commits. So lets document it for everyone >> to lookup when needed. > > A quick ping about this patch. Maybe you missed to include it Junio? I > can not find any reference to it in the cooking mails and in your > repository. That's because I was undecided. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] t6026-merge-attr: child processes must not inherit index.lock handles
Johannes Schindelin writes: > From: Ben Wijen > > On Windows, files cannot be removed unless all file handles to it have s/files/a file/, as the file handles later in the sentence are open on that single file. Alternatively s/file handles to it/file handles to them/. > been released. Hence it is particularly important to close handles when > spawning children (which would probably not even know that they hold on > to those handles). > > The example chosen for this test is a custom merge driver that indeed > has no idea that it blocks the deletion of index.lock. The full use case > is a daemon that lives on after the merge, with subsequent invocations > handing off to the daemon, thereby avoiding hefty start-up costs. We > simulate this behavior by simply sleeping one second. > > Note that the test only fails on Windows, due to the file locking issue. > Since we have no way to say "expect failure with MINGW, success > otherwise", we simply skip this test on Windows for now. > > Signed-off-by: Ben Wijen > Signed-off-by: Johannes Schindelin > --- Otherwise, nicely done. Thanks. > t/t6026-merge-attr.sh | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh > index ef0cbce..3d28c78 100755 > --- a/t/t6026-merge-attr.sh > +++ b/t/t6026-merge-attr.sh > @@ -181,4 +181,17 @@ test_expect_success 'up-to-date merge without common > ancestor' ' > ) > ' > > +test_expect_success !MINGW 'custom merge does not lock index' ' > + git reset --hard anchor && > + write_script sleep-one-second.sh <<-\EOF && > + sleep 1 & > + EOF > + > + test_write_lines >.gitattributes \ > + "* merge=ours" "text merge=sleep-one-second" && > + test_config merge.ours.driver true && > + test_config merge.sleep-one-second.driver ./sleep-one-second.sh && > + git merge master > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes
Johannes Schindelin writes: > From: Ben Wijen > > When the index is locked and child processes inherit the handle to > said lock and the parent process wants to remove the lock before the > child process exits, on Windows there is a problem: it won't work > because files cannot be deleted if a process holds a handle on them. > The symptom: > > Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed. > Should I try again? (y/n) > > Spawning child processes with bInheritHandles==FALSE would not work > because no file handles would be inherited, not even the hStdXxx > handles in STARTUPINFO (stdin/stdout/stderr). > > Opening every file with O_NOINHERIT does not work, either, as e.g. > git-upload-pack expects inherited file handles. > > This leaves us with the only way out: creating temp files with the > O_NOINHERIT flag. This flag is Windows-specific, however. For our > purposes, it is equivalent our purposes) to O_CLOEXEC (which does not > exist on Windows), so let's just open temporary files with the > O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows. The callchain that leads to create_tempfile() eventually goes up to hold_lock_file_for_update() and its _timeout() variant in the current codebase. There is no other create_tempfile() caller. I think all the callers of these public entry points use them to open a lockfile for its own use, and never delegate the writing to it to their children, so this change is safe right now, and I do not think it is too bad that a new caller that wants to do that have to explicitly drop O_CLOEXEC (perhaps by dup(2)ing). But it deserves mention in the lockfile and the tempfile API docs in and that the file descriptor returned from these public entry points does not survive across fork(2). Something along the line shown by the attached patch, perhaps. Other than that, this is very nicely analyzed and discusses possible alternative approaches sufficiently to easily convince readers that this is the best solution. Very nicely done. Thanks. lockfile.h | 4 tempfile.h | 4 2 files changed, 8 insertions(+) diff --git a/lockfile.h b/lockfile.h index 3d30193..e976c7a 100644 --- a/lockfile.h +++ b/lockfile.h @@ -55,6 +55,10 @@ * * calling `fdopen_lock_file()` to get a `FILE` pointer for the * open file and writing to the file using stdio. * + * Note that the file descriptor returned by hold_lock_file_for_update() + * is marked O_CLOEXEC, so the new contents must be written by + * you, and not by your subprocess. + * * When finished writing, the caller can: * * * Close the file descriptor and rename the lockfile to its final diff --git a/tempfile.h b/tempfile.h index 4219fe4..d357177 100644 --- a/tempfile.h +++ b/tempfile.h @@ -33,6 +33,10 @@ * * calling `fdopen_tempfile()` to get a `FILE` pointer for the * open file and writing to the file using stdio. * + * Note that the file descriptor returned by create_tempfile() + * is marked O_CLOEXEC, so the new contents must be written by + * you, and not by your subprocess. + * * When finished writing, the caller can: * * * Close the file descriptor and remove the temporary file by -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --diff-filter on modified but locally deleted files
My use case is that I do a merge from branch A to branch B. Branch A modified a file which is already deleted on B some time before the merge. When I do a `git status -sb`, these locally deleted but remotely modified files show up as "DU". I want to invoke git status or diff (or something else) to get a list of these specific conflicts (locally deleted, remotely modified). I tried this: $ git diff --diff-filter=D --name-status This gave me no results. I also tried adding --cached, didn't make a difference. If I just do this: $ git diff --name-status This lists those files as just U instead of DU. So I'm not sure where the "D" is coming from in this case. Is there a way I can get a list of these specific conflicts? I'd like to rely on working/index status for this if possible, since as I resolve these conflicts I want the list to shorten and only show unresolved conflicts of this type. Which rules out a diff range between B...A that I could do. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.10.0-rc0
On 17.08.16 19:38, Junio C Hamano wrote: > Junio C Hamano writes: > >> Torsten Bögershausen writes: >> >>> Is this `text=auto` correct ? >> >> Thanks for spotting a typo. Definitely. >> >>> I think it should be >>> >>>used to have the same effect as >>>$ echo "* text eol=crlf" >.gitattributes >> >> Thanks. >> >>> # In other words, the `auto` was ignored, as explained here: >>> + $ git config core.eol crlf >>> + i.e. declaring all files are text; the combination now is >>> + equivalent to doing >>> + $ git config core.autocrlf true >>> + > > Just to make sure I got you right, how does this look? > > Documentation/RelNotes/2.10.0.txt | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Documentation/RelNotes/2.10.0.txt > b/Documentation/RelNotes/2.10.0.txt > index 179e575..ccf5f64 100644 > --- a/Documentation/RelNotes/2.10.0.txt > +++ b/Documentation/RelNotes/2.10.0.txt > @@ -235,13 +235,13 @@ Performance, Internal Implementation, Development > Support etc. > * The API to iterate over all the refs (i.e. for_each_ref(), etc.) > has been revamped. > > - * The handling of the "text = auto" attribute has been updated. > + * The handling of the "text=auto" attribute has been corrected. > $ echo "* text=auto eol=crlf" >.gitattributes > used to have the same effect as > - $ echo "* text=auto eol=crlf" >.gitattributes > + $ echo "* text eol=crlf" >.gitattributes # Now we describe/define the eol at checkouts twice. # I think we can drop this line: > $ git config core.eol crlf # The rest looks good: > - i.e. declaring all files are text; the combination now is > - equivalent to doing > + i.e. declaring all files are text (ignoring "auto"). The > + combination has been fixed to be equivalent to doing > $ git config core.autocrlf true > > * A few tests that specifically target "git rebase -i" have been > # Just to be sure: the whole paragraph may look like this: * The handling of the "text=auto" attribute has been corrected. $ echo "* text=auto eol=crlf" >.gitattributes used to have the same effect as $ echo "* text eol=crlf" >.gitattributes i.e. declaring all files are text (ignoring "auto"). The combination has been fixed to be equivalent to doing $ git config core.autocrlf true -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: diff --diff-filter on modified but locally deleted files
Robert Dailey writes: > My use case is that I do a merge from branch A to branch B. Branch A > modified a file which is already deleted on B some time before the > merge. > > When I do a `git status -sb`, these locally deleted but remotely > modified files show up as "DU". > > I want to invoke git status or diff (or something else) to get a list > of these specific conflicts (locally deleted, remotely modified). As far as "git diff [--cached]" (which is "compare HEAD with the working tree through index") is concerned, these paths are in the "U"nmerged category, so you'd give "U" to diff-filter to view them. Of course, that would give you other kinds of unmerged entries. If you know they show up as DU, why not "grep DU" in that output? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-multimail: update to release 1.4.0
Thanks. Will directly take it to 'master'. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: diff --diff-filter on modified but locally deleted files
On Wed, Aug 17, 2016 at 1:34 PM, Junio C Hamano wrote: > Robert Dailey writes: > >> My use case is that I do a merge from branch A to branch B. Branch A >> modified a file which is already deleted on B some time before the >> merge. >> >> When I do a `git status -sb`, these locally deleted but remotely >> modified files show up as "DU". >> >> I want to invoke git status or diff (or something else) to get a list >> of these specific conflicts (locally deleted, remotely modified). > > As far as "git diff [--cached]" (which is "compare HEAD with the > working tree through index") is concerned, these paths are in the > "U"nmerged category, so you'd give "U" to diff-filter to view them. > Of course, that would give you other kinds of unmerged entries. > > If you know they show up as DU, why not "grep DU" in that output? I'm happy to do that and in fact considered it, but since --diff-filter exists, I figured it might solve the problem. I also need a better understanding of how diff-filter works since it technically has 2 sides during a merge conflict. I guess it functions only on the right (local) side. Thanks Junio. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] diff-highlight: add some tests.
Brian Henderson writes:
> Signed-off-by: Brian Henderson
> ---
> contrib/diff-highlight/Makefile | 5 ++
> contrib/diff-highlight/t/Makefile| 22
> contrib/diff-highlight/t/t9400-diff-highlight.sh | 62 +
> contrib/diff-highlight/t/test-diff-highlight.sh | 69
>
> 4 files changed, 158 insertions(+)
> create mode 100644 contrib/diff-highlight/Makefile
> create mode 100644 contrib/diff-highlight/t/Makefile
> create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh
> create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh
I am not sure test-diff-highlight.sh should be there; the function
definitions would still be useful but move them to the beginning of
t9400-diff-highlight.sh perhaps?
> diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
> new file mode 100644
> index 000..b866259
> --- /dev/null
> +++ b/contrib/diff-highlight/Makefile
> @@ -0,0 +1,5 @@
> +# nothing to build
> +all:;
Drop ';'.
> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh
> b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> new file mode 100755
> index 000..8eff178
> --- /dev/null
> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> @@ -0,0 +1,62 @@
> +#!/bin/sh
> +
> +test_description='Test diff-highlight'
> +
> +. ./test-diff-highlight.sh
> +. "$TEST_DIRECTORY"/test-lib.sh
> +
> +# PERL is required, but assumed to be present, although not necessarily
> modern
> +# some tests require 5.8
> +test_expect_success PERL 'name' 'true'
If the platform lacks PERL prerequisite, this will simply be
skipped, and if the platform has it, it will always succeed.
I am not sure what you are trying to achieve by having this line
here.
> +test_expect_success 'diff-highlight does not highlight whole line' '
> + dh_test \
> + "aaa\nbbb\nccc\n" \
> + "aaa\n000\nccc\n"
> +'
Hmm, does this express the desired outcome, or just document the
current (possibly broken--I dunno) behaviour? The same question for
the next one.
> +test_expect_success 'diff-highlight does not highlight mismatched hunk size'
> '
> + dh_test \
> + "aaa\nbbb\n" \
> + "aaa\nb0b\nccc\n"
> +'
> +dh_test() {
Style: "dh_test () {"
The other functions in this file share the same.
> + dh_diff_test "$@" &&
> + dh_commit_test "$@"
> +}
> +
> +dh_diff_test() {
> + a="$1" b="$2"
> +
> + printf "$a" >file
> + git add file
> +
> + printf "$b" >file
> + git diff file >diff.raw
> +
> + if test $# -eq 3
> + then
> + # remove last newline
> + head -n5 diff.raw | test_chomp_eof >diff.exp
A reader can see "remove last newline" by seeing test_chomp_eof and
what it does without a comment, but it is totally unclear why you
need to remove. The comment that says what it does without saying
why it does it is useless.
> + printf "$3" >>diff.exp
> + else
> + cat diff.raw >diff.exp
> + fi
> +
> + diff.act &&
Even though this is technically kosher, I do not see a merit of
deviating from the common practice of starting a line with the
command, i.e.
"$DIFF_HIGHLIGHT" diff.actual
would be much easier to read.
> + test -s diff.act &&
Why? If you always have the expected output that you are going to
compare with, wouldn't that sufficient to do that test without this?
Besides, having "test -s" means that you can never make sure that a
certain pair of input does not show any changes. Perhaps drop it?
> + diff diff.exp diff.act
Use test_cmp unless there is a strong reason why you shouldn't?
> +}
> +
> +dh_commit_test() {
> + a="$1" b="$2"
> +
> + printf "$a" >file
> + git add file
> + git commit -m"Add a file" >/dev/null
Avoid sticking a short-option to its argument, i.e.
git commit -m "Add a file"
> +
> + printf "$b" >file
> + git commit -am"Update a file" >/dev/null
Likewise.
git commit -a -m "Update a file"
The remainder of the file invites the same set of questions and
comments you see for dh_diff_test() above, so I won't repeat them.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] diff-highlight: add failing test for handling --graph output.
Brian Henderson writes:
> Signed-off-by: Brian Henderson
> ---
> contrib/diff-highlight/t/t9400-diff-highlight.sh | 13 +++
> contrib/diff-highlight/t/test-diff-highlight.sh | 43
>
> 2 files changed, 56 insertions(+)
>
> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh
> b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> index 8eff178..39707c6 100755
> --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> @@ -59,4 +59,17 @@ test_expect_success 'diff-highlight does not highlight
> mismatched hunk size' '
>
> # TODO add multi-byte test
>
> +test_expect_success 'diff-highlight highlights the beginning of a line' '
> + dh_graph_test \
> + "aaa\nbbb\nccc\n" \
> + "aaa\n0bb\nccc\n" \
> + "aaa\nb0b\nccc\n" \
> +"
> + aaa
> +-${CW}b${CR}bb
> ++${CW}0${CR}bb
> + ccc
> +"
> +'
Is this expected to pass after applying 1/3 and 2/3? The title says
"add faililng test", so I am assuming this is expected to fail, in
which case the test should start out as "test_expect_failure". A
later patch that makes it pass should turn "test_expect_failure"
into "test_expect_success".
> test_done
> diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh
> b/contrib/diff-highlight/t/test-diff-highlight.sh
> index 38323e8..67f742c 100644
> --- a/contrib/diff-highlight/t/test-diff-highlight.sh
> +++ b/contrib/diff-highlight/t/test-diff-highlight.sh
> @@ -64,6 +64,49 @@ dh_commit_test() {
> test_cmp commit.exp commit.act
> }
>
> +dh_graph_test() {
> + a="$1" b="$2" c="$3"
> +
> + {
> + printf "$a" >file
> + ...
> + git merge master
> + git checkout master
> + git merge branch --no-ff
> + } >/dev/null 2>&1
> +
> + git log -p --graph --no-merges >graph.raw
Hmph, when does it make sense to have "--no-merges" together with
"--graph". Doesn't it result in a disconnected mess? Is that an
interesting and most often used case?
> +
> + # git log --graph orders the commits different than git log so we hack
> it by
> + # using sed to remove the graph part.
Would it help if you knew "log --topo-order" to remove the "hack"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] imap-send: Tell cURL to use imap:// or imaps://
Right now the imap:// or imaps:// part of imap.host is not being passed on to cURL. Perhaps it was able to guess correctly under some circumstances, but I was not able to find one; it was just trying to make HTTP requests for me. It’s better to be explicit in any case. Signed-off-by: Anders Kaseorg --- imap-send.c | 1 + 1 file changed, 1 insertion(+) diff --git a/imap-send.c b/imap-send.c index 938c691..7dd5acf 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1410,6 +1410,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc) curl_easy_setopt(curl, CURLOPT_USERNAME, server.user); curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass); + strbuf_addstr(&path, server.use_ssl ? "imaps://" : "imap://"); strbuf_addstr(&path, server.host); if (!path.len || path.buf[path.len - 1] != '/') strbuf_addch(&path, '/'); -- 2.10.0.rc0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates
Stefan Beller writes:
> +static void prepare_possible_alternates(const char *sm_name,
> + struct string_list *reference)
> +{
> + char *sm_alternate = NULL, *error_strategy = NULL;
> + struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
> +
> + git_config_get_string("submodule.alternateLocation", &sm_alternate);
> + if (!sm_alternate)
> + return;
> +
> + git_config_get_string("submodule.alternateErrorStrategy",
> &error_strategy);
I have to admit that I need to follow the codepath in config.c every
time to check, but I _think_ git_config_get_string() gives you your
own copy of the value. As this function does not give ownership of
sm_alternate or error_strategy to something else, they are leaked
every time this function is called, I think.
> + sas.submodule_name = sm_name;
> + sas.reference = reference;
> + if (!strcmp(error_strategy, "die"))
> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
> + if (!strcmp(error_strategy, "info"))
> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;
> + if (!strcmp(sm_alternate, "superproject"))
> + foreach_alt_odb(add_possible_reference_from_superproject, &sas);
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] push: change submodule default to check
When working with submodules, it is easy to forget to push the submodules. Change the default to 'check' if any existing submodule is present on at least one remote of the submodule remotes. This doesn't affect you if you do not work with submodules. If working with submodules, there are more reports of missing submodules rather than the desire to push the superproject without the submodules to be pushed out. Flipping the default to check for submodules is safer than the current default of ignoring submodules while pushing. Signed-off-by: Stefan Beller --- Probably too late for the 2.10 release as I'd propose to keep it in master for quite a while before actually doing a release with this. Thanks, Stefan builtin/push.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/push.c b/builtin/push.c index 3bb9d6b..479150a 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -22,7 +22,7 @@ static int deleterefs; static const char *receivepack; static int verbosity; static int progress = -1; -static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT; +static int recurse_submodules = RECURSE_SUBMODULES_CHECK; static enum transport_family family; static struct push_cas_option cas; -- 2.9.2.730.g525ad04.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates
On Wed, Aug 17, 2016 at 1:02 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> +static void prepare_possible_alternates(const char *sm_name,
>> + struct string_list *reference)
>> +{
>> + char *sm_alternate = NULL, *error_strategy = NULL;
>> + struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
>> +
>> + git_config_get_string("submodule.alternateLocation", &sm_alternate);
>> + if (!sm_alternate)
>> + return;
>> +
>> + git_config_get_string("submodule.alternateErrorStrategy",
>> &error_strategy);
>
> I have to admit that I need to follow the codepath in config.c every
> time to check, but I _think_ git_config_get_string() gives you your
> own copy of the value. As this function does not give ownership of
> sm_alternate or error_strategy to something else, they are leaked
> every time this function is called, I think.
There are quite a few more occurrences of git_config_get_string
in the submodule helper, I'll also look at those.
Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] push: change submodule default to check
Stefan Beller writes: > If working with submodules, there are more reports of missing submodules > rather than the desire to push the superproject without the submodules > to be pushed out. I do not know how you are counting the "more reports" part of that assertion, but it is very likely that it is biased by the current default. If you flip the default, you would see more reports that say "I know I wasn't ready to push the submodule part out; don't bug me". IOW, those who want to have something different always sound louder, because people who are satisified tend to stay silent. > Flipping the default to check for submodules is safer > than the current default of ignoring submodules while pushing. That part of the assertion, on the other hand, is justifiable. > Signed-off-by: Stefan Beller > --- > > Probably too late for the 2.10 release as I'd propose to keep it in master > for > quite a while before actually doing a release with this. I think you meant 'next' not 'master'. We have a few "let's keep it in 'next' to see if people scream" topics there already--the more the merrier? ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] push: change submodule default to check
On Wed, Aug 17, 2016 at 2:05 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> If working with submodules, there are more reports of missing submodules >> rather than the desire to push the superproject without the submodules >> to be pushed out. > > I do not know how you are counting the "more reports" part of that > assertion, but it is very likely that it is biased by the current > default. If you flip the default, you would see more reports that > say "I know I wasn't ready to push the submodule part out; don't bug > me". > > IOW, those who want to have something different always sound louder, > because people who are satisified tend to stay silent. Yeah I thought about this mistake and how to get real numbers, but I was just misled by some people on #git IRC waving hands. ;) > >> Flipping the default to check for submodules is safer >> than the current default of ignoring submodules while pushing. > > That part of the assertion, on the other hand, is justifiable. ok. > >> Signed-off-by: Stefan Beller >> --- >> >> Probably too late for the 2.10 release as I'd propose to keep it in master >> for >> quite a while before actually doing a release with this. > > I think you meant 'next' not 'master'. We have a few "let's keep it > in 'next' to see if people scream" topics there already--the more > the merrier? ;-) Well we put it into next, but that are not as many people as those running master I would think, so I would want to maximize both times in next as well as in master, e.g. if you put it into next today (unreasonable, but let's assume), then it could make it into master next week, and then be released as part of 2.10 IIUC the release schedule. I would say that is too fast. rather I'd see this patch transition from next to master just after a release, such that it lives in master for a full release cycle before being actually in a release. So far for my line of thinking. Thanks, Stefan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-mergetool reverse file ordering
Hi Luis and Hannes, On Wed, Aug 17, 2016 at 09:35:56AM +0200, Johannes Sixt wrote: > Am 17.08.2016 um 08:46 schrieb David Aguilar: > > The only thing that using diff-files doesn't address is the > > rerere support in mergetool where it processes the files in > > the order specified by "git rerere remaining". This is why I > > initially thought we needed a generic sort-like command. > > I see. This is actually an important code path. How about this code > structure: > > if test $# -eq 0 > then > cd_to_toplevel > > if test -e "$GIT_DIR/MERGE_RR" > then > set -- $(git rerere remaining) > fi > fi > files=$(git diff-files --name-only --diff-filter=U -- "$@") > Beautiful. > This does not require an enhancement of rerere-remaining and still captures > all three cases that currently go through separate branches. (Throw in some > version of --ignore-submodules= if necessary, but I guess it is not.) > > We do have a problem if there are file names with spaces, but it is not a > new problem. Thanks for the heads-up about file names with spaces. We set, IFS=' ' in git-mergetool--lib.sh so file names with spaces should be ok. Naturally, we won't be able to support paths with embedded newlines, but that's not a new problem ;-) We should probably also set core.quotePath=false when calling diff-files so that git doesn't try to quote "unusual" paths, e.g. git -c core.quotePath=false diff-files ... Lastly, for anyone that's curious, I was wondering why we were passing "-u" to "sort", and why we won't need to use "uniq" in the new version. The reason is that "ls-files -u" lists the different index stages separately, and thus it reports duplicate paths. "diff-files" with "--diff-filter=U" does not do that, so that's another benefit to be gained from this change. I think we've touched all the bases now. Luis, I hope that makes sense. Let us know if any of this is unclear. ciao, -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv6 8/8] clone: recursive and reference option triggers submodule alternates
When `--recursive` and `--reference` is given, it is reasonable to
expect that the submodules are created with references to the submodules
of the given alternate for the superproject.
An initial attempt to do this was presented to the mailing list, which
used flags that are passed around ("--super-reference") that instructed
the submodule clone to look for a reference in the submodules of the
referenced superproject. This is not well thought out, as any further
`submodule update` should also respect the initial setup.
When a new submodule is added to the superproject and the alternate
of the superproject does not know about that submodule yet, we rather
error out informing the user instead of being unclear if we did or did
not use a submodules alternate.
To solve this problem introduce new options that store the configuration
for what the user wanted originally.
Signed-off-by: Stefan Beller
---
This replaces the tip of remotes/origin/sb/submodule-clone-rr.
The difference to the queued version are 2 free calls at the end of
prepare_possible_alternates()
Thanks,
Stefan
Documentation/config.txt | 12 ++
builtin/clone.c| 19 +
builtin/submodule--helper.c| 90 ++
t/t7408-submodule-reference.sh | 43
4 files changed, 164 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc1c433..602f43a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2837,6 +2837,18 @@ submodule.fetchJobs::
in parallel. A value of 0 will give some reasonable default.
If unset, it defaults to 1.
+submodule.alternateLocation::
+ Specifies how the submodules obtain alternates when submodules are
+ cloned. Possible values are `no`, `superproject`.
+ By default `no` is assumed, which doesn't add references. When the
+ value is set to `superproject` the submodule to be cloned computes
+ its alternates location relative to the superprojects alternate.
+
+submodule.alternateErrorStrategy
+ Specifies how to treat errors with the alternates for a submodule
+ as computed via `submodule.alternateLocation`. Possible values are
+ `ignore`, `info`, `die`.
+
tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG
signed.
If `--annotate` is specified on the command line, it takes
diff --git a/builtin/clone.c b/builtin/clone.c
index 0182665..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,6 +947,25 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
else
fprintf(stderr, _("Cloning into '%s'...\n"), dir);
}
+
+ if (option_recursive) {
+ if (option_required_reference.nr &&
+ option_optional_reference.nr)
+ die(_("clone --recursive is not compatible with "
+ "both --reference and --reference-if-able"));
+ else if (option_required_reference.nr) {
+ string_list_append(&option_config,
+ "submodule.alternateLocation=superproject");
+ string_list_append(&option_config,
+ "submodule.alternateErrorStrategy=die");
+ } else if (option_optional_reference.nr) {
+ string_list_append(&option_config,
+ "submodule.alternateLocation=superproject");
+ string_list_append(&option_config,
+ "submodule.alternateErrorStrategy=info");
+ }
+ }
+
init_db(option_template, INIT_DB_QUIET);
write_config(&option_config);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7096848..fd571bf 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,93 @@ static int clone_submodule(const char *path, const char
*gitdir, const char *url
return run_command(&cp);
}
+struct submodule_alternate_setup {
+ const char *submodule_name;
+ enum SUBMODULE_ALTERNATE_ERROR_MODE {
+ SUBMODULE_ALTERNATE_ERROR_DIE,
+ SUBMODULE_ALTERNATE_ERROR_INFO,
+ SUBMODULE_ALTERNATE_ERROR_IGNORE
+ } error_mode;
+ struct string_list *reference;
+};
+#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
+ SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
+
+static int add_possible_reference_from_superproject(
+ struct alternate_object_database *alt, void *sas_cb)
+{
+ struct submodule_alternate_setup *sas = sas_cb;
+
+ /* directory name, minus trailing slash */
+ size_t namelen = alt->name - alt->base - 1;
+ struct strbuf name = STRBUF_INIT;
+ strbuf_add(&name, alt->base, namelen);
+
+ /*
+* If the al
Re: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates
Junio C Hamano writes:
> Stefan Beller writes:
>
>> +static void prepare_possible_alternates(const char *sm_name,
>> +struct string_list *reference)
>> +{
>> +char *sm_alternate = NULL, *error_strategy = NULL;
>> +struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
>> +
>> +git_config_get_string("submodule.alternateLocation", &sm_alternate);
>> +if (!sm_alternate)
>> +return;
>> +
>> +git_config_get_string("submodule.alternateErrorStrategy",
>> &error_strategy);
>
> I have to admit that I need to follow the codepath in config.c every
> time to check, but I _think_ git_config_get_string() gives you your
> own copy of the value. As this function does not give ownership of
> sm_alternate or error_strategy to something else, they are leaked
> every time this function is called, I think.
>
>> +sas.submodule_name = sm_name;
>> +sas.reference = reference;
>> +if (!strcmp(error_strategy, "die"))
>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
Another thing I noticed but forgot to mention. Can error_strategy
be NULL here? We are assuming sm_alternate can be, so I presume
that it is sensible to protect against dereferencing a NULL here,
too?
>> +if (!strcmp(error_strategy, "info"))
>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;
>> +if (!strcmp(sm_alternate, "superproject"))
>> +foreach_alt_odb(add_possible_reference_from_superproject, &sas);
>> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates
On Wed, Aug 17, 2016 at 2:31 PM, Junio C Hamano wrote:
> Junio C Hamano writes:
>
>> Stefan Beller writes:
>>
>>> +static void prepare_possible_alternates(const char *sm_name,
>>> +struct string_list *reference)
>>> +{
>>> +char *sm_alternate = NULL, *error_strategy = NULL;
>>> +struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
>>> +
>>> +git_config_get_string("submodule.alternateLocation", &sm_alternate);
>>> +if (!sm_alternate)
>>> +return;
>>> +
>>> +git_config_get_string("submodule.alternateErrorStrategy",
>>> &error_strategy);
>>
>> I have to admit that I need to follow the codepath in config.c every
>> time to check, but I _think_ git_config_get_string() gives you your
>> own copy of the value. As this function does not give ownership of
>> sm_alternate or error_strategy to something else, they are leaked
>> every time this function is called, I think.
>>
>>> +sas.submodule_name = sm_name;
>>> +sas.reference = reference;
>>> +if (!strcmp(error_strategy, "die"))
>>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
>
> Another thing I noticed but forgot to mention. Can error_strategy
> be NULL here? We are assuming sm_alternate can be, so I presume
> that it is sensible to protect against dereferencing a NULL here,
> too?
Oh!
Of course we need to fix that too. That slipped in because I assumed that those
two variables go in tandem (if sm_alternate is set, then
error_strategy is set as well),
but that is not the case of course.
Which leads to another thing: we need to have a default for error_strategy.
I think "die" is reasonable here. That also needs a documentation update.
>
>>> +if (!strcmp(error_strategy, "info"))
>>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;
>>> +if (!strcmp(sm_alternate, "superproject"))
>>> +foreach_alt_odb(add_possible_reference_from_superproject,
>>> &sas);
>>> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv7 8/8] clone: recursive and reference option triggers submodule alternates
When `--recursive` and `--reference` is given, it is reasonable to
expect that the submodules are created with references to the submodules
of the given alternate for the superproject.
An initial attempt to do this was presented to the mailing list, which
used flags that are passed around ("--super-reference") that instructed
the submodule clone to look for a reference in the submodules of the
referenced superproject. This is not well thought out, as any further
`submodule update` should also respect the initial setup.
When a new submodule is added to the superproject and the alternate
of the superproject does not know about that submodule yet, we rather
error out informing the user instead of being unclear if we did or did
not use a submodules alternate.
To solve this problem introduce new options that store the configuration
for what the user wanted originally.
Signed-off-by: Stefan Beller
---
Added a default for alternateErrorStrategy and hence fixed the null pointer
for error_strategy in prepare_possible_alternates(),
Thanks,
Stefan
Documentation/config.txt | 12 ++
builtin/clone.c| 19 +
builtin/submodule--helper.c| 93 ++
t/t7408-submodule-reference.sh | 43 +++
4 files changed, 167 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc1c433..e2571ea 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2837,6 +2837,18 @@ submodule.fetchJobs::
in parallel. A value of 0 will give some reasonable default.
If unset, it defaults to 1.
+submodule.alternateLocation::
+ Specifies how the submodules obtain alternates when submodules are
+ cloned. Possible values are `no`, `superproject`.
+ By default `no` is assumed, which doesn't add references. When the
+ value is set to `superproject` the submodule to be cloned computes
+ its alternates location relative to the superprojects alternate.
+
+submodule.alternateErrorStrategy
+ Specifies how to treat errors with the alternates for a submodule
+ as computed via `submodule.alternateLocation`. Possible values are
+ `ignore`, `info`, `die`. Default is `die`.
+
tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG
signed.
If `--annotate` is specified on the command line, it takes
diff --git a/builtin/clone.c b/builtin/clone.c
index 0182665..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,6 +947,25 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
else
fprintf(stderr, _("Cloning into '%s'...\n"), dir);
}
+
+ if (option_recursive) {
+ if (option_required_reference.nr &&
+ option_optional_reference.nr)
+ die(_("clone --recursive is not compatible with "
+ "both --reference and --reference-if-able"));
+ else if (option_required_reference.nr) {
+ string_list_append(&option_config,
+ "submodule.alternateLocation=superproject");
+ string_list_append(&option_config,
+ "submodule.alternateErrorStrategy=die");
+ } else if (option_optional_reference.nr) {
+ string_list_append(&option_config,
+ "submodule.alternateLocation=superproject");
+ string_list_append(&option_config,
+ "submodule.alternateErrorStrategy=info");
+ }
+ }
+
init_db(option_template, INIT_DB_QUIET);
write_config(&option_config);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7096848..f8f35c1 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,96 @@ static int clone_submodule(const char *path, const char
*gitdir, const char *url
return run_command(&cp);
}
+struct submodule_alternate_setup {
+ const char *submodule_name;
+ enum SUBMODULE_ALTERNATE_ERROR_MODE {
+ SUBMODULE_ALTERNATE_ERROR_DIE,
+ SUBMODULE_ALTERNATE_ERROR_INFO,
+ SUBMODULE_ALTERNATE_ERROR_IGNORE
+ } error_mode;
+ struct string_list *reference;
+};
+#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
+ SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
+
+static int add_possible_reference_from_superproject(
+ struct alternate_object_database *alt, void *sas_cb)
+{
+ struct submodule_alternate_setup *sas = sas_cb;
+
+ /* directory name, minus trailing slash */
+ size_t namelen = alt->name - alt->base - 1;
+ struct strbuf name = STRBUF_INIT;
+ strbuf_add(&name, alt->base, namelen);
+
+ /*
+* If the alternate object store i
Working from different machines.
Right now I have a server at work where I keep a bare repository as a source and backup for projects. So I clone a project from there to my desktop, and work, making a few branches as I try out ideas for new features. Then I go home, and I want to work as though I was sitting at my desktop. If I clone the committed work, I don't get all my branches. How can I work so that I now easily have all my branches, then after I work at home, when I go back to my desktop, the branches now reflect whatever state I last left them in? In other words, I want to work from different machines, and always sit down to the environment exactly as I last left it. Thanks! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 8/8] clone: recursive and reference option triggers submodule alternates
Stefan Beller writes:
> Added a default for alternateErrorStrategy and hence fixed the null pointer
> for error_strategy in prepare_possible_alternates(),
Looks much better.
> +submodule.alternateLocation::
> + Specifies how the submodules obtain alternates when submodules are
> + cloned. Possible values are `no`, `superproject`.
> + By default `no` is assumed, which doesn't add references. When the
> + value is set to `superproject` the submodule to be cloned computes
> + its alternates location relative to the superprojects alternate.
I am not seeing a code that handles 'no' and any other string that
is not 'superproject' differently, though.
I can see that "clone" has codepath that writes 'superproject' to
the variable, but the only thing that seems to care what value the
variable is set to checks "no". When somebody sets the variable to
"yes", shouldn't we at least say "Sorry, I do not understand" and
preferrably stop before spreading potential damage? We'd surely end
up doing something that the user who set the value to "yes" did not
expect.
There is something still missing?
> +submodule.alternateErrorStrategy
> + Specifies how to treat errors with the alternates for a submodule
> + as computed via `submodule.alternateLocation`. Possible values are
> + `ignore`, `info`, `die`. Default is `die`.
> +
> tag.forceSignAnnotated::
> A boolean to specify whether annotated tags created should be GPG
> signed.
> If `--annotate` is specified on the command line, it takes
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 0182665..404c5e8 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -947,6 +947,25 @@ int cmd_clone(int argc, const char **argv, const char
> *prefix)
> else
> fprintf(stderr, _("Cloning into '%s'...\n"), dir);
> }
> +
> + if (option_recursive) {
> + if (option_required_reference.nr &&
> + option_optional_reference.nr)
> + die(_("clone --recursive is not compatible with "
> + "both --reference and --reference-if-able"));
> + else if (option_required_reference.nr) {
> + string_list_append(&option_config,
> + "submodule.alternateLocation=superproject");
> + string_list_append(&option_config,
> + "submodule.alternateErrorStrategy=die");
> + } else if (option_optional_reference.nr) {
> + string_list_append(&option_config,
> + "submodule.alternateLocation=superproject");
> + string_list_append(&option_config,
> + "submodule.alternateErrorStrategy=info");
> + }
> + }
> +
> init_db(option_template, INIT_DB_QUIET);
> write_config(&option_config);
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 7096848..f8f35c1 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -472,6 +472,96 @@ static int clone_submodule(const char *path, const char
> *gitdir, const char *url
> return run_command(&cp);
> }
>
> +struct submodule_alternate_setup {
> + const char *submodule_name;
> + enum SUBMODULE_ALTERNATE_ERROR_MODE {
> + SUBMODULE_ALTERNATE_ERROR_DIE,
> + SUBMODULE_ALTERNATE_ERROR_INFO,
> + SUBMODULE_ALTERNATE_ERROR_IGNORE
> + } error_mode;
> + struct string_list *reference;
> +};
> +#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
> + SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
> +
> +static int add_possible_reference_from_superproject(
> + struct alternate_object_database *alt, void *sas_cb)
> +{
> + struct submodule_alternate_setup *sas = sas_cb;
> +
> + /* directory name, minus trailing slash */
> + size_t namelen = alt->name - alt->base - 1;
> + struct strbuf name = STRBUF_INIT;
> + strbuf_add(&name, alt->base, namelen);
> +
> + /*
> + * If the alternate object store is another repository, try the
> + * standard layout with .git/modules//objects
> + */
> + if (ends_with(name.buf, ".git/objects")) {
> + char *sm_alternate;
> + struct strbuf sb = STRBUF_INIT;
> + struct strbuf err = STRBUF_INIT;
> + strbuf_add(&sb, name.buf, name.len - strlen("objects"));
> + /*
> + * We need to end the new path with '/' to mark it as a dir,
> + * otherwise a submodule name containing '/' will be broken
> + * as the last part of a missing submodule reference would
> + * be taken as a file name.
> + */
> + strbuf_addf(&sb, "modules/%s/", sas->submodule_name);
> +
> + sm_alternate = compute_alternate_path(sb.buf, &err);
> + if (sm_alternate) {
> +
Re: [PATCHv7 8/8] clone: recursive and reference option triggers submodule alternates
On Wed, Aug 17, 2016 at 3:12 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> Added a default for alternateErrorStrategy and hence fixed the null pointer
>> for error_strategy in prepare_possible_alternates(),
>
> Looks much better.
>
>> +submodule.alternateLocation::
>> + Specifies how the submodules obtain alternates when submodules are
>> + cloned. Possible values are `no`, `superproject`.
>> + By default `no` is assumed, which doesn't add references. When the
>> + value is set to `superproject` the submodule to be cloned computes
>> + its alternates location relative to the superprojects alternate.
>
> I am not seeing a code that handles 'no' and any other string that
> is not 'superproject' differently, though.
>
> I can see that "clone" has codepath that writes 'superproject' to
> the variable, but the only thing that seems to care what value the
> variable is set to checks "no". When somebody sets the variable to
> "yes", shouldn't we at least say "Sorry, I do not understand" and
> preferrably stop before spreading potential damage? We'd surely end
> up doing something that the user who set the value to "yes" did not
> expect.
>
> There is something still missing?
>
>> +static void prepare_possible_alternates(const char *sm_name,
>> + struct string_list *reference)
>> +{
...
>> + sas.submodule_name = sm_name;
>> + sas.reference = reference;
>> + if (!strcmp(error_strategy, "die"))
>> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
>> + if (!strcmp(error_strategy, "info"))
>> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;
(see below first)
Here goes the same for alternateErrorStrategy
>> + if (!strcmp(sm_alternate, "superproject"))
>> + foreach_alt_odb(add_possible_reference_from_superproject,
>> &sas);
here is where we would add
else if (!strcmp(sm_alternate, "no")
; /* do nothing */
else
die(_("What's wrong with you?"));
Initially I did not add that as I considered it not relevant. But I
guess it helps as a typo checker as well and it is more comforting
if a wrong value yields an error. Also it is consistent with the rest of
options.
Thanks again,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Working from different machines.
Tobiah writes: > In other words, I want to work from different machines, and always > sit down to the environment exactly as I last left it. You have several options, but it depends on untold expectations you have beyond "exactly as I last left it." So the following is only to show directions and possibilities without going into details. For example, do you save all changes you made in your editor buffers before you leave the 'desktop'? If the answer is "no", then that is not a problem Git-the-tool is interested in solving, and the only solution I could think of is to use mechanism to share the session between 'desktop' and 'home', using things like remote-desktop or screen session. If the answer is "yes", the next question is if you commit all the changes you made before you leave the 'desktop'. If the answer is "no", again, that is not a problem Git-the-tool is interested in solving, and the only solution I could think of (in addition to the "share the session" above) is to use networked filesystem shared between 'desktop' and 'home'. If the answer is "yes", then you are in the problem space that Git-the-tool is interested in solving. Assuming that you have network connection into 'desktop' from 'home', the solution would involve making it the first thing to do when get home to run "git fetch" on 'home' to get the latest state from the 'desktop', and run "git push" on 'home' to push out the latest state to the 'desktop' before you leave 'home'. If your 'server' is for your sole use, and if 'home' has network connection into 'server', then you could instead rendezvous at 'server' by running "git push server" on 'desktop' (or 'home') to 'server' as the last thing before you leave 'desktop' (or 'home'), and running "git fetch server" on 'home' (or 'desktop') as the first thing before you start working on 'home' (or 'desktop'). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv8 8/8] clone: recursive and reference option triggers submodule alternates
When `--recursive` and `--reference` is given, it is reasonable to
expect that the submodules are created with references to the submodules
of the given alternate for the superproject.
An initial attempt to do this was presented to the mailing list, which
used flags that are passed around ("--super-reference") that instructed
the submodule clone to look for a reference in the submodules of the
referenced superproject. This is not well thought out, as any further
`submodule update` should also respect the initial setup.
When a new submodule is added to the superproject and the alternate
of the superproject does not know about that submodule yet, we rather
error out informing the user instead of being unclear if we did or did
not use a submodules alternate.
To solve this problem introduce new options that store the configuration
for what the user wanted originally.
Signed-off-by: Stefan Beller
---
and now with error handling of invalid options.
Documentation/config.txt | 12 +
builtin/clone.c| 19
builtin/submodule--helper.c| 102 +
t/t7408-submodule-reference.sh | 43 +
4 files changed, 176 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc1c433..e2571ea 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2837,6 +2837,18 @@ submodule.fetchJobs::
in parallel. A value of 0 will give some reasonable default.
If unset, it defaults to 1.
+submodule.alternateLocation::
+ Specifies how the submodules obtain alternates when submodules are
+ cloned. Possible values are `no`, `superproject`.
+ By default `no` is assumed, which doesn't add references. When the
+ value is set to `superproject` the submodule to be cloned computes
+ its alternates location relative to the superprojects alternate.
+
+submodule.alternateErrorStrategy
+ Specifies how to treat errors with the alternates for a submodule
+ as computed via `submodule.alternateLocation`. Possible values are
+ `ignore`, `info`, `die`. Default is `die`.
+
tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG
signed.
If `--annotate` is specified on the command line, it takes
diff --git a/builtin/clone.c b/builtin/clone.c
index 0182665..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,6 +947,25 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
else
fprintf(stderr, _("Cloning into '%s'...\n"), dir);
}
+
+ if (option_recursive) {
+ if (option_required_reference.nr &&
+ option_optional_reference.nr)
+ die(_("clone --recursive is not compatible with "
+ "both --reference and --reference-if-able"));
+ else if (option_required_reference.nr) {
+ string_list_append(&option_config,
+ "submodule.alternateLocation=superproject");
+ string_list_append(&option_config,
+ "submodule.alternateErrorStrategy=die");
+ } else if (option_optional_reference.nr) {
+ string_list_append(&option_config,
+ "submodule.alternateLocation=superproject");
+ string_list_append(&option_config,
+ "submodule.alternateErrorStrategy=info");
+ }
+ }
+
init_db(option_template, INIT_DB_QUIET);
write_config(&option_config);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7096848..a1da3ea 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,105 @@ static int clone_submodule(const char *path, const char
*gitdir, const char *url
return run_command(&cp);
}
+struct submodule_alternate_setup {
+ const char *submodule_name;
+ enum SUBMODULE_ALTERNATE_ERROR_MODE {
+ SUBMODULE_ALTERNATE_ERROR_DIE,
+ SUBMODULE_ALTERNATE_ERROR_INFO,
+ SUBMODULE_ALTERNATE_ERROR_IGNORE
+ } error_mode;
+ struct string_list *reference;
+};
+#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
+ SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
+
+static int add_possible_reference_from_superproject(
+ struct alternate_object_database *alt, void *sas_cb)
+{
+ struct submodule_alternate_setup *sas = sas_cb;
+
+ /* directory name, minus trailing slash */
+ size_t namelen = alt->name - alt->base - 1;
+ struct strbuf name = STRBUF_INIT;
+ strbuf_add(&name, alt->base, namelen);
+
+ /*
+* If the alternate object store is another repository, try the
+* standard layout with .git/modules//objects
+*/
+
Re: [PATCHv8 8/8] clone: recursive and reference option triggers submodule alternates
Stefan Beller writes: > and now with error handling of invalid options. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/7] diff.c: remove output_prefix_length field
From: Junio C Hamano
"diff/log --stat" has a logic that determines the display columns
available for the diffstat part of the output and apportions it for
pathnames and diffstat graph automatically.
5e71a84a (Add output_prefix_length to diff_options, 2012-04-16)
added the output_prefix_length field to diff_options structure to
allow this logic to subtract the display columns used for the
history graph part from the total "terminal width"; this matters
when the "git log --graph -p" option is in use.
The field must be set to the number of display columns needed to
show the output from the output_prefix() callback, which is error
prone. As there is only one user of the field, and the user has the
actual value of the prefix string, let's get rid of the field and
have the user count the display width itself.
Signed-off-by: Junio C Hamano
---
diff.c | 2 +-
diff.h | 1 -
graph.c | 2 --
3 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index 534c12e28ea8..50bef1f07658 100644
--- a/diff.c
+++ b/diff.c
@@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct
diff_options *options)
*/
if (options->stat_width == -1)
- width = term_columns() - options->output_prefix_length;
+ width = term_columns() - strlen(line_prefix);
else
width = options->stat_width ? options->stat_width : 80;
number_width = decimal_width(max_change) > number_width ?
diff --git a/diff.h b/diff.h
index 7883729edf10..747a204d75a4 100644
--- a/diff.h
+++ b/diff.h
@@ -174,7 +174,6 @@ struct diff_options {
diff_format_fn_t format_callback;
void *format_callback_data;
diff_prefix_fn_t output_prefix;
- int output_prefix_length;
void *output_prefix_data;
int diff_path_counter;
diff --git a/graph.c b/graph.c
index dd1720148dc5..a46803840511 100644
--- a/graph.c
+++ b/graph.c
@@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct
diff_options *opt, void
assert(opt);
assert(graph);
- opt->output_prefix_length = graph->width;
strbuf_reset(&msgbuf);
graph_padding_line(graph, &msgbuf);
return &msgbuf;
@@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt)
*/
opt->diffopt.output_prefix = diff_output_prefix_callback;
opt->diffopt.output_prefix_data = graph;
- opt->diffopt.output_prefix_length = 0;
return graph;
}
--
2.10.0.rc0.217.g609f9e8.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 7/7] diff: teach diff to display submodule difference with an inline diff
From: Jacob Keller
Teach git-diff and friends a new format for displaying the difference
of a submodule. The new format is an inline diff of the contents of the
submodule between the commit range of the update. This allows the user
to see the actual code change caused by a submodule update.
Add tests for the new format and option.
Signed-off-by: Jacob Keller
---
Documentation/diff-config.txt| 9 +-
Documentation/diff-options.txt | 17 +-
diff.c | 31 +-
diff.h | 1 +
submodule.c | 61 +++
submodule.h | 6 +
t/t4059-diff-submodule-option-diff-format.sh | 733 +++
7 files changed, 838 insertions(+), 20 deletions(-)
create mode 100755 t/t4059-diff-submodule-option-diff-format.sh
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index d5a5b17d5088..0eded24034b5 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -122,10 +122,11 @@ diff.suppressBlankEmpty::
diff.submodule::
Specify the format in which differences in submodules are
- shown. The "log" format lists the commits in the range like
- linkgit:git-submodule[1] `summary` does. The "short" format
- format just shows the names of the commits at the beginning
- and end of the range. Defaults to short.
+ shown. The "short" format just shows the names of the commits
+ at the beginning and end of the range. The "log" format lists
+ the commits in the range like linkgit:git-submodule[1] `summary`
+ does. The "diff" format shows an inline diff of the changed
+ contents of the submodule. Defaults to "short".
diff.wordRegex::
A POSIX Extended Regular Expression used to determine what is a "word"
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index cc4342e2034d..7805a0ccadf2 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -210,13 +210,16 @@ any of those replacements occurred.
of the `--diff-filter` option on what the status letters mean.
--submodule[=]::
- Specify how differences in submodules are shown. When `--submodule`
- or `--submodule=log` is given, the 'log' format is used. This format
lists
- the commits in the range like linkgit:git-submodule[1] `summary` does.
- Omitting the `--submodule` option or specifying `--submodule=short`,
- uses the 'short' format. This format just shows the names of the commits
- at the beginning and end of the range. Can be tweaked via the
- `diff.submodule` configuration variable.
+ Specify how differences in submodules are shown. When specifying
+ `--submodule=short` the 'short' format is used. This format just
+ shows the names of the commits at the beginning and end of the range.
+ When `--submodule` or `--submodule=log` is specified, the 'log'
+ format is used. This format lists the commits in the range like
+ linkgit:git-submodule[1] `summary` does. When `--submodule=diff`
+ is specified, the 'diff' format is used. This format shows an
+ inline diff of the changes in the submodule contents between the
+ commit range. Defaults to `diff.submodule` or the 'short' format
+ if the config option is unset.
--color[=]::
Show colored diff.
diff --git a/diff.c b/diff.c
index d6b321da3d1d..e119758aba82 100644
--- a/diff.c
+++ b/diff.c
@@ -135,6 +135,8 @@ static int parse_submodule_params(struct diff_options
*options, const char *valu
options->submodule_format = DIFF_SUBMODULE_LOG;
else if (!strcmp(value, "short"))
options->submodule_format = DIFF_SUBMODULE_SHORT;
+ else if (!strcmp(value, "diff"))
+ options->submodule_format = DIFF_SUBMODULE_INLINE_DIFF;
else
return -1;
return 0;
@@ -2300,6 +2302,15 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
+ diff_set_mnemonic_prefix(o, "a/", "b/");
+ if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+ a_prefix = o->b_prefix;
+ b_prefix = o->a_prefix;
+ } else {
+ a_prefix = o->a_prefix;
+ b_prefix = o->b_prefix;
+ }
+
if (o->submodule_format == DIFF_SUBMODULE_LOG &&
(!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
@@ -2311,6 +2322,17 @@ static void builtin_diff(const char *name_a,
two->dirty_submodule,
meta, del, add, reset);
return;
+ } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
+ (!one->mode || S_ISGITLINK(one->m
[PATCH v7 3/7] diff: prepare for additional submodule formats
From: Jacob Keller
A future patch will add a new format for displaying the difference of
a submodule. Make it easier by changing how we store the current
selected format. Replace the DIFF_OPT flag with an enumeration, as each
format will be mutually exclusive.
Signed-off-by: Jacob Keller
---
diff.c | 12 ++--
diff.h | 7 ++-
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index e57cf39ad109..d6b321da3d1d 100644
--- a/diff.c
+++ b/diff.c
@@ -132,9 +132,9 @@ static int parse_dirstat_params(struct diff_options
*options, const char *params
static int parse_submodule_params(struct diff_options *options, const char
*value)
{
if (!strcmp(value, "log"))
- DIFF_OPT_SET(options, SUBMODULE_LOG);
+ options->submodule_format = DIFF_SUBMODULE_LOG;
else if (!strcmp(value, "short"))
- DIFF_OPT_CLR(options, SUBMODULE_LOG);
+ options->submodule_format = DIFF_SUBMODULE_SHORT;
else
return -1;
return 0;
@@ -2300,9 +2300,9 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
- if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
- (!one->mode || S_ISGITLINK(one->mode)) &&
- (!two->mode || S_ISGITLINK(two->mode))) {
+ if (o->submodule_format == DIFF_SUBMODULE_LOG &&
+ (!one->mode || S_ISGITLINK(one->mode)) &&
+ (!two->mode || S_ISGITLINK(two->mode))) {
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one->path ? one->path :
two->path,
@@ -3916,7 +3916,7 @@ int diff_opt_parse(struct diff_options *options,
DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
handle_ignore_submodules_arg(options, arg);
} else if (!strcmp(arg, "--submodule"))
- DIFF_OPT_SET(options, SUBMODULE_LOG);
+ options->submodule_format = DIFF_SUBMODULE_LOG;
else if (skip_prefix(arg, "--submodule=", &arg))
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
diff --git a/diff.h b/diff.h
index 1f57aad25c71..ea5aba668eaa 100644
--- a/diff.h
+++ b/diff.h
@@ -83,7 +83,6 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct
diff_options *opt, void *data)
#define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
#define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)
#define DIFF_OPT_DIFF_FROM_CONTENTS (1 << 22)
-#define DIFF_OPT_SUBMODULE_LOG (1 << 23)
#define DIFF_OPT_DIRTY_SUBMODULES(1 << 24)
#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)
@@ -110,6 +109,11 @@ enum diff_words_type {
DIFF_WORDS_COLOR
};
+enum diff_submodule_format {
+ DIFF_SUBMODULE_SHORT = 0,
+ DIFF_SUBMODULE_LOG,
+};
+
struct diff_options {
const char *orderfile;
const char *pickaxe;
@@ -157,6 +161,7 @@ struct diff_options {
int stat_count;
const char *word_regex;
enum diff_words_type word_diff;
+ enum diff_submodule_format submodule_format;
/* this is set by diffcore for DIFF_FORMAT_PATCH */
int found_changes;
--
2.10.0.rc0.217.g609f9e8.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 6/7] submodule: refactor show_submodule_summary with helper function
From: Jacob Keller
A future patch is going to add a new submodule diff format which
displays an inline diff of the submodule changes. To make this easier,
and to ensure that both submodule diff formats use the same initial
header, factor out show_submodule_header() function which will print the
current submodule header line, and then leave the show_submodule_summary
function to lookup and print the submodule log format.
This does create one format change in that "(revision walker failed)"
will now be displayed on its own line rather than as part of the message
because we no longer perform this step directly in the header display
flow. However, this is a rare case and shouldn't impact much. In
addition, since we no longer need to run prepare_submodule_summary to
get the fast_backward and fast_forward variables, these have been
removed from that call, and the show_submodule_header() function does
its own mergebase lookup.
Signed-off-by: Jacob Keller
---
submodule.c | 103 +++-
1 file changed, 74 insertions(+), 29 deletions(-)
diff --git a/submodule.c b/submodule.c
index e1a51b7506ff..e341ca7ffefd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -277,8 +277,7 @@ void handle_ignore_submodules_arg(struct diff_options
*diffopt,
}
static int prepare_submodule_summary(struct rev_info *rev, const char *path,
- struct commit *left, struct commit *right,
- int *fast_forward, int *fast_backward)
+ struct commit *left, struct commit *right)
{
struct commit_list *merge_bases, *list;
@@ -290,12 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev,
const char *path,
add_pending_object(rev, &left->object, path);
add_pending_object(rev, &right->object, path);
merge_bases = get_merge_bases(left, right);
- if (merge_bases) {
- if (merge_bases->item == left)
- *fast_forward = 1;
- else if (merge_bases->item == right)
- *fast_backward = 1;
- }
for (list = merge_bases; list; list = list->next) {
list->item->object.flags |= UNINTERESTING;
add_pending_object(rev, &list->item->object,
@@ -333,31 +326,23 @@ static void print_submodule_summary(struct rev_info *rev,
FILE *f,
strbuf_release(&sb);
}
-void show_submodule_summary(FILE *f, const char *path,
+/* Helper function to display the submodule header line prior to the full
+ * summary output. If it can locate the submodule objects directory it will
+ * attempt to lookup both the left and right commits and put them into the
+ * left and right pointers.
+ */
+static void show_submodule_header(FILE *f, const char *path,
const char *line_prefix,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule, const char *meta,
- const char *del, const char *add, const char *reset)
+ const char *reset,
+ struct commit **left, struct commit **right)
{
- struct rev_info rev;
- struct commit *left = NULL, *right = NULL;
+ struct commit_list *merge_bases;
const char *message = NULL;
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
- if (is_null_sha1(two))
- message = "(submodule deleted)";
- else if (add_submodule_odb(path))
- message = "(not initialized)";
- else if (is_null_sha1(one))
- message = "(new submodule)";
- else if (!(left = lookup_commit_reference(one)) ||
-!(right = lookup_commit_reference(two)))
- message = "(commits not present)";
- else if (prepare_submodule_summary(&rev, path, left, right,
- &fast_forward, &fast_backward))
- message = "(revision walker failed)";
-
if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
fprintf(f, "%sSubmodule %s contains untracked content\n",
line_prefix, path);
@@ -365,11 +350,46 @@ void show_submodule_summary(FILE *f, const char *path,
fprintf(f, "%sSubmodule %s contains modified content\n",
line_prefix, path);
+ if (is_null_sha1(one))
+ message = "(new submodule)";
+ else if (is_null_sha1(two))
+ message = "(submodule deleted)";
+
+ if (add_submodule_odb(path)) {
+ if (!message)
+ message = "(submodule not initialized)";
+ goto output_header;
+ }
+
+ /*
+* Attempt to lookup the commit references, and determine if this is
+* a fast forward or fast backwards update
+*/
+ *left = lookup_commit_reference(one);
+ *right = lookup_commit_reference(two);
+
+ /*
+* Warn about missing commits in the submo
[PATCH v7 0/7] implement inline submodule diff format
From: Jacob Keller As suggested by Junio, I broke this patch into several pieces, and made a common helper function for the submodule header. Note that there are a couple of complicated modifications to the submodule header portion which (should) still result in the same header but allow the diff format a bit better control. It's a bit awkward but it does pass all the current tests, and the format between the two should be similar. I tried to break apart the patches properly, so I hope I didn't accidentally munge them too badly. Jacob Keller (6): graph: add support for --line-prefix on all graph-aware output diff: prepare for additional submodule formats submodule: allow do_submodule_path to work if given gitdir directly submodule: correct output of submodule log format submodule: refactor show_submodule_summary with helper function diff: teach diff to display submodule difference with an inline diff Junio C Hamano (1): diff.c: remove output_prefix_length field Documentation/diff-config.txt | 9 +- Documentation/diff-options.txt | 20 +- builtin/rev-list.c | 70 +- diff.c | 52 +- diff.h | 11 +- graph.c| 106 +-- graph.h| 22 +- log-tree.c | 5 +- path.c | 4 +- submodule.c| 162 - submodule.h| 6 + t/t4013-diff-various.sh| 6 + ...diff.diff_--line-prefix=abc_master_master^_side | 29 + t/t4013/diff.diff_--line-prefix_--cached_--_file0 | 15 + t/t4059-diff-submodule-option-diff-format.sh | 733 + t/t4202-log.sh | 323 + 16 files changed, 1432 insertions(+), 141 deletions(-) create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0 create mode 100755 t/t4059-diff-submodule-option-diff-format.sh -- 2.10.0.rc0.217.g609f9e8.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 4/7] submodule: allow do_submodule_path to work if given gitdir directly
From: Jacob Keller
Currently, do_submodule_path relies on read_gitfile, which will die() if
it can't read from the specified gitfile. Unfortunately, this means that
do_submodule_path will not work when given the path to a submodule which
is checked out directly, such as a newly added submodule which you
cloned and then "git submodule add". Instead, replace the call with
resolve_gitdir. This first checks to see if we've been given a gitdir
already.
Because resolve_gitdir may return the same buffer it was passed, we have
to check for this case as well, since strbuf_reset() will not work as
expected here, and indeed is not necessary.
Signed-off-by: Jacob Keller
---
path.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/path.c b/path.c
index 17551c483476..d1af029152a2 100644
--- a/path.c
+++ b/path.c
@@ -477,8 +477,8 @@ static void do_submodule_path(struct strbuf *buf, const
char *path,
strbuf_complete(buf, '/');
strbuf_addstr(buf, ".git");
- git_dir = read_gitfile(buf->buf);
- if (git_dir) {
+ git_dir = resolve_gitdir(buf->buf);
+ if (git_dir && git_dir != buf->buf) {
strbuf_reset(buf);
strbuf_addstr(buf, git_dir);
}
--
2.10.0.rc0.217.g609f9e8.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 5/7] submodule: correct output of submodule log format
From: Jacob Keller The submodule log diff output incorrectly states that the submodule is "not checked out" in cases where it wants to say the submodule is "not initialized". Change the wording to reflect the actual check being performed. Signed-off-by: Jacob Keller --- submodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodule.c b/submodule.c index 1b5cdfb7e784..e1a51b7506ff 100644 --- a/submodule.c +++ b/submodule.c @@ -348,7 +348,7 @@ void show_submodule_summary(FILE *f, const char *path, if (is_null_sha1(two)) message = "(submodule deleted)"; else if (add_submodule_odb(path)) - message = "(not checked out)"; + message = "(not initialized)"; else if (is_null_sha1(one)) message = "(new submodule)"; else if (!(left = lookup_commit_reference(one)) || -- 2.10.0.rc0.217.g609f9e8.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/7] graph: add support for --line-prefix on all graph-aware output
From: Jacob Keller
Add an extension to git-diff and git-log (and any other graph-aware
displayable output) such that "--line-prefix=" will print the
additional line-prefix on every line of output.
To make this work, we have to fix a few bugs in the graph API that force
graph_show_commit_msg to be used only when you have a valid graph.
Additionally, we extend the default_diff_output_prefix handler to work
even when no graph is enabled.
This is somewhat of a hack on top of the graph API, but I think it
should be acceptable here.
This will be used by a future extension of submodule display which
displays the submodule diff as the actual diff between the pre and post
commit in the submodule project.
Add some tests for both git-log and git-diff to ensure that the prefix
is honored correctly.
Signed-off-by: Jacob Keller
---
Documentation/diff-options.txt | 3 +
builtin/rev-list.c | 70 ++---
diff.c | 7 +
diff.h | 2 +
graph.c| 104 ---
graph.h| 22 +-
log-tree.c | 5 +-
t/t4013-diff-various.sh| 6 +
...diff.diff_--line-prefix=abc_master_master^_side | 29 ++
t/t4013/diff.diff_--line-prefix_--cached_--_file0 | 15 +
t/t4202-log.sh | 323 +
11 files changed, 506 insertions(+), 80 deletions(-)
create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 705a87394200..cc4342e2034d 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -569,5 +569,8 @@ endif::git-format-patch[]
--no-prefix::
Do not show any source or destination prefix.
+--line-prefix=::
+ Prepend an additional prefix to every line of output.
+
For more detailed explanation on these common options, see also
linkgit:gitdiffcore[7].
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0ba82b1635b6..1a75a83538f4 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -122,48 +122,40 @@ static void show_commit(struct commit *commit, void *data)
ctx.fmt = revs->commit_format;
ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(&ctx, commit, &buf);
- if (revs->graph) {
- if (buf.len) {
- if (revs->commit_format != CMIT_FMT_ONELINE)
- graph_show_oneline(revs->graph);
+ if (buf.len) {
+ if (revs->commit_format != CMIT_FMT_ONELINE)
+ graph_show_oneline(revs->graph);
- graph_show_commit_msg(revs->graph, &buf);
+ graph_show_commit_msg(revs->graph, stdout, &buf);
- /*
-* Add a newline after the commit message.
-*
-* Usually, this newline produces a blank
-* padding line between entries, in which case
-* we need to add graph padding on this line.
-*
-* However, the commit message may not end in a
-* newline. In this case the newline simply
-* ends the last line of the commit message,
-* and we don't need any graph output. (This
-* always happens with CMIT_FMT_ONELINE, and it
-* happens with CMIT_FMT_USERFORMAT when the
-* format doesn't explicitly end in a newline.)
-*/
- if (buf.len && buf.buf[buf.len - 1] == '\n')
- graph_show_padding(revs->graph);
- putchar('\n');
- } else {
- /*
-* If the message buffer is empty, just show
-* the rest of the graph output for this
-* commit.
-*/
- if (graph_show_remainder(revs->graph))
- putchar('\n');
- if (revs->commit_format == CMIT_FMT_ONELINE)
- putchar('\n');
- }
+ /*
+ * Add a newline afte
Re: Working from different machines.
On Wed, Aug 17, 2016 at 3:41 PM, Junio C Hamano wrote: > If the answer is "yes", then you are in the problem space that > Git-the-tool is interested in solving. Assuming that you have > network connection into 'desktop' from 'home', the solution would > involve making it the first thing to do when get home to run "git > fetch" on 'home' to get the latest state from the 'desktop', and run > "git push" on 'home' to push out the latest state to the 'desktop' > before you leave 'home'. If your 'server' is for your sole use, and > if 'home' has network connection into 'server', then you could > instead rendezvous at 'server' by running "git push server" on > 'desktop' (or 'home') to 'server' as the last thing before you leave > 'desktop' (or 'home'), and running "git fetch server" on 'home' (or > 'desktop') as the first thing before you start working on 'home' (or > 'desktop'). > > To add to this, you may need to modify the "git push" portion of the phases and the "git fetch" potion to include all the branches you are interested in. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
