Re: whither merge-tree?
On Tue, Feb 23, 2016 at 10:49:34AM +0100, Stefan Frühwirth wrote: > On 23.02.2016 at 06:02 Jeff King wrote: > >>Let's wait and see how many "please don't"s we hear, perhaps, before > >>deciding to go 3.? > > > >I'm guessing we won't see much either way. Even Stefan, the original > >reporter, does not seem to actively be using it, but rather relaying a > >report. > > I _am_ actively using it. Maybe I was unclear on that topic. I'm in favour > of keeping it, because this means I don't have to rewrite Chris' Code in > order to be able to use the Python library that uses merge-tree (Acidfs). > But as a sensible human being I want what's best in the long run. I leave > that up to you as I have no way of assessing that. Ah, sorry for the confusion. I had thought you were just relaying bugs on behalf of acidfs folks, but it makes sense that you are using it indirectly through the library. > So that's a "please don't" leave the code as-is but provide a (transitional) > solution that fixes the reported bug and has the best chances of not causing > any more headaches :) I think the series I posted does what you want, then, and we can stop short of deprecating for now. -Peff -- 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: whither merge-tree?
On Wed, Feb 24, 2016 at 08:28:59AM +0100, Dennis Kaarsemaker wrote: > > So that's a "please don't" leave the code as-is but provide a > > (transitional) solution that fixes the reported bug and has the best > > chances of not causing any more headaches :) > > I am also actively using it. It's the only way (I know of) of trying to > preview a merge result without attempting the actual merge, which is > useful in some of my scripts. OK. I guess we can live with the series I posted earlier (to simplify the add/add behavior and fix the overflow), and leave it in place. I _do_ think it's a useful concept, and there isn't something quite like it in git right now. At one point I contemplated teaching unpack-trees to do the content-level merges too, so you could do this via read-tree, but I didn't ever polish it[1]. So I am sympathetic to the goal, and there's not another way to do it as efficiently. And now I feel like both of you have been warned that the code might be crufty. :) I'm not sure what we should do about warning others. Since people are using it, I don't want to put a deprecation warning. It's not "this will be removed in the next version" anymore, but "watch out, this might be crufty". I guess that can go in the manpage, but I'm not quite sure how to word it. -Peff [1] It looks like my patch has been collecting dust since 2011: git://github.com/peff/git.git jk/read-tree-content-merge-wip It's been rebased along with all of my other topics since then, but otherwise I have no clue if it works or even compiles (it's not part of my day-to-day build). Anyone is welcome to use it as a base if they want to pursue it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] t5504: handle expected output from SIGPIPE death
Commit 8bf4bec (add "ok=sigpipe" to test_must_fail and use
it to fix flaky tests, 2015-11-27) taught t5504 to handle
"git push" racily exiting with SIGPIPE rather than failing.
However, one of the tests checks the output of the command,
as well. In the SIGPIPE case, we will not have produced any
output. If we want the test to be truly non-flaky, we have
to accept either output.
Signed-off-by: Jeff King
---
I'm not sure we're really accomplishing anything with this test_cmp
anymore. We'll complain only if we get _some_ output, and it's not the
expected output. I'm not sure if that's actually useful.
It looks like 8bf4bec just dropped the test_cmp completely in one of the
cases. So an alternative here would be to do the same.
We _could_ also tighten this, to make sure the output matches the exit
code we got (right now we can get code=128 with blank output and not
complain, even though that's clearly bogus). But we'd have to abandon
test_must_fail and do things manually.
t/t5504-fetch-receive-strict.sh | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
index 89224ed..a3e12d2 100755
--- a/t/t5504-fetch-receive-strict.sh
+++ b/t/t5504-fetch-receive-strict.sh
@@ -101,7 +101,10 @@ test_expect_success 'push with receive.fsckobjects' '
git config transfer.fsckobjects false
) &&
test_must_fail ok=sigpipe git push --porcelain dst
master:refs/heads/test >act &&
- test_cmp exp act
+ {
+ test_cmp exp act ||
+ ! test -s act
+ }
'
test_expect_success 'push with transfer.fsckobjects' '
--
2.7.2.645.g4e1306c
--
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-completion.bash: always swallow error output of for-each-ref
On Wed, Feb 24, 2016 at 12:30 AM, Junio C Hamano wrote: > So, have we decided to wait, or we'd rather apply the band-aid in > the meantime? I can go either way, just double checking as I > noticed this thread while updating my leftover bits list. Thanks for the follow-up, I was about to ask for a status update on this. As my patch it ready now, and we don't know how long we'd have to wait for the other solution, I'd vote for applying my patch. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git config: do not create .git/ if it does not exist yet
It is a pilot error to call `git config section.key value` outside of
any Git worktree.
Let's report that error instead of creating the .git/ directory and
writing a fresh config into it.
This addresses https://github.com/git-for-windows/git/issues/643 and
https://groups.google.com/forum/#!topic/git-for-windows/fVRdnDIKVuw
Signed-off-by: Johannes Schindelin
---
I cannot think of a way how to test this: all of the regression
tests run inside Git's own worktree, and we cannot even assume
that /tmp/ is outside of a worktree (or that it exists).
builtin/config.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/config.c b/builtin/config.c
index adc7727..78aab95 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -352,6 +352,9 @@ static int get_colorbool(const char *var, int print)
static void check_write(void)
{
+ if (!given_config_source.file && !startup_info->have_repository)
+ die("not in a git directory");
+
if (given_config_source.use_stdin)
die("writing to stdin is not supported");
--
2.7.2.windows.1.2.gbc859c8
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] test_must_fail: report number of unexpected signal
If a command is marked as test_must_fail but dies with a
signal, we consider that a problem and report the error to
stderr. However, we don't say _which_ signal; knowing that
can make debugging easier. Let's share as much as we know.
Signed-off-by: Jeff King
---
Not necessary for the fix, obviously, but I implemented this while
trying to figure out what in the world was going on with the
write_or_die() thing. :)
t/test-lib-functions.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index c64e5a5..8d99eb3 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -617,7 +617,7 @@ test_must_fail () {
return 0
elif test $exit_code -gt 129 && test $exit_code -le 192
then
- echo >&2 "test_must_fail: died by signal: $*"
+ echo >&2 "test_must_fail: died by signal $(($exit_code - 128)):
$*"
return 1
elif test $exit_code -eq 127
then
--
2.7.2.645.g4e1306c
--
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/4] fetch-pack: ignore SIGPIPE in sideband demuxer
If the other side feeds us a bogus pack, index-pack (or
unpack-objects) may die early, before consuming all of its
input. As a result, the sideband demuxer may get SIGPIPE
(racily, depending on whether our data made it into the pipe
buffer or not). If this happens and we are compiled with
pthread support, it will take down the main thread, too.
This isn't the end of the world, as the main process will
just die() anyway when it sees index-pack failed. But it
does mean we don't get a chance to say "fatal: index-pack
failed" or similar. And it also means that we racily fail
t5504, as we sometimes die() and sometimes are killed by
SIGPIPE.
So let's ignore SIGPIPE while demuxing the sideband. We are
already careful to check the return value of write(), so we
won't waste time writing to a broken pipe. The caller will
notice the error return from the async thread, though in
practice we don't even get that far, as we die() as soon as
we see that index-pack failed.
The non-sideband case is already fine; we let index-pack
read straight from the socket, so there is no SIGPIPE at
all. Technically the non-threaded async case is also OK
without this (the forked async process gets SIGPIPE), but
it's not worth distinguishing from the threaded case here.
Signed-off-by: Jeff King
---
It's tempting to just ignore SIGPIPE in all async processes,
as other sites may have similar problems (and we cannot
depend on SIGPIPE taking down the main thread anyway, as
async code may be implemented via fork()).
But that errs in the opposite direction. If an async process
does not check the return value of write(), it may
wastefully keep writing.
It would also mean implementing a pthread_sigmask() wrapper
on Windows. I have no idea how feasible that would be.
fetch-pack.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 01e34b6..f96f6df 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -15,6 +15,7 @@
#include "version.h"
#include "prio-queue.h"
#include "sha1-array.h"
+#include "sigchain.h"
static int transfer_unpack_limit = -1;
static int fetch_unpack_limit = -1;
@@ -671,9 +672,12 @@ static int everything_local(struct fetch_pack_args *args,
static int sideband_demux(int in, int out, void *data)
{
int *xd = data;
+ int ret;
- int ret = recv_sideband("fetch-pack", xd[0], out);
+ sigchain_push(SIGPIPE, SIG_IGN);
+ ret = recv_sideband("fetch-pack", xd[0], out);
close(out);
+ sigchain_pop(SIGPIPE);
return ret;
}
--
2.7.2.645.g4e1306c
--
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/4] write_or_die: handle EPIPE in async threads
When write_or_die() sees EPIPE, it treats it specially by
converting it into a SIGPIPE death. We obviously cannot
ignore it, as the write has failed and the caller expects us
to die. But likewise, we cannot just call die(), because
printing any message at all would be a nuisance during
normal operations.
However, this is a problem if write_or_die() is called from
a thread. Our raised signal ends up killing the whole
process, when logically we just need to kill the thread
(after all, if we are ignoring SIGPIPE, there is good reason
to think that the main thread is expecting to handle it).
Inside an async thread, the die() code already does the
right thing, because we use our custom die_async() routine,
which calls pthread_join(). So ideally we would piggy-back
on that, and simply call:
die_quietly_with_code(141);
or similar. But refactoring the die code to do this is
surprisingly non-trivial. The die_routines themselves handle
both printing and the decision of the exit code. Every one
of them would have to be modified to take new parameters for
the code, and to tell us to be quiet.
Instead, we can just teach write_or_die() to check for the
async case and handle it specially. We do have to build an
interface to abstract the async exit, but it's simple and
self-contained. If we had many call-sites that wanted to do
this die_quietly_with_code(), this approach wouldn't scale
as well, but we don't. This is the only place where do this
weird exit trick.
Signed-off-by: Jeff King
---
This is needed for patch 2, wherein we (surprise!) call write_or_die()
in a thread and want to ignore SIGPIPE.
Obviously another solution is "don't call write_or_die() in a thread",
and that would work for patch 2 here. But it can be hard to know whether
or not write_or_die() is called deep in the call stack. So even though
this solution is slightly more complex, I like that it behaves correctly
in all cases.
run-command.c | 10 ++
run-command.h | 1 +
write_or_die.c | 4
3 files changed, 15 insertions(+)
diff --git a/run-command.c b/run-command.c
index cdf0184..426387b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -635,6 +635,11 @@ int in_async(void)
return !pthread_equal(main_thread, pthread_self());
}
+void async_exit(int code)
+{
+ pthread_exit((void *)(intptr_t)code);
+}
+
#else
static struct {
@@ -680,6 +685,11 @@ int in_async(void)
return process_is_async;
}
+int async_exit(int code)
+{
+ exit(code);
+}
+
#endif
int start_async(struct async *async)
diff --git a/run-command.h b/run-command.h
index d5a57f9..42917e8 100644
--- a/run-command.h
+++ b/run-command.h
@@ -121,6 +121,7 @@ struct async {
int start_async(struct async *async);
int finish_async(struct async *async);
int in_async(void);
+void NORETURN async_exit(int code);
/**
* This callback should initialize the child process and preload the
diff --git a/write_or_die.c b/write_or_die.c
index e7afe7a..49e80aa 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -1,8 +1,12 @@
#include "cache.h"
+#include "run-command.h"
static void check_pipe(int err)
{
if (err == EPIPE) {
+ if (in_async())
+ async_exit(141);
+
signal(SIGPIPE, SIG_DFL);
raise(SIGPIPE);
/* Should never happen, but just in case... */
--
2.7.2.645.g4e1306c
--
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/4] robustifying t5504
I got a spurious test failure on t5504 while running the test suite
today. This is the result of my quest through the SIGPIPE rabbit hole.
Since this is not the first time I've investigated tests failing under
load, I finally broke down and wrote a helper script. It probably needs
some adapting for other people's environments, but I'll share it here in
case anyone is interested:
-- >8 --
#!/bin/sh
test=$1; shift
test=$(cd t && echo $test*.sh)
: ${GIT_STRESS_LOAD:=$(( 2 * $(grep -c ^processor /proc/cpuinfo)))}
: ${GIT_STRESS_ROOT:=/var/ram/git-stress}
mkdir -p "$GIT_STRESS_ROOT" || exit 1
rm -f "$GIT_STRESS_ROOT/fail"
trap 'echo aborted >"$GIT_STRESS_ROOT/fail"' TERM INT HUP
for i in $(seq $GIT_STRESS_LOAD); do
(
cd t &&
while ! test -e "$GIT_STRESS_ROOT/fail"
do
if ./$test \
--root="$GIT_STRESS_ROOT/trash-$i" \
-v -i >"$GIT_STRESS_ROOT/output-$i" 2>&1
then
echo >&2 "OK $i"
else
echo >&2 "FAIL $i"
cp "$GIT_STRESS_ROOT/output-$i"
"$GIT_STRESS_ROOT/fail"
fi
done
) &
done
wait
cat "$GIT_STRESS_ROOT/fail"
-- 8< --
You can run it like "./stress t5504", and it will run t5504 over and
over in parallel until one instance fails. Without this patch series,
t5504 generally fails for me within 30 seconds or so. With it, I can run
for several minutes without problems.
[1/4]: write_or_die: handle EPIPE in async threads
[2/4]: fetch-pack: ignore SIGPIPE in sideband demuxer
[3/4]: test_must_fail: report number of unexpected signal
[4/4]: t5504: handle expected output from SIGPIPE death
-Peff
--
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: whither merge-tree?
On di, 2016-02-23 at 10:49 +0100, Stefan Frühwirth wrote: > On 23.02.2016 at 06:02 Jeff King wrote: > > > Let's wait and see how many "please don't"s we hear, perhaps, > > > before > > > deciding to go 3.? > > > > I'm guessing we won't see much either way. Even Stefan, the > > original > > reporter, does not seem to actively be using it, but rather > > relaying a > > report. > > I _am_ actively using it. Maybe I was unclear on that topic. I'm in > favour of keeping it, because this means I don't have to rewrite > Chris' > Code in order to be able to use the Python library that uses merge- > tree > (Acidfs). But as a sensible human being I want what's best in the > long > run. I leave that up to you as I have no way of assessing that. > > So that's a "please don't" leave the code as-is but provide a > (transitional) solution that fixes the reported bug and has the best > chances of not causing any more headaches :) I am also actively using it. It's the only way (I know of) of trying to preview a merge result without attempting the actual merge, which is useful in some of my scripts. -- Dennis Kaarsemaker www.kaarsemaker.net -- 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/5] README: use markdown syntax
Hi Junio, On Tue, 23 Feb 2016, Junio C Hamano wrote: > Matthieu Moy writes: > > > This allows repository browsers like GitHub to display the content of > > the file nicely formatted. > > > > Signed-off-by: Matthieu Moy > > --- > > To be honest, I have the most problem with this step in the whole > series. > > Markdown when rendered may be easier to read, but plain text is even > easier, and it somehow feels backward to cater to those who browse > at GitHub sacrificing those who use "less" in the source tree. That assumes that the primary audience of the README file is the developers who already decided to clone the repository, as opposed to people browsing the README file in the browser to determine whether they found the correct project, or to read up on the background of the project without downloading the entire source code. I'd wager real money (without scientific evidence. just going on common sense) that your 'less' people are in the vast minority. Since I am convinced that markdown'ed READMEs enhance the user experience dramatically, Git for Windows has one already for a long time. Hence *my* main objection: this patch series would conflict with patches we carry in Git for Windows. ;-) Ciao, Dscho P.S.: If it was not clear, my objection was meant as a joke. I am very much in favor of enhancing the user experience via Matthieu's patches. -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
On Tue, Feb 23, 2016 at 10:30:04PM -0800, Junio C Hamano wrote: > Fengguang Wu writes: > > > The necessary lines for the robot are > > > > base commit: > > base patch-id: > > or > > base tree-id: > > base patch-id: > > I will not repeat why a commit object name would be more appropriate > than a tree object name here (please see my response to HPA). Yes I see that reasoning in your other email. > > The "base tree-id" will be useful if the submitted patchset is based > > on a public (maintainer) commit. > > > > The "base patch-id" will be useful if the submitted patchset is based > > on another patchset someone (likely the developer himself) posted to > > the mailing list. > > Is there a database of in-flight patches indexed by their patch-ids > with a large enough coverage (hopefully those who maintain such a Yes, the 0day robot internally maintains such a patch-id => commit-id (of the below git tree) database for in-flight patches. We exported a git tree which holds all in-flight patches, where each patchset maps to a new branch: https://github.com/0day-ci/linux/branches We monitor dozens of linux kernel mailing lists, the coverage is pretty good for the linux kernel project. > database are using the --stable version of the patch-id for indexing > the patches)? Right, we do use the --stable option. > I am wondering how well this scales, especially if a > well-known commit named by "base commit" needs to be checked out and > then many in-flight patches identified by "base patch-id"s need to > be applied on top of it, to prepare the tree-ish the patch being > evaluated can be applied to. The database is effectively a key-value store, in the scale of 1000 new mappings per day. If we only keep 100 days data, there will be 100k mappings, which could be hold in 10MB memory. > This starts to sound more like something you would want to write in > the cover letter, or the trailer block next to Signed-off-by: at the > end of the first patch in the series. Yes, that's roughly what the current patch does, except in the latter case we add new info after diffstat. > Or even after the mail > signature at the very end of the message (incidentally that would > probably minimize the damage to the Git codebase needed for this > addition--you should be able to do this without touching anything > other than builtin/log.c). That's an interesting place. It looks worth trying. Thanks, Fengguang -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
Fengguang Wu writes: > The necessary lines for the robot are > > base commit: > base patch-id: > or > base tree-id: > base patch-id: I will not repeat why a commit object name would be more appropriate than a tree object name here (please see my response to HPA). > The "base tree-id" will be useful if the submitted patchset is based > on a public (maintainer) commit. > > The "base patch-id" will be useful if the submitted patchset is based > on another patchset someone (likely the developer himself) posted to > the mailing list. Is there a database of in-flight patches indexed by their patch-ids with a large enough coverage (hopefully those who maintain such a database are using the --stable version of the patch-id for indexing the patches)? I am wondering how well this scales, especially if a well-known commit named by "base commit" needs to be checked out and then many in-flight patches identified by "base patch-id"s need to be applied on top of it, to prepare the tree-ish the patch being evaluated can be applied to. This starts to sound more like something you would want to write in the cover letter, or the trailer block next to Signed-off-by: at the end of the first patch in the series. Or even after the mail signature at the very end of the message (incidentally that would probably minimize the damage to the Git codebase needed for this addition--you should be able to do this without touching anything other than builtin/log.c). -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
"H. Peter Anvin" writes: > Personally, as a maintainer, I would love to see the tree ID and > ideally also the commit ID a series is based on. The commit ID is > in some ways less useful since it is non-recreatable (and > therefore will never match for anything but the first patch of a > series), but could be useful to the maintainer. I admit that the very first "applies-to" proposal I made long time ago was based on a tree object name, not a commit object name like the proposal under discussion here, but I doubt that a tree object name is that much more useful than a commit object name in this context. Below, I assume that you are envisioning that the "base tree" recorded in a patch does not necessarily name a public, well-known tree (e.g. a tree-ish that already appears in Linus's tree for those who work with his tree, or other relevant trees like linux-next or net tree) [*1*]. It would name an unknown tree that results by applying a set of well-known patches in-flight on a public well-known commit. In that set-up, because you cannot guess committer identity and timestamp that are used by the patch submitter when these in-flight patches are applied to prepare the base for these private commits, a commit object name is useless, but it may still be possible for you to independently compute these trees that would result from set of well-known in-flight patches. But I do not think "it may be possible" above translates to usefulness in practice. Suppose we have only three well-known in-flight patches that are unrelated and independent, and you somehow know that the patch submitter built the first patch in the series by working on either a recently tagged commit (say v4.4) or a result of applying some of these in-flight patches on top of that commit. Even with these three commits, the base tree the patch submitter based his or her work on could be v4.4 itself, v4.4 plus one of the three patches (v4.4+A, v4.4+B, v4.4+C, three possibilities in total), v4.4 plus two of the three patches in some order (v4.4+AB, v4.4+BC, v4.4+CA, three possibilities in total) or v4.4 plus all of the three patches, so there are 8 possible top-level tree objects in total. Unless you are doing something unusual [*2*], even if you have all of these three well-known in-flight patches in your repository, you would have only a subset of them (you would certainly have v4.4, and v4.4 plus all three patches, but you would likely to have only one path between these two points, that's four commits recording four trees, out of possible 8). In the real world, of course you have far more than three well-known in-flight patches, so even though in theory trees may have better chance to be "figured out", I do not think it is practical to even attempt to "figure out" an unknown state given a tree object name. So assuming that it is a good idea to add some information to a patch that identifies the whole tree it applies to, I think it is sensible to (1) limit that identifiable set of tree-ishes only to well-known public ones, and (2) use the commit object name, not the tree object name, for the purpose of identifying these tree-ishes. If I understand Fengguang's plan correctly, a new work based on a public well-known base tree-ish plus other patches in-flight are to be accompanied by the identifier for that well-known base tree-ish and some identifiers for these in-flight patches, i.e. the robot will be told to check out the well-known base tree-ish, apply the prerequisite patches and then the patches for the work are applied on top to be evaluated. So the above two limitations I placed in the previous paragraph would not hurt the identifiability of the "base" tree-ish, I would think. [Footnote] *1* If you limit the bases to these well known ones, then there is no practical difference between commit and tree, because we can assume as a maintainer you would have these commits so you would have both, and once located, a commit would be easier to reason about (e.g. run "log" to see what changes there are between it and a well known tags). *2* By "something unusual" I mean you prepare the permutations of in-flight patches in your repository, to make it possible to find any of the 8 tree objects in this senario. -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
On Tue, Feb 23, 2016 at 12:35:12PM -0800, Junio C Hamano wrote: > ebied...@xmission.com (Eric W. Biederman) writes: > > > Junio C Hamano writes: > > > >> It is valuable for a testing organization to say "We tested this > >> series on top of version X. We know it works, we have tested on a > >> lot more hardware than the original developer had, we know this is > >> good to go." It is a valuable service. > >> > >> But that is valuable only if version X is still relevant, isn't it? > >> > >> Is the relevance of a version something that is decided by a > >> developer who submits a patch series, or is it more of an attribute > >> of the project and where the current integration is happening? > >> Judging from the responses from Dan to this thread, I think the > >> answer is the latter, and for the purpose of identifying the > >> relevant version(s), the project does not even care about the exact > >> commit, but it wants to know more about which branch the series is > >> targetted to. > >> > >> With that understanding, I find it hard to believe that it buys the > >> project much for the "base" commit to be recorded in a patch series > >> and automated testing is done by applying the patches to that exact > >> commit, which possibly is no-longer-relevant, even though it may > >> help shielding the testing machinery from "you tested with a wrong > >> version" complaints. > >> > >> Isn't it more valuable for the test robot to say "this may or may > >> not have worked well with whatever old version the patch series was > >> based on, but it no longer is useful to the current tip of the > >> 'master'"? If you consider what benefit the project would gain by > >> having such a robot, that is the conclusion I have to draw. > >> > >> So I still am not convinced that this "record base commit" is a > >> useful thing to do. > > > > So I don't know what value this has to the git project. > > Oh, don't worry, I wasn't talking about value this may have to the > Git project at all. "The value to the project" I mentioned in my > response was all about the value to the kernel project. > > > The value of Fengguag's automated testing to the kernel project is to > > smoke out really stupid things. > > I'll snip your bullet points, but as I wrote, I do understand the > value of prescreening test. > > What I was questioning was what value it gives to that testing to > use "the developer based this patch on this exact commit" added to > each incoming patch, and have Fengguag's testing machinery test a > patch with a base version that may no longer be relevant in the > context of the project. Compared to that, wouldn't "this no longer > applies to the branch the series targets" or "this still applies > cleanly, but no longer compiles because the surrounding API has > changed" be much more valuable? > > In your other message, you mentioned the "index $old..$new" lines, > and it is possible to use them to find a tree that the patch cleanly > applies to, but it will not uniquely identify _the_ version the > patch submitter used. IMHO, finding such _a_ tree from the recent > history of the branch that the patch targets and testing the patch Problem is, it's typically not clear what the patch "targets". Linux is such a big project, there are dozens of maintainer trees and an order more branches in their trees. The robot does a hard work trying to guess what are the patches' targets, based on various hints like the files being modified, the TO/CC list, the subject, etc. However it's error prone -- even big maintainers cannot make a certain judgement at times: "shall me or you take the patch?". Not to mention the semi-private rules of co-maintainers, sub-maintainers and topic branches. Sometimes people are backporting patches, hence "don't test my patch on new RC kernels"; Or people may be working on cool futurism patches that depend on more changes than the latest RC kernel. To a robot, it's really hard to avoid stupid actions in such a versatile bazaar. The "base commit/tree-id" under discussion would be a perfect compass to guide the bot. Thanks, Fengguang > on top of that tree (as opposed to testing the patch in the exact > context the developer worked on) would give the project a better > value. -- 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] submodule: Try harder to fetch needed sha1 by direct fetching sha1
When reviewing a change in Gerrit, which also updates a submodule, a common review practice is to download and cherry-pick the patch locally to test it. However when testing it locally, the 'git submodule update' may fail fetching the correct submodule sha1 as the corresponding commit in the submodule is not yet part of the project history, but also just a proposed change. If $sha1 was not part of the default fetch, we try to fetch the $sha1 directly. Some servers however do not support direct fetch by sha1, which leads git-fetch to fail quickly. We can fail ourselves here as the still missing sha1 would lead to a failure later in the checkout stage anyway, so failing here is as good as we can get. Helped-by: Junio C Hamano Signed-off-by: Stefan Beller --- Junio, I took your patch and reworded the error message, so 25 / 26 lines of the code are still originally authored by you. But as "it is my itch not yours" I am resending it. If you changed your mind, take authorship of this. Thanks, Stefan git-submodule.sh | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 9bc5c5f..babfc64 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -591,6 +591,24 @@ cmd_deinit() done } +is_tip_reachable () ( + clear_local_git_env + cd "$1" && + rev=$(git rev-list -n 1 "$2" --not --all 2>/dev/null) && + test -z "$rev" +) + +fetch_in_submodule () ( + clear_local_git_env + cd "$1" && + case "$2" in + '') + git fetch ;; + *) + git fetch $(get_default_remote) "$2" ;; + esac +) + # # Update each submodule path to correct revision, using clone and checkout as needed # @@ -745,10 +763,15 @@ Maybe you want to use 'update --init'?")" then # Run fetch only if $sha1 isn't present or it # is not reachable from a ref. - (clear_local_git_env; cd "$sm_path" && - ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) && -test -z "$rev") || git-fetch)) || + is_tip_reachable "$sm_path" "$sha1" || + fetch_in_submodule "$sm_path" || die "$(eval_gettext "Unable to fetch in submodule path '\$displaypath'")" + + # Now we tried the usual fetch, but $sha1 may + # not be reachable from any of the refs + is_tip_reachable "$sm_path" "$sha1" || + fetch_in_submodule "$sm_path" "$sha1" || + die "$(eval_gettext "Fetched in submodule path '\$displaypath', but it did not contain $sha1. Direct fetching of that commit failed.")" fi # Is this something we just cloned? -- 2.7.0.rc0.34.ga06e0b3.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
[PATCHv15 2/5] run_processes_parallel: add LF when caller is sloppy
When the callers of parallel processing machine are sloppy with their
messages, make sure the output is terminated with LF after one child
process is handled. This will not mess with the internal state of the
output, i.e. after all messages for one child process are process
including the callbacks as well as the actual output of the child
we may add an LF if the output is not ended with an LF.
Signed-off-by: Stefan Beller
---
>From a discussion on a later patch, Jonathan said:
> It looks like pp_start_one takes the content of err without adding
> a \n. That's a bug in pp_start_one and pp_collect_finished IMHO.
run-command.c | 17 +++--
t/t0061-run-command.sh | 26 ++
test-run-command.c | 18 ++
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/run-command.c b/run-command.c
index d03ecaa..8abaae4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -897,6 +897,7 @@ struct parallel_processes {
struct pollfd *pfd;
unsigned shutdown : 1;
+ unsigned ended_with_newline: 1;
int output_owner;
struct strbuf buffered_output; /* of finished children */
@@ -979,6 +980,7 @@ static void pp_init(struct parallel_processes *pp,
pp->nr_processes = 0;
pp->output_owner = 0;
pp->shutdown = 0;
+ pp->ended_with_newline = 1;
pp->children = xcalloc(n, sizeof(*pp->children));
pp->pfd = xcalloc(n, sizeof(*pp->pfd));
strbuf_init(&pp->buffered_output, 0);
@@ -1053,6 +1055,7 @@ static int pp_start_one(struct parallel_processes *pp)
pp->data,
&pp->children[i].data);
strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);
+ strbuf_addch(&pp->buffered_output, '\n');
strbuf_reset(&pp->children[i].err);
if (code)
pp->shutdown = 1;
@@ -1095,9 +1098,11 @@ static void pp_buffer_stderr(struct parallel_processes
*pp, int output_timeout)
static void pp_output(struct parallel_processes *pp)
{
int i = pp->output_owner;
- if (pp->children[i].state == GIT_CP_WORKING &&
- pp->children[i].err.len) {
+ size_t len = pp->children[i].err.len;
+ if (pp->children[i].state == GIT_CP_WORKING && len) {
fputs(pp->children[i].err.buf, stderr);
+ pp->ended_with_newline =
+ (pp->children[i].err.buf[len - 1] == '\n');
strbuf_reset(&pp->children[i].err);
}
}
@@ -1107,6 +1112,7 @@ static int pp_collect_finished(struct parallel_processes
*pp)
int i, code;
int n = pp->max_processes;
int result = 0;
+ ssize_t len;
while (pp->nr_processes > 0) {
for (i = 0; i < pp->max_processes; i++)
@@ -1131,12 +1137,19 @@ static int pp_collect_finished(struct
parallel_processes *pp)
pp->pfd[i].fd = -1;
child_process_init(&pp->children[i].process);
+ len = pp->children[i].err.len - 1;
+ if (len >= 0 && pp->children[i].err.buf[len] != '\n')
+ strbuf_addch(&pp->children[i].err, '\n');
+
if (i != pp->output_owner) {
strbuf_addbuf(&pp->buffered_output,
&pp->children[i].err);
strbuf_reset(&pp->children[i].err);
} else {
+ if (len == -1 && !pp->ended_with_newline)
+ strbuf_addch(&pp->children[i].err, '\n');
fputs(pp->children[i].err.buf, stderr);
strbuf_reset(&pp->children[i].err);
+ pp->ended_with_newline = 1;
/* Output all other finished child processes */
fputs(pp->buffered_output.buf, stderr);
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 12228b4..5c6822c 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -77,6 +77,32 @@ test_expect_success 'run_command runs in parallel with more
tasks than jobs avai
test_cmp expect actual
'
+test_expect_success 'run_command ensures each command ends in LF' '
+ test-run-command run-command-parallel 3 sh -c "printf \"%s\n%s\" Hello
World" 2>actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<-EOF
+preloaded output of a child
+preloaded output of a child
+preloaded output of a child
+preloaded output of a child
+EOF
+
+test_expect_success 'run_command ensures each command ends in LF when output
is only in starting cb' '
+ test-run-command run-command-parallel 3 sh -c true 2>actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<-EOF
+EOF
+
+test_expect_success 'run_command ensures each command ends in LF except when
there is no output' '
+ test-run-command run-command-parallel-silent 3 sh -c true 2>actual &&
+ test_cmp ex
[PATCHv15 4/5] submodule update: expose parallelism to the user
Expose possible parallelism either via the "--jobs" CLI parameter or
the "submodule.fetchJobs" setting.
By having the variable initialized to -1, we make sure 0 can be passed
into the parallel processing machine, which will then pick as many parallel
workers as there are CPUs.
Signed-off-by: Stefan Beller
Signed-off-by: Junio C Hamano
---
Documentation/git-submodule.txt | 7 ++-
builtin/submodule--helper.c | 16
git-submodule.sh| 9 +
t/t7406-submodule-update.sh | 12
4 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 1572f05..13adebf 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -16,7 +16,7 @@ SYNOPSIS
'git submodule' [--quiet] deinit [-f|--force] [--] ...
'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
[-f|--force] [--rebase|--merge] [--reference ]
- [--depth ] [--recursive] [--] [...]
+ [--depth ] [--recursive] [--jobs ] [--] [...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]
[commit] [--] [...]
'git submodule' [--quiet] foreach [--recursive]
@@ -377,6 +377,11 @@ for linkgit:git-clone[1]'s `--reference` and `--shared`
options carefully.
clone with a history truncated to the specified number of revisions.
See linkgit:git-clone[1]
+-j ::
+--jobs ::
+ This option is only valid for the update command.
+ Clone new submodules in parallel with as many jobs.
+ Defaults to the `submodule.fetchJobs` option.
...::
Paths to submodule(s). When specified this will restrict the command
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 85fb702..32254cd 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -429,6 +429,7 @@ static int update_clone_task_finished(int result,
static int update_clone(int argc, const char **argv, const char *prefix)
{
const char *update = NULL;
+ int max_jobs = -1;
struct string_list_item *item;
struct pathspec pathspec;
struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
@@ -449,6 +450,8 @@ static int update_clone(int argc, const char **argv, const
char *prefix)
OPT_STRING(0, "depth", &suc.depth, "",
N_("Create a shallow clone truncated to the "
"specified number of revisions")),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("parallel jobs")),
OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
OPT_END()
};
@@ -475,10 +478,15 @@ static int update_clone(int argc, const char **argv,
const char *prefix)
gitmodules_config();
/* Overlay the parsed .gitmodules file with .git/config */
git_config(submodule_config, NULL);
- run_processes_parallel(1, update_clone_get_next_task,
- update_clone_start_failure,
- update_clone_task_finished,
- &suc);
+
+ if (max_jobs < 0)
+ max_jobs = parallel_submodules();
+
+ run_processes_parallel(max_jobs,
+ update_clone_get_next_task,
+ update_clone_start_failure,
+ update_clone_task_finished,
+ &suc);
/*
* We saved the output and put it out all at once now.
diff --git a/git-submodule.sh b/git-submodule.sh
index a6a82d2..86018ee 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -645,6 +645,14 @@ cmd_update()
--depth=*)
depth=$1
;;
+ -j|--jobs)
+ case "$2" in '') usage ;; esac
+ jobs="--jobs=$2"
+ shift
+ ;;
+ --jobs=*)
+ jobs=$1
+ ;;
--)
shift
break
@@ -671,6 +679,7 @@ cmd_update()
${update:+--update "$update"} \
${reference:+--reference "$reference"} \
${depth:+--depth "$depth"} \
+ ${jobs:+$jobs} \
"$@" || echo "#unmatched"
} | {
err=
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dda3929..7fd5142 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops
module name before recur
test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked
out" actual
)
'
+
+test_expect_success 'submodule update can be run in parallel' '
+ (cd super2 &&
+
[PATCHv15 0/5] Expose submodule parallelism to the user
This build on top of 163b9b1f919c762a4bfb693b3aa05ef1aa627fee
(origin/sb/submodule-parallel-update~3) and replaces the commits
origin/sb/submodule-parallel-update~2..
* Renamed inspect_clone_next_submodule to prepare_to_clone_next_submodule
and reordered the arguments thereof
* Comments for the struct submodule_update_clone which is passed around
* Better handling around LFs.
* Renamed struct child_process *cp to *child
* Print #unmatched in git-submodule.sh instead of the helper
>
> if (pp->update.type == SM_UPDATE_NONE
> || (pp->update.type == SM_UPDATE_UNSPECIFIED
> && sub->update_strategy.type == SM_UPDATE_NONE)) {
>
> What does pp stand for?
I think I took it as parallel_process when starting off from the parallel
processing machinery. I'll rename it (probably to suc as short for struct
submodule_update_clone).
> > + if (pp->recursive_prefix)
> > + displaypath = relative_path(pp->recursive_prefix,
> > + ce->name, &displaypath_sb);
>
> Nit: could use braces.
Why? I would understand a few lines above where we have an if nested in an
if with braces. But here we have a pretty straighforward one statement per case
condition.
> > + sub = submodule_from_path(null_sha1, ce->name);
>
> It's common to call submodule_from_path with null_sha1 as a parameter
> but I have trouble continuing to remember what that means. Maybe
> there should be a separate function that handles that? As a
> side-effect, the name and docstring of that function could explain
> what it means, which I still am not sure about. :)
I'll do that as a followup cleanup patch as it affects more than just the
new code.
>> +OPT_STRING(0, "reference", &pp.reference, "",
>> + N_("Use the local reference repository "
>> + "instead of a full clone")),
> Is this allowed to be relative? If so, what is it relative to?
It is passing on the argument to clone, so I assume the same rules apply as for
git-clone.
Thanks,
Stefan
Stefan Beller (5):
run-command: expose default_{start_failure, task_finished}
run_processes_parallel: add LF when caller is sloppy
git submodule update: have a dedicated helper for cloning
submodule update: expose parallelism to the user
clone: allow an explicit argument for parallel submodule clones
Documentation/git-clone.txt | 6 +-
Documentation/git-submodule.txt | 7 +-
builtin/clone.c | 19 ++-
builtin/submodule--helper.c | 255 +++-
git-submodule.sh| 56 -
run-command.c | 35 --
run-command.h | 19 +++
t/t0061-run-command.sh | 26
t/t7406-submodule-update.sh | 27 +
test-run-command.c | 18 +++
10 files changed, 412 insertions(+), 56 deletions(-)
--
2.7.0.rc0.34.ga06e0b3.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
[PATCHv15 3/5] git submodule update: have a dedicated helper for cloning
This introduces a new helper function in git submodule--helper
which takes care of cloning all submodules, which we want to
parallelize eventually.
Some tests (such as empty URL, update_mode=none) are required in the
helper to make the decision for cloning. These checks have been
moved into the C function as well (no need to repeat them in the
shell script).
Signed-off-by: Stefan Beller
Signed-off-by: Junio C Hamano
---
builtin/submodule--helper.c | 243
git-submodule.sh| 47 +++--
2 files changed, 256 insertions(+), 34 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..85fb702 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,6 +255,248 @@ static int module_clone(int argc, const char **argv,
const char *prefix)
return 0;
}
+struct submodule_update_clone {
+ /* index into 'list', the list of submodules to look into for cloning */
+ int current;
+ struct module_list list;
+ int warn_if_uninitialized : 1;
+ /* update parameter passed via commandline*/
+ struct submodule_update_strategy update;
+ /* configuration parameters which are passed on to the children */
+ int quiet;
+ const char *reference;
+ const char *depth;
+ const char *recursive_prefix;
+ const char *prefix;
+ /* lines to be output */
+ struct string_list projectlines;
+ /* If we want to stop as fast as possible and return an error */
+ int quickstop : 1;
+};
+#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
+ SUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \
+ STRING_LIST_INIT_DUP, 0}
+
+/**
+ * Inspect if 'ce' needs to be cloned. If so, prepare the 'child' to be running
+ * the clone and return non zero.
+ */
+static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
+ struct child_process *child,
+ struct submodule_update_clone *suc,
+ struct strbuf *err)
+{
+ const struct submodule *sub = NULL;
+ struct strbuf displaypath_sb = STRBUF_INIT;
+ struct strbuf sb = STRBUF_INIT;
+ const char *displaypath = NULL;
+ char *url = NULL;
+ int needs_cloning = 0;
+
+ if (ce_stage(ce)) {
+ if (suc->recursive_prefix) {
+ strbuf_addf(err, "Skipping unmerged submodule %s/%s\n",
+ suc->recursive_prefix, ce->name);
+ } else {
+ strbuf_addf(err, "Skipping unmerged submodule %s\n",
+ ce->name);
+ }
+ goto cleanup;
+ }
+
+ sub = submodule_from_path(null_sha1, ce->name);
+
+ if (suc->recursive_prefix)
+ displaypath = relative_path(suc->recursive_prefix,
+ ce->name, &displaypath_sb);
+ else
+ displaypath = ce->name;
+
+ if (suc->update.type == SM_UPDATE_NONE
+ || (suc->update.type == SM_UPDATE_UNSPECIFIED
+ && sub->update_strategy.type == SM_UPDATE_NONE)) {
+ strbuf_addf(err, "Skipping submodule '%s'\n",
+ displaypath);
+ goto cleanup;
+ }
+
+ /*
+* Looking up the url in .git/config.
+* We must not fall back to .gitmodules as we only want
+* to process configured submodules.
+*/
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "submodule.%s.url", sub->name);
+ git_config_get_string(sb.buf, &url);
+ if (!url) {
+ /*
+* Only mention uninitialized submodules when its
+* path have been specified
+*/
+ if (suc->warn_if_uninitialized)
+ strbuf_addf(err, _("Submodule path '%s' not
initialized\n"
+ "Maybe you want to use 'update --init'?\n"),
+ displaypath);
+ goto cleanup;
+ }
+
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s/.git", ce->name);
+ needs_cloning = !file_exists(sb.buf);
+
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%06o %s %d %d\t%s\n", ce->ce_mode,
+ sha1_to_hex(ce->sha1), ce_stage(ce),
+ needs_cloning, ce->name);
+ string_list_append(&suc->projectlines, sb.buf);
+
+ if (!needs_cloning)
+ goto cleanup;
+
+ child->git_cmd = 1;
+ child->no_stdin = 1;
+ child->stdout_to_stderr = 1;
+ child->err = -1;
+ argv_array_push(&child->args, "submodule--helper");
+ argv_array_push(&child->args, "clone");
+ if (suc->quiet)
+ argv_array_push(&child->args, "--quiet");
+ if (s
[PATCHv15 1/5] run-command: expose default_{start_failure, task_finished}
We want to reuse the error reporting facilities in a later patch.
Signed-off-by: Stefan Beller
---
run-command.c | 18 +-
run-command.h | 19 +++
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/run-command.c b/run-command.c
index 51fd72c..d03ecaa 100644
--- a/run-command.c
+++ b/run-command.c
@@ -902,10 +902,10 @@ struct parallel_processes {
struct strbuf buffered_output; /* of finished children */
};
-static int default_start_failure(struct child_process *cp,
-struct strbuf *err,
-void *pp_cb,
-void *pp_task_cb)
+int default_start_failure(struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb)
{
int i;
@@ -916,11 +916,11 @@ static int default_start_failure(struct child_process *cp,
return 0;
}
-static int default_task_finished(int result,
-struct child_process *cp,
-struct strbuf *err,
-void *pp_cb,
-void *pp_task_cb)
+int default_task_finished(int result,
+ struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb)
{
int i;
diff --git a/run-command.h b/run-command.h
index d5a57f9..a054fa6 100644
--- a/run-command.h
+++ b/run-command.h
@@ -164,6 +164,15 @@ typedef int (*start_failure_fn)(struct child_process *cp,
void *pp_task_cb);
/**
+ * If a command fails to start, then print an error message stating the
+ * exact command which failed.
+ */
+int default_start_failure(struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb);
+
+/**
* This callback is called on every child process that finished processing.
*
* You must not write to stdout or stderr in this function. Add your
@@ -184,6 +193,16 @@ typedef int (*task_finished_fn)(int result,
void *pp_task_cb);
/**
+ * If the child process returns with a non zero error code, print
+ * an error message of the exact command which failed.
+ */
+int default_task_finished(int result,
+ struct child_process *cp,
+ struct strbuf *err,
+ void *pp_cb,
+ void *pp_task_cb);
+
+/**
* Runs up to n processes at the same time. Whenever a process can be
* started, the callback get_next_task_fn is called to obtain the data
* required to start another child process.
--
2.7.0.rc0.34.ga06e0b3.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
[PATCHv15 5/5] clone: allow an explicit argument for parallel submodule clones
Just pass it along to "git submodule update", which may pick reasonable
defaults if you don't specify an explicit number.
Signed-off-by: Stefan Beller
Signed-off-by: Junio C Hamano
---
Documentation/git-clone.txt | 6 +-
builtin/clone.c | 19 +--
t/t7406-submodule-update.sh | 15 +++
3 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6bf000d..6db7b6d 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,7 +14,7 @@ SYNOPSIS
[-o ] [-b ] [-u ] [--reference ]
[--dissociate] [--separate-git-dir ]
[--depth ] [--[no-]single-branch]
- [--recursive | --recurse-submodules] [--]
+ [--recursive | --recurse-submodules] [--jobs ] [--]
[]
DESCRIPTION
@@ -221,6 +221,10 @@ objects from the source repository into a pack in the
cloned repository.
The result is Git repository can be separated from working
tree.
+-j ::
+--jobs ::
+ The number of submodules fetched at the same time.
+ Defaults to the `submodule.fetchJobs` option.
::
The (possibly remote) repository to clone from. See the
diff --git a/builtin/clone.c b/builtin/clone.c
index a0b3cd9..b004fb4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -50,6 +50,7 @@ static int option_progress = -1;
static struct string_list option_config;
static struct string_list option_reference;
static int option_dissociate;
+static int max_jobs = -1;
static struct option builtin_clone_options[] = {
OPT__VERBOSITY(&option_verbosity),
@@ -72,6 +73,8 @@ static struct option builtin_clone_options[] = {
N_("initialize submodules in the clone")),
OPT_BOOL(0, "recurse-submodules", &option_recursive,
N_("initialize submodules in the clone")),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("number of submodules cloned in parallel")),
OPT_STRING(0, "template", &option_template, N_("template-directory"),
N_("directory from which templates will be used")),
OPT_STRING_LIST(0, "reference", &option_reference, N_("repo"),
@@ -95,10 +98,6 @@ static struct option builtin_clone_options[] = {
OPT_END()
};
-static const char *argv_submodule[] = {
- "submodule", "update", "--init", "--recursive", NULL
-};
-
static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
{
static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -724,8 +723,16 @@ static int checkout(void)
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
sha1_to_hex(sha1), "1", NULL);
- if (!err && option_recursive)
- err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+ if (!err && option_recursive) {
+ struct argv_array args = ARGV_ARRAY_INIT;
+ argv_array_pushl(&args, "submodule", "update", "--init",
"--recursive", NULL);
+
+ if (max_jobs != -1)
+ argv_array_pushf(&args, "--jobs=%d", max_jobs);
+
+ err = run_command_v_opt(args.argv, RUN_GIT_CMD);
+ argv_array_clear(&args);
+ }
return err;
}
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 7fd5142..090891e 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -786,4 +786,19 @@ test_expect_success 'submodule update can be run in
parallel' '
grep "9 tasks" trace.out
)
'
+
+test_expect_success 'git clone passes the parallel jobs config on to
submodules' '
+ test_when_finished "rm -rf super4" &&
+ GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 .
super4 &&
+ grep "7 tasks" trace.out &&
+ rm -rf super4 &&
+ git config --global submodule.fetchJobs 8 &&
+ GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules . super4 &&
+ grep "8 tasks" trace.out &&
+ rm -rf super4 &&
+ GIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 9 .
super4 &&
+ grep "9 tasks" trace.out &&
+ rm -rf super4
+'
+
test_done
--
2.7.0.rc0.34.ga06e0b3.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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
On Tue, Feb 23, 2016 at 11:51:31AM -0800, Junio C Hamano wrote: > Fengguang Wu writes: > > >> >> I have a mixed feeling about this one, primarily because this was > >> >> already tried quite early in the life of "format-patch" command. > >> >> > >> >> > >> >> http://thread.gmane.org/gmane.comp.version-control.git/9694/focus=9757 > >> >> > >> >> Only the name is different (it was called "applies-to" and named a > >> >> tree object). > >> > > >> > Either commit or tree object will work for us. We can use it in > >> > v2 if you prefer tree object. > >> > >> Sorry, I think you misunderstood. By "only the name is different", I > >> didn't mean to say that the tree object name should be shown as the > >> old proposal did. What I meant but didn't explicitly say, as I > >> thought it was sufficient to point at an old discussion thread, was > >> that this was already tried and rejected. This round uses different > >> name but does essentially the same thing as the old proposal, and I > >> do not think I heard anything new that supports this patch against > >> earlier rejection by Linus. That is what gave me a mixed feeling. > > > > I can understand the rejection by Linus in development process POV. > > > > However we are facing a new situation: in test robot POV, IMHO there > > are values to test exactly the same tree as the patch submitter. > > Otherwise the robot risks > > > > - false negative: failing to apply and test some patches > > - false positive: sending wrong bug reports due to guessed wrong base tree > > I always get negatives and positives confused, so let me think aloud > with an example. Let's say that somebody worked on adding a new > feature based on v4.2 codebase and sent in a patch series. The > series touched files in quiescent part of the system, these files > are identical between v4.2 and the current codebase at v4.5-rc5, and > the series applies cleanly to a "wrong" base tree at the tip of > 'master'. But it turns out that the series uses an old API that was > removed in the meantime. The test robot may say "the result of > applying the series does not even build" and the developer would > complain to you saying "You tested with a wrong version". > > I've already said that I can see the value this approach has for > you. By having the developer state which commit the series was > based on, it will shield you from such a complaint, because you > would not use closer-to-tip 'master' as the base, but instead use > v4.2 codebase for the test. > > As I said, what is unclear to me is what value this apporach gives > to the project. Problem arises when a developer based his work on a maintainer's topic branch. The robot doesn't know that and tests the patch on v4.5-rc5, which may trigger a false error because the patch depends on some changes in that maintainer's topic branch. In that case, the error report will be pure noise. Thanks, Fengguang -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
On Tue, Feb 23, 2016 at 04:31:35PM +0300, Dan Carpenter wrote: > Blergh... You want it machine readable and I want it human readable. I Yeah. It's kind of tasting which may differ among people. I'll leave the judgments to Junio and others, and only add necessary comments to your points. > don't care so much about the cover letter but for the first patch then I > really want something minimal (one line) and human readable. > > base tree/branch: > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > base commit: afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc > base patch-id: a849260a843115dbac4b1a330d44256ee6b16d7b > base patch-subject: Linux 4.4 > base tag: v4.4 The necessary lines for the robot are base commit: base patch-id: or base tree-id: base patch-id: The "base tree-id" will be useful if the submitted patchset is based on a public (maintainer) commit. The "base patch-id" will be useful if the submitted patchset is based on another patchset someone (likely the developer himself) posted to the mailing list. > To me that looks like an unparseable wall of text. My version of that > is: > > Applies-to: afd2ff9b7e1b+ origin > > As a human all I really want to know is the tree to apply this to. If > it doesn't apply then I don't debug it, I just send an automatic note > "This doesn't apply to staging-next. Please redo." > > I think that Applies-to is a better name and also that grepping for > "^base " is less reliable than grepping for ^Applies-to. Grep reliability should be the same, if you use "^base tree-id" and "^base patch-id". If necessary, we can avoid white space by naming the keys base-tree-id and base-patch-id. > I used "origin" because that's the name in Next/Trees. The + means > private patches are applied. That's what we already do in naming the > kernel. If the + matters, then I would include a cover letter. > > I have no idea what a "base patch-id" is so that doesn't work at all. It'll come from this command man git patch-id It'll be useful if the patchset's base commit is a private one -- not in any public maintainer tree, however the developer may have posted it to LKML before. The "base patch-id" can more reliably track different versions of patches than "base patch-subject", and do not have the risk of information leaking in case it's a confidential patch. > Including the tag is just duplicative since we already have the hash. That's right. Just in case it's more human readable. > In my email, I proposed that we list all the other private patches in a > cover letter, but I think you are saying that we only need to know the > most recent private patch? Yes in test robot POV. However it's a general git feature, so I guess there will be more potential use cases and requirements. > Another idea would be to list them newest > to oldest (git log order instead of email order) in the cover letter. > > Btw, I always work against linux-next and Dave M is always getting > annoyed with me for not marking which patches go to net and which go to > net-next. I don't use git format-patch, but I will probably start using > "Applies-to: net" or "Applies-to: net-next". As for now, I see the netdev ML has the convention [PATCH net] [PATCH net-next] to tell Dave the target tree. Thanks, Fengguang -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
Hi Eric, On Tue, Feb 23, 2016 at 01:56:07PM -0600, Eric W. Biederman wrote: > > Fengguag Wu, Xiaolong Ye, have you attempted to use the truncated > sha1 of the file the patch applies to? Git already places a file sha1 > at the top of a patch. See the index line? > > > diff --git a/fs/namespace.c b/fs/namespace.c > > index eccd925c6e82..3c3f8172c734 100644 Yes we've evaluated to make use of that index. The conclusion is, it helps make a better guess, however it's still a guessing work and far from perfect. A simple accounting shows only 1/5 files will be changed between two major kernel releases: wfg /c/linux% git ls-files |wc -l 52915 wfg /c/linux% git diff --name-only v4.3 v4.4|wc -l 10606 That means a huge number candidate base tree IDs matching the given blob IDs. > > --- a/fs/namespace.c > > +++ b/fs/namespace.c > > As I understand it you are aiming for making a good guess what the patch > or patches apply to, having a set of file hashes looks like it would > give you that. > > All it should take is to iterate over a patchset and for each file in > the patchset capture the first file hash. Then in the smallish set of > maintainer trees see if that set of file hashes matches any of their > recent commits. You should be able to prune the set of possible > maintainer trees even more by looking at the mailling list or lists > the patch was submitted to. We actually start with the above thinking half year ago. Yes it'll help narrow down the list of candidate maintainer trees. And the chance will be increased if the patchset modifies multiple files, and the fact some files are modified more frequently than the others. However it's still fundamentally a guess work. The best choice is to ask for explicit "base tree ID". > Before we talk about adding anything more I think we need a clear > picture of what you have tried with what already exists. A decade ago > part of the problem was that not everyone used git. At best it will > take a little while before everyone upgrades to a version of git diff > containing your changes, and if possibly even longer if they have to > start specifying an additional option when a diff is generated. That's a good concern. It may take year long delay before reaching reasonable population of the new feature. To speedup the process, we could advocate the new git option in 0day robot's error reports. Since we catch errors in ~10 LKML patches each day, within months most kernel developers should get the tips on how to set it up and enable the feature by default. Thanks, Fengguang -- 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 v3 3/3] t3034: test deprecated interface
From: Felipe Gonçalves Assis --find-renames= and --rename-threshold= should be aliases. Signed-off-by: Felipe Gonçalves Assis --- t/t3034-merge-recursive-rename-options.sh | 46 +++ 1 file changed, 46 insertions(+) diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh index 2479910..b9c4028 100755 --- a/t/t3034-merge-recursive-rename-options.sh +++ b/t/t3034-merge-recursive-rename-options.sh @@ -263,4 +263,50 @@ test_expect_success '--find-renames rejects non-numbers' ' git diff --quiet --cached ' +test_expect_success 'rename-threshold= is a synonym for find-renames=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold=$th0 $tail && + check_threshold_0 +' + +test_expect_success 'last wins in --no-renames --rename-threshold=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --no-renames --rename-threshold=$th0 $tail && + check_threshold_0 +' + +test_expect_success 'last wins in --rename-threshold= --no-renames' ' + git read-tree --reset -u HEAD && + git merge-recursive --rename-threshold=$th0 --no-renames $tail && + check_no_renames +' + +test_expect_success '--rename-threshold= rejects negative argument' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold=-25 \ + HEAD -- HEAD HEAD && + git diff --quiet --cached +' + +test_expect_success '--rename-threshold= rejects non-numbers' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold=0xf \ + HEAD -- HEAD HEAD && + git diff --quiet --cached +' + +test_expect_success 'last wins in --rename-threshold= --find-renames=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --rename-threshold=$th0 --find-renames=$th2 $tail && + check_threshold_2 +' + +test_expect_success 'last wins in --find-renames= --rename-threshold=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --find-renames=$th2 --rename-threshold=$th0 $tail && + check_threshold_0 +' + test_done -- 2.7.1.492.gd821b20 -- 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 v3 2/3] t3034: test option to disable renames
From: Felipe Gonçalves Assis
Signed-off-by: Felipe Gonçalves Assis
---
t/t3034-merge-recursive-rename-options.sh | 28
1 file changed, 28 insertions(+)
diff --git a/t/t3034-merge-recursive-rename-options.sh
b/t/t3034-merge-recursive-rename-options.sh
index 51c2f87..2479910 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -83,6 +83,14 @@ check_exact_renames () {
rename_detected 3
}
+check_no_renames () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_undetected 1 &&
+ rename_undetected 2 &&
+ rename_undetected 3
+}
+
test_expect_success 'setup repo' '
cat <<-\EOF >3-old &&
33a
@@ -195,6 +203,12 @@ test_expect_success 'rename threshold is truncated' '
check_exact_renames
'
+test_expect_success 'disabled rename detection' '
+ git read-tree --reset -u HEAD &&
+ git merge-recursive --no-renames $tail &&
+ check_no_renames
+'
+
test_expect_success 'last wins in --find-renames= --find-renames=' '
git read-tree --reset -u HEAD &&
test_must_fail git merge-recursive \
@@ -209,6 +223,18 @@ test_expect_success '--find-renames resets threshold' '
$check_50
'
+test_expect_success 'last wins in --no-renames --find-renames' '
+ git read-tree --reset -u HEAD &&
+ test_must_fail git merge-recursive --no-renames --find-renames $tail &&
+ $check_50
+'
+
+test_expect_success 'last wins in --find-renames --no-renames' '
+ git read-tree --reset -u HEAD &&
+ git merge-recursive --find-renames --no-renames $tail &&
+ check_no_renames
+'
+
test_expect_success 'assumption for further tests: trivial merge succeeds' '
git read-tree --reset -u HEAD &&
git merge-recursive HEAD -- HEAD HEAD &&
@@ -218,6 +244,8 @@ test_expect_success 'assumption for further tests: trivial
merge succeeds' '
git merge-recursive --find-renames=$th2 HEAD -- HEAD HEAD &&
git diff --quiet --cached &&
git merge-recursive --find-renames=100% HEAD -- HEAD HEAD &&
+ git diff --quiet --cached &&
+ git merge-recursive --no-renames HEAD -- HEAD HEAD &&
git diff --quiet --cached
'
--
2.7.1.492.gd821b20
--
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 v3 0/3] Tests for merge-recursive rename-options
Just a quick update incorporating Eric's latest comments.
Still based on
c443d39 (merge-recursive: find-renames resets threshold, 2016-02-21).
Felipe Gonçalves Assis (3):
t3034: add rename threshold tests
t3034: test option to disable renames
t3034: test deprecated interface
...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-
t/t3034-merge-recursive-rename-options.sh | 312 +
2 files changed, 313 insertions(+), 1 deletion(-)
rename t/{t3032-merge-recursive-options.sh =>
t3032-merge-recursive-space-options.sh} (99%)
create mode 100755 t/t3034-merge-recursive-rename-options.sh
--
2.7.1.492.gd821b20
--
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 v3 1/3] t3034: add rename threshold tests
From: Felipe Gonçalves Assis
10ae752 (merge-recursive: option to specify rename threshold,
2010-09-27) introduced this feature but did not include any tests.
The tests use the new option --find-renames, which replaces the then
introduced and now deprecated option --rename-threshold.
Also update name and description of t3032 for consistency:
"merge-recursive options" -> "merge-recursive space options"
Signed-off-by: Felipe Gonçalves Assis
---
...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-
t/t3034-merge-recursive-rename-options.sh | 238 +
2 files changed, 239 insertions(+), 1 deletion(-)
rename t/{t3032-merge-recursive-options.sh =>
t3032-merge-recursive-space-options.sh} (99%)
create mode 100755 t/t3034-merge-recursive-rename-options.sh
diff --git a/t/t3032-merge-recursive-options.sh
b/t/t3032-merge-recursive-space-options.sh
similarity index 99%
rename from t/t3032-merge-recursive-options.sh
rename to t/t3032-merge-recursive-space-options.sh
index 4029c9c..b56180e 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-space-options.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='merge-recursive options
+test_description='merge-recursive space options
* [master] Clarify
! [remote] Remove cruft
diff --git a/t/t3034-merge-recursive-rename-options.sh
b/t/t3034-merge-recursive-rename-options.sh
new file mode 100755
index 000..51c2f87
--- /dev/null
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -0,0 +1,238 @@
+#!/bin/sh
+
+test_description='merge-recursive rename options
+
+Test rename detection by examining rename/delete conflicts.
+
+* (HEAD -> rename) rename
+| * (master) delete
+|/
+* base
+
+git diff --name-status base master
+D 0-old
+D 1-old
+D 2-old
+D 3-old
+
+git diff --name-status -M01 base rename
+R0250-old 0-new
+R0501-old 1-new
+R0752-old 2-new
+R1003-old 3-new
+
+Actual similarity indices are parsed from diff output. We rely on the fact that
+they are rounded down (see, e.g., Documentation/diff-generate-patch.txt, which
+mentions this in a different context).
+'
+
+. ./test-lib.sh
+
+get_expected_stages () {
+ git checkout rename -- $1-new &&
+ git ls-files --stage $1-new >expected-stages-undetected-$1 &&
+ sed "s/ 0 / 2 /" expected-stages-detected-$1 &&
+ git read-tree -u --reset HEAD
+}
+
+rename_detected () {
+ git ls-files --stage $1-old $1-new >stages-actual-$1 &&
+ test_cmp expected-stages-detected-$1 stages-actual-$1
+}
+
+rename_undetected () {
+ git ls-files --stage $1-old $1-new >stages-actual-$1 &&
+ test_cmp expected-stages-undetected-$1 stages-actual-$1
+}
+
+check_common () {
+ git ls-files --stage >stages-actual &&
+ test_line_count = 4 stages-actual
+}
+
+check_threshold_0 () {
+ check_common &&
+ rename_detected 0 &&
+ rename_detected 1 &&
+ rename_detected 2 &&
+ rename_detected 3
+}
+
+check_threshold_1 () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_detected 1 &&
+ rename_detected 2 &&
+ rename_detected 3
+}
+
+check_threshold_2 () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_undetected 1 &&
+ rename_detected 2 &&
+ rename_detected 3
+}
+
+check_exact_renames () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_undetected 1 &&
+ rename_undetected 2 &&
+ rename_detected 3
+}
+
+test_expect_success 'setup repo' '
+ cat <<-\EOF >3-old &&
+ 33a
+ 33b
+ 33c
+ 33d
+ EOF
+ sed s/33/22/ <3-old >2-old &&
+ sed s/33/11/ <3-old >1-old &&
+ sed s/33/00/ <3-old >0-old &&
+ git add [0-3]-old &&
+ git commit -m base &&
+ git rm [0-3]-old &&
+ git commit -m delete &&
+ git checkout -b rename HEAD^ &&
+ cp 3-old 3-new &&
+ sed 1,1s/./x/ <2-old >2-new &&
+ sed 1,2s/./x/ <1-old >1-new &&
+ sed 1,3s/./x/ <0-old >0-new &&
+ git add [0-3]-new &&
+ git rm [0-3]-old &&
+ git commit -m rename &&
+ get_expected_stages 0 &&
+ get_expected_stages 1 &&
+ get_expected_stages 2 &&
+ get_expected_stages 3 &&
+ check_50="false" &&
+ tail="HEAD^ -- HEAD master"
+'
+
+test_expect_success 'setup thresholds' '
+ git diff --name-status -M01 HEAD^ HEAD >diff-output &&
+ test_debug "cat diff-output" &&
+ test_line_count = 4 diff-output &&
+ grep "R[0-9][0-9][0-9] \([0-3]\)-old \1-new" diff-output \
+ >grep-output &&
+ test_cmp diff-output grep-output &&
+ th0=$(sed -n "s/R\(...\)0-old 0-new/\1/p" diff-output-0 &&
+ git diff --name-status -M$th1 --diff-filter=R HEAD^ HEAD \
+ >diff-output-1 &&
+ git diff --name-status -M$th2 --diff-filter=R HEAD^ HEAD \
+ >diff-output-2 &&
+ git diff --name-status -M100% --
Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
On 02/23/2016 01:49 PM, Eric W. Biederman wrote: > > So I could really respect a patch header line that said: > tree abcdef0123456789...0123456789abcdef > > Where the numbers where the truncated tree hash before and after a patch > was applied. That would seem to give a little bit of extra sanity > checking in the application of git diffs as well. > I would rather have the untruncated base tree ID. The truncated file IDs already provide the verification component, and it is *way* cheaper to search for an untruncated ID. -hpa -- 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 "thin" submodule clone to feed "describe"
On Tue, Feb 23, 2016 at 4:33 PM, Junio C Hamano wrote: > No, I do not think so. Thanks. I will probably setup a pre-commit hook at the top level project to update a submodule metadata file. Not the prettiest but... :-) m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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] t3034: add rename threshold tests
On 23 February 2016 at 21:50, Eric Sunshine wrote:
> On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis
> wrote:
>> 10ae752 (merge-recursive: option to specify rename threshold,
>> 2010-09-27) introduced this feature but did not include any tests.
>>
>> The tests use the new option --find-renames, which replaces the then
>> introduced and now deprecated option --rename-threshold.
>>
>> Also update name and description of t3032 for consistency:
>> "merge-recursive options" -> "merge-recursive space options"
>
> A few superficial comments below...
>
>> + cat <<-\EOF >3-old &&
>> + 33a
>> + 33b
>> + 33c
>> + 33d
>> + EOF
>> + sed s/33/22/ <3-old >2-old &&
>> + sed s/33/11/ <3-old >1-old &&
>> + sed s/33/00/ <3-old >0-old &&
>> + git add [0-3]-old &&
>> + git commit -m base &&
>> + git rm [0-3]-old &&
>> + git commit -m delete &&
>> + git checkout -b rename HEAD^ &&
>> + cp 3-old 3-new &&
>> + sed 1,1s/./x/ <2-old >2-new &&
>> + sed 1,2s/./x/ <1-old >1-new &&
>> + sed 1,3s/./x/ <0-old >0-new &&
>> + git add [0-3]-new &&
>> + git rm [0-3]-old &&
>> + git commit -m rename &&
>> + get_expected_stages 0 &&
>> + get_expected_stages 1 &&
>> + get_expected_stages 2 &&
>> + get_expected_stages 3 &&
>> + check_50="false" &&
>
> Why isn't this assignment done in setup 2/2 where all the other
> assignments to 'check_50' are done?
>
Oh, it might be a minor thing, but I would like to make tests using
check_50 fail if setup 2/2 is skipped, instead of succeeding without
any actual checks.
>> + th0=$(sed -n "s/R\(...\)0-old 0-new/\1/p" > + th1=$(sed -n "s/R\(...\)1-old 1-new/\1/p" > + th2=$(sed -n "s/R\(...\)2-old 2-new/\1/p" > + th3=$(sed -n "s/R\(...\)3-old 3-new/\1/p" > + test "$th0" -lt "$th1" &&
>> + test "$th1" -lt "$th2" &&
>> + test "$th2" -lt "$th3" &&
>> + test "$th3" = 100 &&
>
> It's very slightly odd to see '=' rather than '-eq' among all these
> other algebraic operators ('-lt', '-le'), but not so odd that I'd
> mention it (unless I just did), and not necessarily worth changing.
>
Here I favoured the stricter test. If the string is not exactly "100",
then a lot should be reviewed.
Once more, thanks for the review. Your other comments should be
included in the next version of the patches.
--
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 History Rewriting in a public repository - capability to remove one or more commits from a public repository
Thanks Ilya and Stefan for your detailed responses. In general I am trying to avoid having to work with patch sets/queues. This may be acceptable as a replacement for the “testing-stage” repo. But we don’t want to have the developer deal with patches or patch queues in his workflow. 1. The developer will pulling from the “mainline” repository to do development. 2. They will be committing and pushing to the “testing-stage” repository or branch as you suggest. 3. To do (2) above they will need to sync to “testing-stage” repository or branch, which might pull in 20-30 commits that are already in that branch or repository. 4. He might have to resolve some conflicts before he commits it into that “testing-stage” repository or branch. 5. During the window of (4) above, Auto bisection testing process happening on the “testing-stage” repository or branch might have kicked out 3 commits from that repository 6. The kicking out of the 3 commits in (5) above, would require the developer sync to the “testing-stage” repository or branch again before pushing it. We want the developer workflow at (3) and (6) above, to not have to deal with patch queues and stuff. For him to be able to do the “sync" to the “testing-stage” repository branch in (3) and (6) above, allow him to merge the the commits he has already made in his local repository with the rewritten history of commits coming from the “testing-stage” repository or branch. What I am hearing you suggest as the way forward is the following 1. Removing bad patches in “testin-stage” repository or branch 1. Use may be branch instead of a separate repository for “testing-stage”. 2. Pull a workspace repo for removing the bad commit and related commits in that workspace by rewriting history in it. 3. Do git push -n --force-with-lease remote branch. If that fails because additional commits have come bring in the additional changes and re-attempt the push. 4. When 3 completes successfully the history would have been rewritten into the “testing-stage” 2. Developer has made commits to history space. 3. Developer syncs to “testing-stage” repo or branch, the first time, in preparation for pushing his changes to that branch or repo. All should go well. Since his repo has not seen history from that branch or repo till that time. 3. Takes time resolve issues.(While the history has changed in the “testing-stage” repo or branch) 4. He syncs again, which is where we will have a problem. “git pull” needs to recognize that history has changed and fail. Will it do that? 5. Drawing from Stefan Beller’s response, it looks the developer has to either manually or through git extension 1. Recognize that the history has changed, during a git pull failure. 2. Use "git cherrypick” get his own commits in his workspace have them recommited over the new history that is being pulled from “testing-stage” repo or branch. Do something along the lines of http://think-like-a-git.net/sections/rebase-from-the-ground-up/using-git-ch erry-pick-to-simulate-git-rebase.html to achieve that. Do I have the workflow right? Thanks, Sarvi - Occam's Razor Rules On 2/23/16, 2:15 PM, "Ilya Terentyev" wrote: >Hi Saravanan, > >Changes that rewrite history, including (but not limited to) deleted >commits, >can be pushed with the --force or --force-with-lease options, like this: > > $ git push --force remote branch > >--force pushes your changes unconditionally, which may overwrite changes >that someone else pushed between the moment you cloned the repo and pushed >your own. --force-with-lease will check for others' pushes, so you can >use it >in a dry run (without actually changing anything in the remote repository) >like this: > > $ git push -n --force-with-lease remote branch > >If someone else (like another developer with access to "testing-stage") >pushes anything before your attempt to push, you will receive a message >like: > > $ git push -n --force-with-lease remote branch > ! [rejected]branch -> branch (stale info) > error: failed to push some refs to remote > >Generally speaking, your idea is, probably, better implemented with >patches >or pull requests: > > 1) Your developers rewrite their local history as they wish > 2) They generate patches from their commits (with git format-patch, > for instance) > 3) Send those patches to "testing-stage" > 4) Apply them to staging area (without committing) > 5) Run required checks > 6) If checks don't pass, discard those changes > 7) If checks pass, commit those patches > 8) Push committed changes to "mainline" > >But in any case, you should better consider using feature branches for >that. > >Best regards, >Ilya T. > > >On 02/24/2016 12:30 AM, Saravanan Shanmugham (sarvi) wrote: >> Hi Git Leads, >>I am looking for git capability/way to be able to remove commits >> from a public repository. >> >> Background: >> We are looking for a multi-stage commit process where commits get pushed >>
Re: [PATCH 1/3] t3034: add rename threshold tests
On Tue, Feb 23, 2016 at 7:50 PM, Eric Sunshine wrote: > On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis > wrote: >> + if [ 50 -le "$th0" ]; then >> + check_50=check_threshold_0 >> + elif [ 50 -le "$th1" ]; then >> + check_50=check_threshold_1 >> + elif [ 50 -le "$th2" ]; then >> + check_50=check_threshold_2 >> + fi && Oh, I totally forgot to mention a few style issues: 1. use 'test' rather than '[' 2. place 'then' on its own line 3. drop semicolon if test 50 -le "$th0" then ... elif test ... then ... -- 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] t3034: add rename threshold tests
On Tue, Feb 23, 2016 at 6:48 PM, Felipe Gonçalves Assis
wrote:
> 10ae752 (merge-recursive: option to specify rename threshold,
> 2010-09-27) introduced this feature but did not include any tests.
>
> The tests use the new option --find-renames, which replaces the then
> introduced and now deprecated option --rename-threshold.
>
> Also update name and description of t3032 for consistency:
> "merge-recursive options" -> "merge-recursive space options"
A few superficial comments below...
> Signed-off-by: Felipe Gonçalves Assis
> ---
> diff --git a/t/t3034-merge-recursive-rename-options.sh
> b/t/t3034-merge-recursive-rename-options.sh
> @@ -0,0 +1,235 @@
> +test_expect_success 'setup 1/2: basic setup' '
If someone ever adds a third setup test, it's likely that that person
will forget to update this title to say "1/3" (ditto regarding "setup
2/2" below). Perhaps just call this "setup repo" and 2/2 "setup
thresholds" or something.
> + cat <<-\EOF >3-old &&
> + 33a
> + 33b
> + 33c
> + 33d
> + EOF
> + sed s/33/22/ <3-old >2-old &&
> + sed s/33/11/ <3-old >1-old &&
> + sed s/33/00/ <3-old >0-old &&
> + git add [0-3]-old &&
> + git commit -m base &&
> + git rm [0-3]-old &&
> + git commit -m delete &&
> + git checkout -b rename HEAD^ &&
> + cp 3-old 3-new &&
> + sed 1,1s/./x/ <2-old >2-new &&
> + sed 1,2s/./x/ <1-old >1-new &&
> + sed 1,3s/./x/ <0-old >0-new &&
> + git add [0-3]-new &&
> + git rm [0-3]-old &&
> + git commit -m rename &&
> + get_expected_stages 0 &&
> + get_expected_stages 1 &&
> + get_expected_stages 2 &&
> + get_expected_stages 3 &&
> + check_50="false" &&
Why isn't this assignment done in setup 2/2 where all the other
assignments to 'check_50' are done?
> + tail="HEAD^ -- HEAD master"
> +'
> +
> +test_expect_success 'setup 2/2: threshold array' '
> + git diff --name-status -M01 HEAD^ HEAD >diff-output &&
> + test_debug "cat diff-output" &&
> + test_line_count = 4 diff-output &&
> + grep "R[0-9]\{3\} \([0-3]\)-old \1-new" diff-output \
> + >grep-output &&
I'd be slightly concerned about the use of \{3\} with older grep's
such as on Solaris, and would just code it as "[0-9][0-9][0-9]" and be
done with it, but perhaps it's not worth worrying about until someone
complains.
> + test_cmp diff-output grep-output &&
So, this is asserting that you only see "renames" and nothing else. Okay.
> + th0=$(sed -n "s/R\(...\)0-old 0-new/\1/p" + th1=$(sed -n "s/R\(...\)1-old 1-new/\1/p" + th2=$(sed -n "s/R\(...\)2-old 2-new/\1/p" + th3=$(sed -n "s/R\(...\)3-old 3-new/\1/p" + test "$th0" -lt "$th1" &&
> + test "$th1" -lt "$th2" &&
> + test "$th2" -lt "$th3" &&
> + test "$th3" = 100 &&
It's very slightly odd to see '=' rather than '-eq' among all these
other algebraic operators ('-lt', '-le'), but not so odd that I'd
mention it (unless I just did), and not necessarily worth changing.
> + if [ 50 -le "$th0" ]; then
> + check_50=check_threshold_0
> + elif [ 50 -le "$th1" ]; then
> + check_50=check_threshold_1
> + elif [ 50 -le "$th2" ]; then
> + check_50=check_threshold_2
> + fi &&
> + th0="$th0%" &&
> + th1="$th1%" &&
> + th2="$th2%" &&
> + th3="$th3%"
> +'
> +
> +test_expect_success 'assumption for tests: rename detection with diff' '
> + git diff --name-status -M$th0 --diff-filter=R HEAD^ HEAD \
> + >diff-output-0 &&
> + git diff --name-status -M$th1 --diff-filter=R HEAD^ HEAD \
> + >diff-output-1 &&
> + git diff --name-status -M$th2 --diff-filter=R HEAD^ HEAD \
> + >diff-output-2 &&
> + git diff --name-status -M100% --diff-filter=R HEAD^ HEAD \
> + >diff-output-3 &&
> + test_line_count = 4 diff-output-0 &&
> + test_line_count = 3 diff-output-1 &&
> + test_line_count = 2 diff-output-2 &&
> + test_line_count = 1 diff-output-3
> +'
> +
> +test_expect_success 'default similarity threshold is 50%' '
> + git read-tree --reset -u HEAD &&
> + test_must_fail git merge-recursive $tail &&
> + $check_50
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] t3034: test deprecated interface
From: Felipe Gonçalves Assis --find-renames= and --rename-threshold= should be aliases. Signed-off-by: Felipe Gonçalves Assis --- t/t3034-merge-recursive-rename-options.sh | 46 +++ 1 file changed, 46 insertions(+) diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh index c07a4cb..96d8767 100755 --- a/t/t3034-merge-recursive-rename-options.sh +++ b/t/t3034-merge-recursive-rename-options.sh @@ -260,4 +260,50 @@ test_expect_success '--find-renames rejects non-numbers' ' git diff --quiet --cached ' +test_expect_success 'rename-threshold= is a synonym for find-renames=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold=$th0 $tail && + check_threshold_0 +' + +test_expect_success 'last wins in --no-renames --rename-threshold=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --no-renames --rename-threshold=$th0 $tail && + check_threshold_0 +' + +test_expect_success 'last wins in --rename-threshold= --no-renames' ' + git read-tree --reset -u HEAD && + git merge-recursive --rename-threshold=$th0 --no-renames $tail && + check_no_renames +' + +test_expect_success '--rename-threshold= rejects negative argument' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold=-25 \ + HEAD -- HEAD HEAD && + git diff --quiet --cached +' + +test_expect_success '--rename-threshold= rejects non-numbers' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive --rename-threshold=0xf \ + HEAD -- HEAD HEAD && + git diff --quiet --cached +' + +test_expect_success 'last wins in --rename-threshold= --find-renames=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --rename-threshold=$th0 --find-renames=$th2 $tail && + check_threshold_2 +' + +test_expect_success 'last wins in --find-renames= --rename-threshold=' ' + git read-tree --reset -u HEAD && + test_must_fail git merge-recursive \ + --find-renames=$th2 --rename-threshold=$th0 $tail && + check_threshold_0 +' + test_done -- 2.7.1.492.gd821b20 -- 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/3] t3034: add rename threshold tests
From: Felipe Gonçalves Assis
10ae752 (merge-recursive: option to specify rename threshold,
2010-09-27) introduced this feature but did not include any tests.
The tests use the new option --find-renames, which replaces the then
introduced and now deprecated option --rename-threshold.
Also update name and description of t3032 for consistency:
"merge-recursive options" -> "merge-recursive space options"
Signed-off-by: Felipe Gonçalves Assis
---
...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-
t/t3034-merge-recursive-rename-options.sh | 235 +
2 files changed, 236 insertions(+), 1 deletion(-)
rename t/{t3032-merge-recursive-options.sh =>
t3032-merge-recursive-space-options.sh} (99%)
create mode 100755 t/t3034-merge-recursive-rename-options.sh
diff --git a/t/t3032-merge-recursive-options.sh
b/t/t3032-merge-recursive-space-options.sh
similarity index 99%
rename from t/t3032-merge-recursive-options.sh
rename to t/t3032-merge-recursive-space-options.sh
index 4029c9c..b56180e 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-space-options.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='merge-recursive options
+test_description='merge-recursive space options
* [master] Clarify
! [remote] Remove cruft
diff --git a/t/t3034-merge-recursive-rename-options.sh
b/t/t3034-merge-recursive-rename-options.sh
new file mode 100755
index 000..fbec68c
--- /dev/null
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -0,0 +1,235 @@
+#!/bin/sh
+
+test_description='merge-recursive rename options
+
+Test rename detection by examining rename/delete conflicts.
+
+* (HEAD -> rename) rename
+| * (master) delete
+|/
+* base
+
+git diff --name-status base master
+D 0-old
+D 1-old
+D 2-old
+D 3-old
+
+git diff --name-status -M01 base rename
+R0250-old 0-new
+R0501-old 1-new
+R0752-old 2-new
+R1003-old 3-new
+
+Actual similarity indices are parsed from diff output. We rely on the fact that
+they are rounded down (see, e.g., Documentation/diff-generate-patch.txt, which
+mentions this in a different context).
+'
+
+. ./test-lib.sh
+
+get_expected_stages () {
+ git checkout rename -- $1-new &&
+ git ls-files --stage $1-new >expected-stages-undetected-$1 &&
+ sed "s/ 0 / 2 /" expected-stages-detected-$1 &&
+ git read-tree -u --reset HEAD
+}
+
+rename_detected () {
+ git ls-files --stage $1-old $1-new >stages-actual-$1 &&
+ test_cmp expected-stages-detected-$1 stages-actual-$1
+}
+
+rename_undetected () {
+ git ls-files --stage $1-old $1-new >stages-actual-$1 &&
+ test_cmp expected-stages-undetected-$1 stages-actual-$1
+}
+
+check_common () {
+ git ls-files --stage >stages-actual &&
+ test_line_count = 4 stages-actual
+}
+
+check_threshold_0 () {
+ check_common &&
+ rename_detected 0 &&
+ rename_detected 1 &&
+ rename_detected 2 &&
+ rename_detected 3
+}
+
+check_threshold_1 () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_detected 1 &&
+ rename_detected 2 &&
+ rename_detected 3
+}
+
+check_threshold_2 () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_undetected 1 &&
+ rename_detected 2 &&
+ rename_detected 3
+}
+
+check_exact_renames () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_undetected 1 &&
+ rename_undetected 2 &&
+ rename_detected 3
+}
+
+test_expect_success 'setup 1/2: basic setup' '
+ cat <<-\EOF >3-old &&
+ 33a
+ 33b
+ 33c
+ 33d
+ EOF
+ sed s/33/22/ <3-old >2-old &&
+ sed s/33/11/ <3-old >1-old &&
+ sed s/33/00/ <3-old >0-old &&
+ git add [0-3]-old &&
+ git commit -m base &&
+ git rm [0-3]-old &&
+ git commit -m delete &&
+ git checkout -b rename HEAD^ &&
+ cp 3-old 3-new &&
+ sed 1,1s/./x/ <2-old >2-new &&
+ sed 1,2s/./x/ <1-old >1-new &&
+ sed 1,3s/./x/ <0-old >0-new &&
+ git add [0-3]-new &&
+ git rm [0-3]-old &&
+ git commit -m rename &&
+ get_expected_stages 0 &&
+ get_expected_stages 1 &&
+ get_expected_stages 2 &&
+ get_expected_stages 3 &&
+ check_50="false" &&
+ tail="HEAD^ -- HEAD master"
+'
+
+test_expect_success 'setup 2/2: threshold array' '
+ git diff --name-status -M01 HEAD^ HEAD >diff-output &&
+ test_debug "cat diff-output" &&
+ test_line_count = 4 diff-output &&
+ grep "R[0-9]\{3\} \([0-3]\)-old \1-new" diff-output \
+ >grep-output &&
+ test_cmp diff-output grep-output &&
+ th0=$(sed -n "s/R\(...\)0-old 0-new/\1/p" diff-output-0 &&
+ git diff --name-status -M$th1 --diff-filter=R HEAD^ HEAD \
+ >diff-output-1 &&
+ git diff --name-status -M$th2 --diff-filter=R HEAD^ HEAD \
+ >diff-output-2 &&
+ git diff -
[PATCH 2/3] t3034: test option to disable renames
From: Felipe Gonçalves Assis
Signed-off-by: Felipe Gonçalves Assis
---
t/t3034-merge-recursive-rename-options.sh | 28
1 file changed, 28 insertions(+)
diff --git a/t/t3034-merge-recursive-rename-options.sh
b/t/t3034-merge-recursive-rename-options.sh
index fbec68c..c07a4cb 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -83,6 +83,14 @@ check_exact_renames () {
rename_detected 3
}
+check_no_renames () {
+ check_common &&
+ rename_undetected 0 &&
+ rename_undetected 1 &&
+ rename_undetected 2 &&
+ rename_undetected 3
+}
+
test_expect_success 'setup 1/2: basic setup' '
cat <<-\EOF >3-old &&
33a
@@ -192,6 +200,12 @@ test_expect_success 'rename threshold is truncated' '
check_exact_renames
'
+test_expect_success 'disabled rename detection' '
+ git read-tree --reset -u HEAD &&
+ git merge-recursive --no-renames $tail &&
+ check_no_renames
+'
+
test_expect_success 'last wins in --find-renames= --find-renames=' '
git read-tree --reset -u HEAD &&
test_must_fail git merge-recursive \
@@ -206,6 +220,18 @@ test_expect_success '--find-renames resets threshold' '
$check_50
'
+test_expect_success 'last wins in --no-renames --find-renames' '
+ git read-tree --reset -u HEAD &&
+ test_must_fail git merge-recursive --no-renames --find-renames $tail &&
+ $check_50
+'
+
+test_expect_success 'last wins in --find-renames --no-renames' '
+ git read-tree --reset -u HEAD &&
+ git merge-recursive --find-renames --no-renames $tail &&
+ check_no_renames
+'
+
test_expect_success 'assumption for further tests: trivial merge succeeds' '
git read-tree --reset -u HEAD &&
git merge-recursive HEAD -- HEAD HEAD &&
@@ -215,6 +241,8 @@ test_expect_success 'assumption for further tests: trivial
merge succeeds' '
git merge-recursive --find-renames=$th2 HEAD -- HEAD HEAD &&
git diff --quiet --cached &&
git merge-recursive --find-renames=100% HEAD -- HEAD HEAD &&
+ git diff --quiet --cached &&
+ git merge-recursive --no-renames HEAD -- HEAD HEAD &&
git diff --quiet --cached
'
--
2.7.1.492.gd821b20
--
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] Tests for merge-recursive rename options
Get rid of blatant bash-isms in favour of simple and portable constructions.
Felipe Gonçalves Assis (3):
t3034: add rename threshold tests
t3034: test option to disable renames
t3034: test deprecated interface
...s.sh => t3032-merge-recursive-space-options.sh} | 2 +-
t/t3034-merge-recursive-rename-options.sh | 309 +
2 files changed, 310 insertions(+), 1 deletion(-)
rename t/{t3032-merge-recursive-options.sh =>
t3032-merge-recursive-space-options.sh} (99%)
create mode 100755 t/t3034-merge-recursive-rename-options.sh
--
2.7.1.492.gd821b20
--
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: [PATCHv14 0/7] Expose submodule parallelism to the user
Stefan Beller writes: > On Tue, Feb 23, 2016 at 3:33 PM, Junio C Hamano wrote: >> Junio C Hamano writes: >> >>> Looks good. I didn't notice these unnecessary blank lines while >>> reading the previous rounds, and it is good to see them go. >>> >>> Let's wait for a few days and merge them to 'next'. David's ref >>> backend topic textually depends on this, and we'd better give it a >>> solid foundation to build on soon. >> >> So... it seems that you and Jonathan had a few rounds of back and >> forth on 5/7 that didn't sound like it saw a satisfactory end. Will >> we see a reroll to address the issues raised? > > Yes, I was about to send one out, but then I wanted to fix the last > of Jonathans small nits, which resulted in some heavy refactoring. > > I'll reroll, but I suspect it won't make it out today in time for integration. Ah, that's OK, I've already pushed today's out. Take time to read it over one more time, perhaps? 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: [PATCHv14 0/7] Expose submodule parallelism to the user
On Tue, Feb 23, 2016 at 3:33 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Looks good. I didn't notice these unnecessary blank lines while >> reading the previous rounds, and it is good to see them go. >> >> Let's wait for a few days and merge them to 'next'. David's ref >> backend topic textually depends on this, and we'd better give it a >> solid foundation to build on soon. > > So... it seems that you and Jonathan had a few rounds of back and > forth on 5/7 that didn't sound like it saw a satisfactory end. Will > we see a reroll to address the issues raised? Yes, I was about to send one out, but then I wanted to fix the last of Jonathans small nits, which resulted in some heavy refactoring. I'll reroll, but I suspect it won't make it out today in time for integration. Thanks, Stefan > > 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: [PATCHv14 0/7] Expose submodule parallelism to the user
Junio C Hamano writes: > Looks good. I didn't notice these unnecessary blank lines while > reading the previous rounds, and it is good to see them go. > > Let's wait for a few days and merge them to 'next'. David's ref > backend topic textually depends on this, and we'd better give it a > solid foundation to build on soon. So... it seems that you and Jonathan had a few rounds of back and forth on 5/7 that didn't sound like it saw a satisfactory end. Will we see a reroll to address the issues raised? 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] git-completion.bash: always swallow error output of for-each-ref
Junio C Hamano writes: > Jeff King writes: > >> On Fri, Feb 12, 2016 at 10:40:48PM +0100, SZEDER Gábor wrote: >> >>> * It would swallow even those errors that we are interested in, >>>e.g. (note the missing quotes around $foo): >>> [...] >>> * I often find myself tracing/debugging the completion script >>>through stderr by scattering >>> >>> echo >&2 "foo: '$foo'" >> >> One alternative to deal with these would be to add a flag to >> conditionally turn off stderr, and then leave it on during normal >> operation and disable it (letting everything through, including whatever >> random cruft git commands produce) for debugging. >> >> But... >> >>> * I have a WIP patch series that deals with errors from git >>>commands. >> >> I'm happy to wait and see what this patch looks like (and generally >> happy to defer to you on maintenance issues for completion, as you are >> much more likely than me to be the one fixing things later on :) ). >> >> -Peff > > Likewise on both counts. So, have we decided to wait, or we'd rather apply the band-aid in the meantime? I can go either way, just double checking as I noticed this thread while updating my leftover bits list. 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] git-p4.py: Make submit working on bare repository
Hi Luke,
Luke Diamand writes:
> I'm guessing that the reason for using a bare repo is so that changes
> can be pushed to it from several different git repos. This then saves
> doing the initial git-p4 clone multiple times.
I have created a Git repository acting as a bridge between Perforce and
pure Git repos. Changes pushed to master branch on this bridge repo get
submitted to Perforce repository (referenced via remote p4/master).
> If this had actually worked, I think the next thing I would want to do
> is to rebase one or more branches in the bare repo against p4/master.
> I don't think there's any way that git-p4 can work out which branches
> would be rebased, and nor should it.
It actually has all information needed. It submits commits from a given
branch to a branch specified with --branch option (or default p4
remote). When submitting from a non-bare repo git-p4 has the same set of
information: the current branch and a branch specified with --branch (or
default p4 remote).
> I think the approach of using a submitFromBare config variable to
> force the user to make a choice feels a bit bogus, since they clearly
> *want* to submit from this bare repo, as otherwise they wouldn't have
> done "git-p4 submit" in the first place.
I agree, a good point.
> It might make sense to have a command-line or config option
> ("--skip-rebase" ?) to tell "submit" to only do the submit part, and
> skip the rebase stage (and get the rebase stage to give a more useful
> error message on a bare repo when the option isn't used). That would
> then mean that git-p4 does not have to know if it's running in a bare
> repo or not, and the submit-without-rebase functionality is available
> to people doing other different things not involving bare repos (which
> we haven't though of yet) but still requiring submit without rebase.
While having additional --skip-rebase is a good idea, having git-p4
doing rebase would be more elegant for those who actually use GitP4 in
bare repository. In message 87fuwnd4u7@freja.aidecoe.name I have
described how state of branches changes during submit. It clearly shows
that in case of a bare repository it ends up in undesired state.
To simplify things, why not just update ref during submit from bare
repository? As you have pointed out, if user invokes submit in this
context he/she actually wants to submit from bare repo and probably
knows what he/she is doing - especially if he/she reads man page. (-:
Kind regards,
--
Amadeusz Żołnowski
signature.asc
Description: PGP signature
Re: interactive rebase results across shared histories
On Tue, 23 Feb 2016 23:57:06 +0100, Moritz Neeb wrote: [...] >> OK, I've followed this advice and looked at the dependency graphs in >> gitk before and after rebasing, I've managed to obtain what I was >> after. The repository now has two branches: master and topic. >> However, Gitk reveals a problem with a string of commits that are not >> part of any branch: >> A---B---H---I (master) \ C---D---E (loose string of commits) \ >> D'---E'---F---G (topic) >> How do I remove these loose commits (C, D, E)? > what you might be after is "git gc". But I never used it, it was not > neccesary for me. I would let the automatic garbage collection drop my > dangling commits. It's safer - who knows when you will still want to > restore your recent "loose string of commits". > How exactly are the loose commits causing trouble? Sure enough, these dangling commits were removed automatically without any intervention. All is good. Thanks! -- Seb -- 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: interactive rebase results across shared histories
On Tue, Feb 23, 2016 at 11:57:06PM +0100, Moritz Neeb wrote: > On 02/23/2016 06:39 PM, Seb wrote: > > On Sun, 21 Feb 2016 03:12:49 +0100, > > Moritz Neeb wrote: > > > >> Hi Seb, > >> On 02/20/2016 11:58 PM, Seb wrote: > >>> Hello, > > > >>> I've recently learnt how to consolidate and clean up the master > >>> branch's commit history. I've squashed/fixuped many commits thinking > >>> these would propagate to the children branches with whom it shares > >>> the earlier parts of the its history. However, this is not the case; > >>> switching to the child branch still shows the non-rebased (dirty) > >>> commit history from master. Am I misunderstanding something with > >>> this? > > > >> I am not sure what you meand by "child branch". If I understand > >> corretly, you have something like: > > > > [...] > > > >> Maybe, to get a better understanding, you could use visualization tool > >> like "tig" or "gitk" to observe what happens to your commits (hashes) > >> and branches (labels) and just play around with some of these > >> operations. > > > > OK, I've followed this advice and looked at the dependency graphs in > > gitk before and after rebasing, I've managed to obtain what I was > > after. The repository now has two branches: master and topic. However, > > Gitk reveals a problem with a string of commits that are not part of any > > branch: > > > > A---B---H---I (master) > > \ > > C---D---E (loose string of commits) > >\ > > D'---E'---F---G (topic) > > > > How do I remove these loose commits (C, D, E)? > > > > what you might be after is "git gc". But I never used it, it was not > neccesary for me. I would let the automatic garbage collection drop my > dangling commits. It's safer - who knows when you will still want to > restore your recent "loose string of commits". > > How exactly are the loose commits causing trouble? > Right, you should normally not care about these commits. In any case, the reflog is still keeping a reference to these commits, and objects not older then 14 days are also kept. But git is regularly running its gc to clean up older objects, and they will not be shared when pushing. -- 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] t8005: avoid grep on non-ASCII data
John Keeping writes:
> My original sed version was:
>
> sed -ne "/^author /p" -e "/^summary /p"
>
> which I think will work on all platforms (we already use it in
> t-basic.sh) but then I decided to be too clever :-(
>
> I still think sed is simpler than introducing a new function to wrap a
> perl script.
Let's do this, before everybody forgets what we discussed.
-- >8 --
From: John Keeping
Date: Sun, 21 Feb 2016 17:32:21 +
Subject: [PATCH] t8005: avoid grep on non-ASCII data
GNU grep 2.23 detects the input used in this test as binary data so it
does not work for extracting lines from a file. We could add the "-a"
option to force grep to treat the input as text, but not all
implementations support that. Instead, use sed to extract the desired
lines since it will always treat its input as text.
While touching these lines, modernize the test style to avoid hiding the
exit status of "git blame" and remove a space following a redirection
operator. Also swap the order of the expected and actual output
files given to test_cmp; we compare expect and actual to show how
actual output differs from what is expected.
Signed-off-by: John Keeping
Signed-off-by: Junio C Hamano
---
t/t8005-blame-i18n.sh | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/t/t8005-blame-i18n.sh b/t/t8005-blame-i18n.sh
index 847d098..75da219 100755
--- a/t/t8005-blame-i18n.sh
+++ b/t/t8005-blame-i18n.sh
@@ -33,11 +33,15 @@ author $SJIS_NAME
summary $SJIS_MSG
EOF
+filter_author_summary () {
+ sed -n -e '/^author /p' -e '/^summary /p' "$@"
+}
+
test_expect_success !MINGW \
'blame respects i18n.commitencoding' '
- git blame --incremental file | \
- egrep "^(author|summary) " > actual &&
- test_cmp actual expected
+ git blame --incremental file >output &&
+ filter_author_summary output >actual &&
+ test_cmp expected actual
'
cat >expected < actual &&
- test_cmp actual expected
+ git blame --incremental file >output &&
+ filter_author_summary output >actual &&
+ test_cmp expected actual
'
cat >expected < actual &&
- test_cmp actual expected
+ git blame --incremental --encoding=UTF-8 file >output &&
+ filter_author_summary output >actual &&
+ test_cmp expected actual
'
cat >expected < actual &&
- test_cmp actual expected
+ git blame --incremental --encoding=none file >output &&
+ filter_author_summary output >actual &&
+ test_cmp expected actual
'
test_done
--
2.7.2-532-g79873b4
--
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: interactive rebase results across shared histories
On 02/23/2016 06:39 PM, Seb wrote: > On Sun, 21 Feb 2016 03:12:49 +0100, > Moritz Neeb wrote: > >> Hi Seb, >> On 02/20/2016 11:58 PM, Seb wrote: >>> Hello, > >>> I've recently learnt how to consolidate and clean up the master >>> branch's commit history. I've squashed/fixuped many commits thinking >>> these would propagate to the children branches with whom it shares >>> the earlier parts of the its history. However, this is not the case; >>> switching to the child branch still shows the non-rebased (dirty) >>> commit history from master. Am I misunderstanding something with >>> this? > >> I am not sure what you meand by "child branch". If I understand >> corretly, you have something like: > > [...] > >> Maybe, to get a better understanding, you could use visualization tool >> like "tig" or "gitk" to observe what happens to your commits (hashes) >> and branches (labels) and just play around with some of these >> operations. > > OK, I've followed this advice and looked at the dependency graphs in > gitk before and after rebasing, I've managed to obtain what I was > after. The repository now has two branches: master and topic. However, > Gitk reveals a problem with a string of commits that are not part of any > branch: > > A---B---H---I (master) > \ > C---D---E (loose string of commits) >\ > D'---E'---F---G (topic) > > How do I remove these loose commits (C, D, E)? > what you might be after is "git gc". But I never used it, it was not neccesary for me. I would let the automatic garbage collection drop my dangling commits. It's safer - who knows when you will still want to restore your recent "loose string of commits". How exactly are the loose commits causing trouble? -Moritz -- 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] t9200: avoid grep on non-ASCII data
Jeff King writes:
> On Sun, Feb 21, 2016 at 11:43:45PM +, John Keeping wrote:
>
>> On Sun, Feb 21, 2016 at 04:15:31PM -0500, Eric Sunshine wrote:
>> > On Sun, Feb 21, 2016 at 12:32 PM, John Keeping wrote:
>> > > GNU grep 2.23 detects the input used in this test as binary data so it
>> > > does not work for extracting lines from a file. We could add the "-a"
>> > > option to force grep to treat the input as text, but not all
>> > > implementations support that. Instead, use sed to extract the desired
>> > > lines since it will always treat its input as text.
>> > >
>> > > Signed-off-by: John Keeping
>> > > ---
>> > > diff --git a/t/t9200-git-cvsexportcommit.sh
>> > > b/t/t9200-git-cvsexportcommit.sh
>> > > @@ -35,7 +35,7 @@ exit 1
>> > > check_entries () {
>> > > # $1 == directory, $2 == expected
>> > > - grep '^/' "$1/CVS/Entries" | sort | cut -d/ -f2,3,5 >actual
>> > > + sed -ne '\!^/!p' "$1/CVS/Entries" | sort | cut -d/ -f2,3,5
>> > > >actual
>> >
>> > This works with BSD sed, but double negatives are confusing. Have you
>> > considered this instead?
>> >
>> > sed -ne '/^\//p' ...
>>
>> What do you mean double negatives? Do you mean using "!" as an
>> alternative delimiter? I find changing delimters is normally simpler
>> than following multiple levels of quoting for escaping slashes, although
>> in this case it's simple enough that it doesn't make much difference.
>
> I agree that changing delimiters is much nicer than backslashes. But I
> wonder if using "!" is more confusing than it needs to be, given its
> other meanings.
>
> I dunno. I admit that the backslash threw me off, too (since it needs
> escaped in interactive shells, I first assumed that's what was going
> on). Using backslash to select the delimiter was new to me. I've usually
> seen:
>
> s!/foo/!/bar/!
>
> which is arguably a little more clear. Too bad we cannot do:
>
> m!/foo!
>
> which I think reads better. Oh well. Maybe:
>
> sed -ne '\#^/#p'
>
> would be more readable, but I'm just bikeshedding at this point. The
> grep invocation really was the most clear. :-/
Eric's '/^\//' was the most straight-forward and easiest to see what
is going on, I would think.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git-rebase + git-mergetool results in broken state
I'm experiencing an annoying issue which leaves the repository in a weird, broken state. I am attempting a rather vanilla rebase, rebasing the commits from a feature branch on top of the newest commits on master. So, I run a typical series of commands: 1. git checkout feature-branch 2. git rebase master (conflicts ensue) 3. git mergetool The conflicts are expected, but when using mergetool to resolve them, I encounter many "no such file or directory" errors. mv: cannot stat ‘app/components/mediaManager/kbImageEditor.directive.coffee’: No such file or directory cp: cannot stat ‘./app/components/mediaManager/kbImageEditor.directive_BACKUP_13615.coffee’: No such file or directory mv: cannot move ‘.merge_file_ogGjXX’ to ‘./app/components/mediaManager/kbImageEditor.directive_BASE_13615.coffee’: No such file or directory /usr/lib/git-core/git-mergetool: 229: /usr/lib/git-core/git-mergetool: cannot create ./app/components/mediaManager/kbImageEditor.directive_LOCAL_13615.coffee: Directory nonexistent /usr/lib/git-core/git-mergetool: 229: /usr/lib/git-core/git-mergetool: cannot create ./app/components/mediaManager/kbImageEditor.directive_REMOTE_13615.coffee: Directory nonexistent This leaves weird dangling files like '.merge_file_ogGjXX' in the repo, and I assume I should not proceed with the merge since it couldn't even create the files to compare. Is this a known issue? Is there any workaround? Is it safe to proceed with the merge? Thanks, Joe Einertson -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
On Tue, Feb 23, 2016 at 12:46 PM, H. Peter Anvin wrote: > On February 23, 2016 12:35:12 PM PST, Junio C Hamano > wrote: >>ebied...@xmission.com (Eric W. Biederman) writes: >> >>> Junio C Hamano writes: >>> It is valuable for a testing organization to say "We tested this series on top of version X. We know it works, we have tested on a lot more hardware than the original developer had, we know this is good to go." It is a valuable service. But that is valuable only if version X is still relevant, isn't it? Is the relevance of a version something that is decided by a developer who submits a patch series, or is it more of an attribute of the project and where the current integration is happening? Judging from the responses from Dan to this thread, I think the answer is the latter, and for the purpose of identifying the relevant version(s), the project does not even care about the exact commit, but it wants to know more about which branch the series is targetted to. With that understanding, I find it hard to believe that it buys the project much for the "base" commit to be recorded in a patch series and automated testing is done by applying the patches to that exact commit, which possibly is no-longer-relevant, even though it may help shielding the testing machinery from "you tested with a wrong version" complaints. Isn't it more valuable for the test robot to say "this may or may not have worked well with whatever old version the patch series was based on, but it no longer is useful to the current tip of the 'master'"? If you consider what benefit the project would gain by having such a robot, that is the conclusion I have to draw. So I still am not convinced that this "record base commit" is a useful thing to do. >>> >>> So I don't know what value this has to the git project. >> >>Oh, don't worry, I wasn't talking about value this may have to the >>Git project at all. "The value to the project" I mentioned in my >>response was all about the value to the kernel project. >> >>> The value of Fengguag's automated testing to the kernel project is to >>> smoke out really stupid things. >> >>I'll snip your bullet points, but as I wrote, I do understand the >>value of prescreening test. >> >>What I was questioning was what value it gives to that testing to >>use "the developer based this patch on this exact commit" added to >>each incoming patch, and have Fengguag's testing machinery test a >>patch with a base version that may no longer be relevant in the >>context of the project. Compared to that, wouldn't "this no longer >>applies to the branch the series targets" or "this still applies >>cleanly, but no longer compiles because the surrounding API has >>changed" be much more valuable? >> >>In your other message, you mentioned the "index $old..$new" lines, >>and it is possible to use them to find a tree that the patch cleanly >>applies to, but it will not uniquely identify _the_ version the >>patch submitter used. IMHO, finding such _a_ tree from the recent >>history of the branch that the patch targets and testing the patch >>on top of that tree (as opposed to testing the patch in the exact >>context the developer worked on) would give the project a better >>value. > > Personally, as a maintainer, I would love to see the tree ID and ideally also > the commit ID a series is based on. The commit ID is in some ways less > useful since it is non-recreatable (and therefore will never match for > anything but the first patch of a series), but could be useful to the > maintainer. As a contributor a commit id would also add value I would think. When it is unclear where a series is headed, contributors in Git land say things like: This applies cleanly on origin/master. And usually this is the master branch from yesterday as Junio pushes once a day. origin/master being a moving target, so it may not apply any more, so a commit sha1 would help for finding out what to do in the maintainer role. This discussion sounds to me as if we'd want to have some advantages of the (kernel pull style, not github style) pull-request here for patch submissions. I don't remember the exact quote, but Linus said once upon a time about the pull request workflow roughly: "Please pull from ... And by the way I tested this software for 2 month during development" (For kernels that makes sense as the contributor run the kernel and it worked) as pull requests have the new patches on top of the exact parents the contributor put them on, there can be more faith in the process to divide between the problems the contributor overlooked/introduced and problems as introduced by the merge of the maintainer. Now when applying patches at another parent than the contributor developed on, some subtle problems may arise, which are not easily spotted by compile tests or runnin
Re: Git History Rewriting in a public repository - capability to remove one or more commits from a public repository
Hi Saravanan, Changes that rewrite history, including (but not limited to) deleted commits, can be pushed with the --force or --force-with-lease options, like this: $ git push --force remote branch --force pushes your changes unconditionally, which may overwrite changes that someone else pushed between the moment you cloned the repo and pushed your own. --force-with-lease will check for others' pushes, so you can use it in a dry run (without actually changing anything in the remote repository) like this: $ git push -n --force-with-lease remote branch If someone else (like another developer with access to "testing-stage") pushes anything before your attempt to push, you will receive a message like: $ git push -n --force-with-lease remote branch ! [rejected]branch -> branch (stale info) error: failed to push some refs to remote Generally speaking, your idea is, probably, better implemented with patches or pull requests: 1) Your developers rewrite their local history as they wish 2) They generate patches from their commits (with git format-patch, for instance) 3) Send those patches to "testing-stage" 4) Apply them to staging area (without committing) 5) Run required checks 6) If checks don't pass, discard those changes 7) If checks pass, commit those patches 8) Push committed changes to "mainline" But in any case, you should better consider using feature branches for that. Best regards, Ilya T. On 02/24/2016 12:30 AM, Saravanan Shanmugham (sarvi) wrote: Hi Git Leads, I am looking for git capability/way to be able to remove commits from a public repository. Background: We are looking for a multi-stage commit process where commits get pushed into a public ³testing-stage² repository. Where we do testing of commits before they are pushed to another public ³mainline² repository. When there are failures seen in the public ³testing-stage² repository. We would like to implement some process to go identify the bad patch and completely eject it from that public ³testing-stage² repository, as if it was not connected. The plan is to use the Git History Rewriting capability described here https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History So I can pull a pull workspace from the public ³testing-stage² repository use the above mechanism to eject one or more commits from it. Now I would like to be able push it back to public ³testing-stage² repository. And allow other people to be able to sync their workspaces to this public ³testing-stage² repository, correctly. This as I understand is not supported?/recommended? in GIT. Mercurial addresses this with the capability to mark commits with a phase such as ³Draft² or ³Experimental² and having a workflow around them. Described here https://www.mercurial-scm.org/wiki/Phases http://www.gerg.ca/evolve/user-guide.html#evolve-user-guide Question: What are the issues? What needs to be done in terms of development, to support this functionality and make it work properly in GIT? Is there additional development that needs to be done to git core to allow this development process? Thanks, Sarvi - Occam's Razor Rules -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message tomajord...@vger.kernel.org More majordomo info athttp://vger.kernel.org/majordomo-info.html -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
"H. Peter Anvin" writes: > On February 23, 2016 12:35:12 PM PST, Junio C Hamano > wrote: >>ebied...@xmission.com (Eric W. Biederman) writes: >> >>> Junio C Hamano writes: >>> It is valuable for a testing organization to say "We tested this series on top of version X. We know it works, we have tested on a lot more hardware than the original developer had, we know this is good to go." It is a valuable service. But that is valuable only if version X is still relevant, isn't it? Is the relevance of a version something that is decided by a developer who submits a patch series, or is it more of an attribute of the project and where the current integration is happening? Judging from the responses from Dan to this thread, I think the answer is the latter, and for the purpose of identifying the relevant version(s), the project does not even care about the exact commit, but it wants to know more about which branch the series is targetted to. With that understanding, I find it hard to believe that it buys the project much for the "base" commit to be recorded in a patch series and automated testing is done by applying the patches to that exact commit, which possibly is no-longer-relevant, even though it may help shielding the testing machinery from "you tested with a wrong version" complaints. Isn't it more valuable for the test robot to say "this may or may not have worked well with whatever old version the patch series was based on, but it no longer is useful to the current tip of the 'master'"? If you consider what benefit the project would gain by having such a robot, that is the conclusion I have to draw. So I still am not convinced that this "record base commit" is a useful thing to do. >>> >>> So I don't know what value this has to the git project. >> >>Oh, don't worry, I wasn't talking about value this may have to the >>Git project at all. "The value to the project" I mentioned in my >>response was all about the value to the kernel project. >> >>> The value of Fengguag's automated testing to the kernel project is to >>> smoke out really stupid things. >> >>I'll snip your bullet points, but as I wrote, I do understand the >>value of prescreening test. >> >>What I was questioning was what value it gives to that testing to >>use "the developer based this patch on this exact commit" added to >>each incoming patch, and have Fengguag's testing machinery test a >>patch with a base version that may no longer be relevant in the >>context of the project. Compared to that, wouldn't "this no longer >>applies to the branch the series targets" or "this still applies >>cleanly, but no longer compiles because the surrounding API has >>changed" be much more valuable? >> >>In your other message, you mentioned the "index $old..$new" lines, >>and it is possible to use them to find a tree that the patch cleanly >>applies to, but it will not uniquely identify _the_ version the >>patch submitter used. IMHO, finding such _a_ tree from the recent >>history of the branch that the patch targets and testing the patch >>on top of that tree (as opposed to testing the patch in the exact >>context the developer worked on) would give the project a better >>value. > > Personally, as a maintainer, I would love to see the tree ID and > ideally also the commit ID a series is based on. The commit ID is in > some ways less useful since it is non-recreatable (and therefore will > never match for anything but the first patch of a series), but could > be useful to the maintainer. > > As far as putting in a bunch of metainformation that has to be > manually or semimanually obtained, there are a lot of issues with > that, as it way too easily turns into wrong information or potential > information leaks that might make corporate contributors > uncomfortable. I would really love to hear from Fengguag about where this is a practical problem, but I expect the flock of x86 topic tress is probably one of those cases where there are problems for automation to figure out which tree a patch applies to in practice. The mailing list for x86 patches is linux-kernel. Even if you filter by x...@kernel.org to detect it is an x86 patch which topic branch a patch goes to was not clear to me from a quick skim of recent lkml x86 patches. So I could really respect a patch header line that said: tree abcdef0123456789...0123456789abcdef Where the numbers where the truncated tree hash before and after a patch was applied. That would seem to give a little bit of extra sanity checking in the application of git diffs as well. Eric -- 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 History Rewriting in a public repository - capability to remove one or more commits from a public repository
On Tue, Feb 23, 2016 at 1:30 PM, Saravanan Shanmugham (sarvi) wrote: > > > Hi Git Leads, > I am looking for git capability/way to be able to remove commits > from a public repository. > > Background: > We are looking for a multi-stage commit process where commits get pushed > into a public ³testing-stage² repository. > Where we do testing of commits before they are pushed to another public > ³mainline² repository. instead of different repositories you could have different branches, but as you like... > > When there are failures seen in the public ³testing-stage² repository. > We would like to implement some process to go identify the bad patch and > completely eject it from that public ³testing-stage² repository, as if it > was not connected. So git revert is not an option, but you really want to purge the commit as if it never existed? That is not possible without rewriting history (which is considered bad behavior for public repositories) You can use a cherry-picking workflow where you cherry-pick the good commits, once you are sure they are good indeed. > > The plan is to use the Git History Rewriting capability described here > https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History > So I can pull a pull workspace from the public ³testing-stage² repository > use the above mechanism to eject one or more commits from it. > > Now I would like to be able push it back to public ³testing-stage² > repository. when using the cherry-pick workflow above (cherry picking from testing to public), you can still merge back public to testing > And allow other people to be able to sync their workspaces to this public > ³testing-stage² repository, correctly. git fetch testing && git reset --hard testing-stage test-branch would do that client side. > > This as I understand is not supported?/recommended? in GIT. It is supported as Git is a toolkit and you can align your workflow using different tools from the box. But it's not what anyone with prior knowledge of "How to work with Git" expects things to be. > > Mercurial addresses this with the capability to mark commits with a phase > such as ³Draft² or ³Experimental² and having a workflow around them. > Described here > https://www.mercurial-scm.org/wiki/Phases > > http://www.gerg.ca/evolve/user-guide.html#evolve-user-guide > > > > Question: > What are the issues? Rewriting history is considered bad practice, and that is probably the reason why there are no good tools AFAIK to verify rewritten history easily. When you consider published history permanent, you can use gpg or local tags to know what "is already good" and only inspect new incoming deltas for correctness. > What needs to be done in terms of development, to support this > functionality and make it work properly in GIT? > Is there additional development that needs to be done to git core to allow > this development process? I think the workflow you described can be done using current tools. You would just need to polish things or create aliases for things as this seems to be an unusual workflow? Thanks, Stefan > > > Thanks, > Sarvi > - > Occam's Razor Rules > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] activate diff.renames by default
Matthieu Moy writes: > Junio C Hamano writes: > >> Matthieu Moy writes: >> >>> I have always wondered why diff.renames was not activated by default. >>> I've had it to true in my configuration for 9 years, and I've been >>> teaching newbies to set it for a while without issue. I think it's >>> time to activate it by default, but please let me know if I missed a >>> reason to keep it to false. >>> >>> In any case, the first 3 patches are useful cleanups. >> >> It's a long time coming since I heard "I love how I can just say >> 'oh, keep in mind that we might want to..' and 24 hours later you >> did it." [*1*] >> >> I can hardly be an impartial judge for this series, though. >> >> [Reference] >> >> *1* http://thread.gmane.org/gmane.comp.version-control.git/3541/focus=3702 > > I guess there's another reference needed to fully understand your > message, and I'm not sure I have it right. Are you refering to the > "merge-recursive: test rename threshold option" discussion? Not at all. I was just mentioning that it would at last be a major achievement, if this can safely turned on by default for everybody, after having invented the machinery and the feature long time ago ;-) -- 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 4/5] README.md: don't call git stupid in the title
Matthieu Moy writes: > Junio C Hamano writes: > >> Having said that, I agree with the spirit of 4/5 and 5/5; but it is >> sad that this line is not resurrected by 5/5 in some way. > > Do you mean something like this: > > diff --git a/README.md b/README.md > index 40de78e..b1c89bd 100644 > --- a/README.md > +++ b/README.md > @@ -41,7 +41,8 @@ list. The discussion following them give a good reference > for > project status, development direction and remaining tasks. > > The name "git" was given by Linus Torvalds when he wrote the very > -first version. He described it as (depending on your mood): > +first version. He described the tool as "the stupid content tracker" > +and the name as (depending on your mood): Exactly. -- 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 "thin" submodule clone to feed "describe"
Martin Langhoff writes: > Hi git list! long time no see! :-) Been missing you lots. > > Do we currently have any means to clone _history_ but not _blobs_ of a > repo, or some approximation thereof? > > With a bit more context: If I have a top-level project using a couple > dozen submodules, where the submodules are huge, do I have a > git-native means of running git-describe on each submodule without > pulling the whole thing down? No, I do not think so. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git History Rewriting in a public repository - capability to remove one or more commits from a public repository
Hi Git Leads, I am looking for git capability/way to be able to remove commits from a public repository. Background: We are looking for a multi-stage commit process where commits get pushed into a public ³testing-stage² repository. Where we do testing of commits before they are pushed to another public ³mainline² repository. When there are failures seen in the public ³testing-stage² repository. We would like to implement some process to go identify the bad patch and completely eject it from that public ³testing-stage² repository, as if it was not connected. The plan is to use the Git History Rewriting capability described here https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History So I can pull a pull workspace from the public ³testing-stage² repository use the above mechanism to eject one or more commits from it. Now I would like to be able push it back to public ³testing-stage² repository. And allow other people to be able to sync their workspaces to this public ³testing-stage² repository, correctly. This as I understand is not supported?/recommended? in GIT. Mercurial addresses this with the capability to mark commits with a phase such as ³Draft² or ³Experimental² and having a workflow around them. Described here https://www.mercurial-scm.org/wiki/Phases http://www.gerg.ca/evolve/user-guide.html#evolve-user-guide Question: What are the issues? What needs to be done in terms of development, to support this functionality and make it work properly in GIT? Is there additional development that needs to be done to git core to allow this development process? Thanks, Sarvi - Occam's Razor Rules -- 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 5/5] diff: activate diff.renames by default
Junio C Hamano writes:
> Matthieu Moy writes:
>
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 7f96c64..6e34df3 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -103,6 +103,7 @@ static int log_line_range_callback(const struct option
>> *option, const char *arg,
>> static void init_log_defaults()
>> {
>> init_grep_defaults();
>> +git_diff_ui_default_config();
>> }
>
> Why isn't the new function called init_diff_ui_defaults()?
Indeed, that's a better name. I changed it.
>> diff --git a/diff.c b/diff.c
>> index 2136b69..d5db898 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -168,6 +168,11 @@ long parse_algorithm_value(const char *value)
>> * never be affected by the setting of diff.renames
>> * the user happens to have in the configuration file.
>> */
>> +void git_diff_ui_default_config()
>
> s/()/(void)/; same for the declaration in the header file.
Fixed. 'doing too much C++ and not enough C ;-).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 3/3] credential-cache--daemon: change to the socket dir on startup
On Tue, Feb 23, 2016 at 01:09:44PM -0800, Junio C Hamano wrote:
> I tentatively squashed this, which I think reads better.
>
> diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
> index 9a3a7a3..6b00ee0 100644
> --- a/credential-cache--daemon.c
> +++ b/credential-cache--daemon.c
> @@ -237,12 +237,13 @@ static void init_socket_directory(const char *path)
> die_errno("unable to mkdir '%s'", dir);
> }
>
> - /*
> - * We don't actually care what our cwd is; we chdir here just to
> - * be a friendly daemon and avoid tying up our original cwd.
> - * If this fails, it's OK to just continue without that benefit.
> - */
> - chdir(dir);
> + if (chdir(dir))
> + /*
> + * We don't actually care what our cwd is; we chdir here just to
> + * be a friendly daemon and avoid tying up our original cwd.
> + * If this fails, it's OK to just continue without that benefit.
> + */
> + ;
>
> free(path_copy);
> }
That looks great, thanks.
-Peff
--
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 3/3] credential-cache--daemon: change to the socket dir on startup
Jeff King writes:
> On Tue, Feb 23, 2016 at 01:06:10PM -0800, Junio C Hamano wrote:
>
>> Jeff King writes:
>>
>> > + /*
>> > + * We don't actually care what our cwd is; we chdir here just to
>> > + * be a friendly daemon and avoid tying up our original cwd.
>> > + * If this fails, it's OK to just continue without that benefit.
>> > + */
>> > + chdir(dir);
>>
>> I fully do agree with this comment, but my copy of gcc and system
>> headers doesn't, unfortunately.
>>
>> credential-cache--daemon.c: In function 'init_socket_directory':
>> credential-cache--daemon.c:245:7: error: ignoring return value of 'chdir',
>> declared with attribute warn_unused_result [-Werror=unused-result]
>> chdir(dir);
>>^
>> cc1: all warnings being treated as errors
>> make: *** [credential-cache--daemon.o] Error 1
>
> Ugh. Is:
>
> (void)chdir(dir);
>
> enough? Or do we have to do:
>
> if (chdir(dir))
> ; /* nothing */
>
> ?
>
> -Peff
I tentatively squashed this, which I think reads better.
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index 9a3a7a3..6b00ee0 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -237,12 +237,13 @@ static void init_socket_directory(const char *path)
die_errno("unable to mkdir '%s'", dir);
}
- /*
-* We don't actually care what our cwd is; we chdir here just to
-* be a friendly daemon and avoid tying up our original cwd.
-* If this fails, it's OK to just continue without that benefit.
-*/
- chdir(dir);
+ if (chdir(dir))
+ /*
+* We don't actually care what our cwd is; we chdir here just to
+* be a friendly daemon and avoid tying up our original cwd.
+* If this fails, it's OK to just continue without that benefit.
+*/
+ ;
free(path_copy);
}
--
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 3/3] credential-cache--daemon: change to the socket dir on startup
On Tue, Feb 23, 2016 at 01:06:10PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > + /* > > +* We don't actually care what our cwd is; we chdir here just to > > +* be a friendly daemon and avoid tying up our original cwd. > > +* If this fails, it's OK to just continue without that benefit. > > +*/ > > + chdir(dir); > > I fully do agree with this comment, but my copy of gcc and system > headers doesn't, unfortunately. > > credential-cache--daemon.c: In function 'init_socket_directory': > credential-cache--daemon.c:245:7: error: ignoring return value of 'chdir', > declared with attribute warn_unused_result [-Werror=unused-result] > chdir(dir); >^ > cc1: all warnings being treated as errors > make: *** [credential-cache--daemon.o] Error 1 Ugh. Is: (void)chdir(dir); enough? Or do we have to do: if (chdir(dir)) ; /* nothing */ ? -Peff -- 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 3/3] credential-cache--daemon: change to the socket dir on startup
Jeff King writes: > + /* > + * We don't actually care what our cwd is; we chdir here just to > + * be a friendly daemon and avoid tying up our original cwd. > + * If this fails, it's OK to just continue without that benefit. > + */ > + chdir(dir); I fully do agree with this comment, but my copy of gcc and system headers doesn't, unfortunately. credential-cache--daemon.c: In function 'init_socket_directory': credential-cache--daemon.c:245:7: error: ignoring return value of 'chdir', declared with attribute warn_unused_result [-Werror=unused-result] chdir(dir); ^ cc1: all warnings being treated as errors make: *** [credential-cache--daemon.o] Error 1 -- 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] Documentation/git-push: document that 'simple' is the default
The default behavior is well documented already in git-config(1), but git-push(1) itself did not mention it at all. For users willing to learn how "git push" works but not how to configure it, this makes the documentation cumbersome to read. Make the git-push(1) page self-contained by adding a short summary of what 'push.default=simple' does, early in the page. Signed-off-by: Matthieu Moy --- Documentation/git-push.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 32482ce..a992793 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -37,6 +37,13 @@ the default `` by consulting `remote.*.push` configuration, and if it is not found, honors `push.default` configuration to decide what to push (See linkgit:git-config[1] for the meaning of `push.default`). +When neither the command-line nor the configuration specify what to +push, the default behavior is used, which corresponds to the `simple` +value for `push.default`: the current branch is pushed to the +corresponding upstream branch, but as a safety measure, the push is +aborted if the upstream branch does not have the same name as the +local one. + OPTIONS[[OPTIONS]] -- -- 2.7.2.334.g35ed2ae.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
git "thin" submodule clone to feed "describe"
Hi git list! long time no see! :-) Been missing you lots. Do we currently have any means to clone _history_ but not _blobs_ of a repo, or some approximation thereof? With a bit more context: If I have a top-level project using a couple dozen submodules, where the submodules are huge, do I have a git-native means of running git-describe on each submodule without pulling the whole thing down? In this context, most developers want to get full checkout of some submodules, but not of all; and 'git describe' of the submodules is needed to 'shim' the missing submodules appropriately. If the answer is no, there's a bunch of ways I can carry that as extra data in the top level project. It's possible, yet inelegant & duplicative. thanks, martin -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- 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: shorten "push.default is unset" warning message
"Philip Oakley" writes: > Shouldn't this also update the 'push' man page to state what the new > default is. @gerry's comment to the top answer > http://stackoverflow.com/a/13148313/717355 highlights that the word > 'simple' is not even mentioned in the 'push' man page. This is more or less a different topic IMHO. If git-push(1) is not clear enough, then it should be clarified regardless of this patch. But a patch follows. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 5/5] diff: activate diff.renames by default
Matthieu Moy writes:
> diff --git a/builtin/log.c b/builtin/log.c
> index 7f96c64..6e34df3 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -103,6 +103,7 @@ static int log_line_range_callback(const struct option
> *option, const char *arg,
> static void init_log_defaults()
> {
> init_grep_defaults();
> + git_diff_ui_default_config();
> }
Why isn't the new function called init_diff_ui_defaults()?
> diff --git a/diff.c b/diff.c
> index 2136b69..d5db898 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -168,6 +168,11 @@ long parse_algorithm_value(const char *value)
> * never be affected by the setting of diff.renames
> * the user happens to have in the configuration file.
> */
> +void git_diff_ui_default_config()
s/()/(void)/; same for the declaration in the header file.
--
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 diff HEAD^(255) fails
On Sat, Feb 06, 2016 at 10:56:46PM +0100, Ole Tange wrote: > git diff first looks for a file, then looks if it is a reference to a > revision. If the file fails due to being too long, the diff fails: > > $ git init > $ git diff > 'HEAD^^^' > HEAD > fatal: failed to stat > 'HEAD^^^': > File name too long > > If file name too long it should just try to see if it is a reference > to a revision. > Is there a reason you are repeating 255 "^" instead of using HEAD~255? -- 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/1] convert.c: correct attr_action
tbo...@web.de writes:
> From: Torsten Bögershausen
>
> Commit "convert.c: refactor crlf_action" unified the crlf_handling
> and introdused a bug for "git ls-files --eol".
> The "text" attribute was shown as "text eol=lf" or "text eol=crlf",
> depending on core.autpcrlf or core.eol.
> Correct this and add test cases in t0027.
Sigh. I think I typofixed this when I queued the previous version
while commenting on it.
Do people not pay attention to what is queued in 'pu' these days?
>
> Signed-off-by: Torsten Bögershausen
> ---
> convert.c| 18 +-
> t/t0027-auto-crlf.sh | 33 ++---
> 2 files changed, 35 insertions(+), 16 deletions(-)
I looked at the interdiff from the previous version; the updated
parts looked sensible.
Thanks, will queue.
>
> diff --git a/convert.c b/convert.c
> index 18af685..d8b1f17 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -708,7 +708,7 @@ static enum crlf_action git_path_check_crlf(struct
> git_attr_check *check)
> const char *value = check->value;
>
> if (ATTR_TRUE(value))
> - return text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;
> + return CRLF_TEXT;
> else if (ATTR_FALSE(value))
> return CRLF_BINARY;
> else if (ATTR_UNSET(value))
> @@ -778,20 +778,20 @@ static void convert_attrs(struct conv_attrs *ca, const
> char *path)
> }
>
> if (!git_check_attr(path, NUM_CONV_ATTRS, ccheck)) {
> - enum eol eol_attr;
> ca->crlf_action = git_path_check_crlf(ccheck + 4);
> if (ca->crlf_action == CRLF_UNDEFINED)
> ca->crlf_action = git_path_check_crlf(ccheck + 0);
> ca->attr_action = ca->crlf_action;
> ca->ident = git_path_check_ident(ccheck + 1);
> ca->drv = git_path_check_convert(ccheck + 2);
> - if (ca->crlf_action == CRLF_BINARY)
> - return;
> - eol_attr = git_path_check_eol(ccheck + 3);
> - if (eol_attr == EOL_LF)
> - ca->crlf_action = CRLF_TEXT_INPUT;
> - else if (eol_attr == EOL_CRLF)
> - ca->crlf_action = CRLF_TEXT_CRLF;
> + if (ca->crlf_action != CRLF_BINARY) {
> + enum eol eol_attr = git_path_check_eol(ccheck + 3);
> + if (eol_attr == EOL_LF)
> + ca->crlf_action = CRLF_TEXT_INPUT;
> + else if (eol_attr == EOL_CRLF)
> + ca->crlf_action = CRLF_TEXT_CRLF;
> + }
> + ca->attr_action = ca->crlf_action;
> } else {
> ca->drv = NULL;
> ca->crlf_action = CRLF_UNDEFINED;
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index fc4c628..f33962b 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -159,6 +159,25 @@ stats_ascii () {
>
> }
>
> +
> +# contruct the attr/ returned by git ls-files --eol
> +# Take none (=empty), one or two args
> +attr_ascii () {
> + case $1,$2 in
> + -text,*) echo "-text" ;;
> + text,) echo "text" ;;
> + text,lf) echo "text eol=lf" ;;
> + text,crlf) echo "text eol=crlf" ;;
> + auto,) echo "text=auto" ;;
> + auto,lf) echo "text=auto eol=lf" ;;
> + auto,crlf) echo "text=auto eol=crlf" ;;
> + lf,) echo "text eol=lf" ;;
> + crlf,) echo "text eol=crlf" ;;
> + ,) echo "" ;;
> + *) echo invalid_attr "$1,$2" ;;
> + esac
> +}
> +
> check_files_in_repo () {
> crlf=$1
> attr=$2
> @@ -228,15 +247,15 @@ checkout_files () {
> test_expect_success "ls-files --eol attr=$attr $ident $aeol
> core.autocrlf=$crlf core.eol=$ceol" '
> test_when_finished "rm expect actual" &&
> sort <<-EOF >expect &&
> - i/crlf w/$(stats_ascii $crlfname) crlf_false_attr__CRLF.txt
> - i/mixed w/$(stats_ascii $lfmixcrlf)
> crlf_false_attr__CRLF_mix_LF.txt
> - i/lf w/$(stats_ascii $lfname) crlf_false_attr__LF.txt
> - i/-text w/$(stats_ascii $lfmixcr) crlf_false_attr__LF_mix_CR.txt
> - i/-text w/$(stats_ascii $crlfnul) crlf_false_attr__CRLF_nul.txt
> - i/-text w/$(stats_ascii $crlfnul) crlf_false_attr__LF_nul.txt
> + i/crlf w/$(stats_ascii $crlfname) attr/$(attr_ascii $attr
> $aeol) crlf_false_attr__CRLF.txt
> + i/mixed w/$(stats_ascii $lfmixcrlf) attr/$(attr_ascii $attr
> $aeol) crlf_false_attr__CRLF_mix_LF.txt
> + i/lf w/$(stats_ascii $lfname) attr/$(attr_ascii $attr $aeol)
> crlf_false_attr__LF.txt
> + i/-text w/$(stats_ascii $lfmixcr) attr/$(attr_ascii $attr
> $aeol) crlf_false_attr__LF_mix_CR.txt
> + i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr
> $aeol) crlf_false_attr__CRLF_nul.txt
> + i/-text w/$(stats_ascii $crlfnul) attr/$(attr_ascii $attr
> $aeol) crl
Re: [PATCH 4/5] README.md: don't call git stupid in the title
Junio C Hamano writes: > Matthieu Moy writes: > >> "the stupid content tracker" was true in the early days of Git, but >> hardly applicable these days. "fast, scalable, distributed" describes >> Git more accuralety. >> >> Also, "stupid" can be seen as offensive by some people. Let's not use it >> in the very first words of the README. >> >> The new formulation is taken from the description of the Debian package. >> >> Signed-off-by: Matthieu Moy >> --- > > This self-derogatory reference shouldn't offend those who didn't > help write it. > > Having said that, I agree with the spirit of 4/5 and 5/5; but it is > sad that this line is not resurrected by 5/5 in some way. Do you mean something like this: diff --git a/README.md b/README.md index 40de78e..b1c89bd 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,8 @@ list. The discussion following them give a good reference for project status, development direction and remaining tasks. The name "git" was given by Linus Torvalds when he wrote the very -first version. He described it as (depending on your mood): +first version. He described the tool as "the stupid content tracker" +and the name as (depending on your mood): - random three-letter combination that is pronounceable, and not actually used by any common UNIX command. The fact that it is a ? Why not, but I don't think it adds really much, and I'd rather keep the README as short as possible. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
On February 23, 2016 12:35:12 PM PST, Junio C Hamano wrote: >ebied...@xmission.com (Eric W. Biederman) writes: > >> Junio C Hamano writes: >> >>> It is valuable for a testing organization to say "We tested this >>> series on top of version X. We know it works, we have tested on a >>> lot more hardware than the original developer had, we know this is >>> good to go." It is a valuable service. >>> >>> But that is valuable only if version X is still relevant, isn't it? >>> >>> Is the relevance of a version something that is decided by a >>> developer who submits a patch series, or is it more of an attribute >>> of the project and where the current integration is happening? >>> Judging from the responses from Dan to this thread, I think the >>> answer is the latter, and for the purpose of identifying the >>> relevant version(s), the project does not even care about the exact >>> commit, but it wants to know more about which branch the series is >>> targetted to. >>> >>> With that understanding, I find it hard to believe that it buys the >>> project much for the "base" commit to be recorded in a patch series >>> and automated testing is done by applying the patches to that exact >>> commit, which possibly is no-longer-relevant, even though it may >>> help shielding the testing machinery from "you tested with a wrong >>> version" complaints. >>> >>> Isn't it more valuable for the test robot to say "this may or may >>> not have worked well with whatever old version the patch series was >>> based on, but it no longer is useful to the current tip of the >>> 'master'"? If you consider what benefit the project would gain by >>> having such a robot, that is the conclusion I have to draw. >>> >>> So I still am not convinced that this "record base commit" is a >>> useful thing to do. >> >> So I don't know what value this has to the git project. > >Oh, don't worry, I wasn't talking about value this may have to the >Git project at all. "The value to the project" I mentioned in my >response was all about the value to the kernel project. > >> The value of Fengguag's automated testing to the kernel project is to >> smoke out really stupid things. > >I'll snip your bullet points, but as I wrote, I do understand the >value of prescreening test. > >What I was questioning was what value it gives to that testing to >use "the developer based this patch on this exact commit" added to >each incoming patch, and have Fengguag's testing machinery test a >patch with a base version that may no longer be relevant in the >context of the project. Compared to that, wouldn't "this no longer >applies to the branch the series targets" or "this still applies >cleanly, but no longer compiles because the surrounding API has >changed" be much more valuable? > >In your other message, you mentioned the "index $old..$new" lines, >and it is possible to use them to find a tree that the patch cleanly >applies to, but it will not uniquely identify _the_ version the >patch submitter used. IMHO, finding such _a_ tree from the recent >history of the branch that the patch targets and testing the patch >on top of that tree (as opposed to testing the patch in the exact >context the developer worked on) would give the project a better >value. Personally, as a maintainer, I would love to see the tree ID and ideally also the commit ID a series is based on. The commit ID is in some ways less useful since it is non-recreatable (and therefore will never match for anything but the first patch of a series), but could be useful to the maintainer. As far as putting in a bunch of metainformation that has to be manually or semimanually obtained, there are a lot of issues with that, as it way too easily turns into wrong information or potential information leaks that might make corporate contributors uncomfortable. -- Sent from my Android device with K-9 Mail. Please excuse brevity and formatting. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] activate diff.renames by default
Junio C Hamano writes: > Matthieu Moy writes: > >> I have always wondered why diff.renames was not activated by default. >> I've had it to true in my configuration for 9 years, and I've been >> teaching newbies to set it for a while without issue. I think it's >> time to activate it by default, but please let me know if I missed a >> reason to keep it to false. >> >> In any case, the first 3 patches are useful cleanups. > > It's a long time coming since I heard "I love how I can just say > 'oh, keep in mind that we might want to..' and 24 hours later you > did it." [*1*] > > I can hardly be an impartial judge for this series, though. > > [Reference] > > *1* http://thread.gmane.org/gmane.comp.version-control.git/3541/focus=3702 I guess there's another reference needed to fully understand your message, and I'm not sure I have it right. Are you refering to the "merge-recursive: test rename threshold option" discussion? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: shorten "push.default is unset" warning message
From: "Matthieu Moy"
From: Matthieu Moy
The warning was purposely long, both to explain the situation properly
and to give a strong incentive to set push.default explicitly. This was
important before the 2.0 transition, and remained important for a while
after, so that new users get push.default explicitly in their
configuration and do not experience inconsistent behavior if they ever
used an older version of Git.
The warning has been there since version 1.8.0 (Oct 2012), hence we can
expect the vast majority of current Git users to have been exposed to
it, and most of them have already set push.default explicitly. The
switch from 'matching' to 'simple' was planned for 2.0 (May 2014), but
actually happened only for 2.3 (Feb 2015).
The warning is mostly seen by beginners, who have not set their
push.default configuration (yet). For many of them, the warning is
confusing because it talks about concepts that they have not learned and
asks them a choice that they are not able to make yet. See for example
http://stackoverflow.com/questions/13148066/warning-push-default-is-unset-its-implicit-value-is-changing-in-git-2-0
Shouldn't this also update the 'push' man page to state what the new default
is. @gerry's comment to the top answer
http://stackoverflow.com/a/13148313/717355 highlights that the word 'simple'
is not even mentioned in the 'push' man page.
(1260 votes for the question, 1824 for the answer as of writing)
Shorten the warning and mention only the way to remove the warning
without changing the behavior. Keep a pointer to the documentation so
that people willing to learn can still find the alternative behaviors
easily.
Eventually, the warning should be removed completely, but this can wait
a couple more releases or years.
Signed-off-by: Matthieu Moy
---
builtin/push.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 960ffc3..00eba2f 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -205,26 +205,12 @@ static void setup_push_current(struct remote
*remote, struct branch *branch)
}
static char warn_unspecified_push_default_msg[] =
-N_("push.default is unset; its implicit value has changed in\n"
- "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
- "and maintain the traditional behavior, use:\n"
- "\n"
- " git config --global push.default matching\n"
- "\n"
- "To squelch this message and adopt the new behavior now, use:\n"
+N_("push.default is unset; its default value has changed in Git 2.0
from\n"
+ "'matching' to 'simple'. To squelch this message and adopt the new
behavior, use:\n"
"\n"
" git config --global push.default simple\n"
"\n"
- "When push.default is set to 'matching', git will push local
branches\n"
- "to the remote branches that already exist with the same name.\n"
- "\n"
- "Since Git 2.0, Git defaults to the more conservative 'simple'\n"
- "behavior, which only pushes the current branch to the
corresponding\n"
- "remote branch that 'git pull' uses to update the current branch.\n"
- "\n"
- "See 'git help config' and search for 'push.default' for further
information.\n"
- "(the 'simple' mode was introduced in Git 1.7.11. Use the similar
mode\n"
- "'current' instead of 'simple' if you sometimes use older versions of
Git)");
+ "See 'git help config' and search for 'push.default' for further
information.");
static void warn_unspecified_push_default_configuration(void)
{
--
https://github.com/git/git/pull/201
--
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
--
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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
ebied...@xmission.com (Eric W. Biederman) writes: > Junio C Hamano writes: > >> It is valuable for a testing organization to say "We tested this >> series on top of version X. We know it works, we have tested on a >> lot more hardware than the original developer had, we know this is >> good to go." It is a valuable service. >> >> But that is valuable only if version X is still relevant, isn't it? >> >> Is the relevance of a version something that is decided by a >> developer who submits a patch series, or is it more of an attribute >> of the project and where the current integration is happening? >> Judging from the responses from Dan to this thread, I think the >> answer is the latter, and for the purpose of identifying the >> relevant version(s), the project does not even care about the exact >> commit, but it wants to know more about which branch the series is >> targetted to. >> >> With that understanding, I find it hard to believe that it buys the >> project much for the "base" commit to be recorded in a patch series >> and automated testing is done by applying the patches to that exact >> commit, which possibly is no-longer-relevant, even though it may >> help shielding the testing machinery from "you tested with a wrong >> version" complaints. >> >> Isn't it more valuable for the test robot to say "this may or may >> not have worked well with whatever old version the patch series was >> based on, but it no longer is useful to the current tip of the >> 'master'"? If you consider what benefit the project would gain by >> having such a robot, that is the conclusion I have to draw. >> >> So I still am not convinced that this "record base commit" is a >> useful thing to do. > > So I don't know what value this has to the git project. Oh, don't worry, I wasn't talking about value this may have to the Git project at all. "The value to the project" I mentioned in my response was all about the value to the kernel project. > The value of Fengguag's automated testing to the kernel project is to > smoke out really stupid things. I'll snip your bullet points, but as I wrote, I do understand the value of prescreening test. What I was questioning was what value it gives to that testing to use "the developer based this patch on this exact commit" added to each incoming patch, and have Fengguag's testing machinery test a patch with a base version that may no longer be relevant in the context of the project. Compared to that, wouldn't "this no longer applies to the branch the series targets" or "this still applies cleanly, but no longer compiles because the surrounding API has changed" be much more valuable? In your other message, you mentioned the "index $old..$new" lines, and it is possible to use them to find a tree that the patch cleanly applies to, but it will not uniquely identify _the_ version the patch submitter used. IMHO, finding such _a_ tree from the recent history of the branch that the patch targets and testing the patch on top of that tree (as opposed to testing the patch in the exact context the developer worked on) would give the project a better value. -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
Junio C Hamano writes: > Fengguang Wu writes: > >>> >> I have a mixed feeling about this one, primarily because this was >>> >> already tried quite early in the life of "format-patch" command. >>> >> >>> >> >>> >> http://thread.gmane.org/gmane.comp.version-control.git/9694/focus=9757 >>> >> >>> >> Only the name is different (it was called "applies-to" and named a >>> >> tree object). >>> > >>> > Either commit or tree object will work for us. We can use it in >>> > v2 if you prefer tree object. >>> >>> Sorry, I think you misunderstood. By "only the name is different", I >>> didn't mean to say that the tree object name should be shown as the >>> old proposal did. What I meant but didn't explicitly say, as I >>> thought it was sufficient to point at an old discussion thread, was >>> that this was already tried and rejected. This round uses different >>> name but does essentially the same thing as the old proposal, and I >>> do not think I heard anything new that supports this patch against >>> earlier rejection by Linus. That is what gave me a mixed feeling. >> >> I can understand the rejection by Linus in development process POV. >> >> However we are facing a new situation: in test robot POV, IMHO there >> are values to test exactly the same tree as the patch submitter. >> Otherwise the robot risks >> >> - false negative: failing to apply and test some patches >> - false positive: sending wrong bug reports due to guessed wrong base tree > > I always get negatives and positives confused, so let me think aloud > with an example. Let's say that somebody worked on adding a new > feature based on v4.2 codebase and sent in a patch series. The > series touched files in quiescent part of the system, these files > are identical between v4.2 and the current codebase at v4.5-rc5, and > the series applies cleanly to a "wrong" base tree at the tip of > 'master'. But it turns out that the series uses an old API that was > removed in the meantime. The test robot may say "the result of > applying the series does not even build" and the developer would > complain to you saying "You tested with a wrong version". > > I've already said that I can see the value this approach has for > you. By having the developer state which commit the series was > based on, it will shield you from such a complaint, because you > would not use closer-to-tip 'master' as the base, but instead use > v4.2 codebase for the test. > > As I said, what is unclear to me is what value this apporach gives > to the project. > >>> I can see that recording the exact commit object name allows you to >>> claim that you identified the exact commit to apply the patch, and >>> that you tested the exact tree contents. It however is unclear what >>> the value of such a claim would be to the project or to the >>> integrator. >> >> The value of base commit info is: providing a solid ground to the >> tester, to reliably avoid false positive/negatives. > > It is valuable for a testing organization to say "We tested this > series on top of version X. We know it works, we have tested on a > lot more hardware than the original developer had, we know this is > good to go." It is a valuable service. > > But that is valuable only if version X is still relevant, isn't it? > > Is the relevance of a version something that is decided by a > developer who submits a patch series, or is it more of an attribute > of the project and where the current integration is happening? > Judging from the responses from Dan to this thread, I think the > answer is the latter, and for the purpose of identifying the > relevant version(s), the project does not even care about the exact > commit, but it wants to know more about which branch the series is > targetted to. > > With that understanding, I find it hard to believe that it buys the > project much for the "base" commit to be recorded in a patch series > and automated testing is done by applying the patches to that exact > commit, which possibly is no-longer-relevant, even though it may > help shielding the testing machinery from "you tested with a wrong > version" complaints. > > Isn't it more valuable for the test robot to say "this may or may > not have worked well with whatever old version the patch series was > based on, but it no longer is useful to the current tip of the > 'master'"? If you consider what benefit the project would gain by > having such a robot, that is the conclusion I have to draw. > > So I still am not convinced that this "record base commit" is a > useful thing to do. So I don't know what value this has to the git project. The value of Fengguag's automated testing to the kernel project is to smoke out really stupid things. - A developer forgot to actually compile test their code. An error report that the code does not even compile from Fengguag allows the patch to be dropped without spending any time on it. - A developer failed to test some weird configuration option in like !CONFIG_SY
Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
Fengguag Wu, Xiaolong Ye, have you attempted to use the truncated sha1 of the file the patch applies to? Git already places a file sha1 at the top of a patch. See the index line? > diff --git a/fs/namespace.c b/fs/namespace.c > index eccd925c6e82..3c3f8172c734 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c As I understand it you are aiming for making a good guess what the patch or patches apply to, having a set of file hashes looks like it would give you that. All it should take is to iterate over a patchset and for each file in the patchset capture the first file hash. Then in the smallish set of maintainer trees see if that set of file hashes matches any of their recent commits. You should be able to prune the set of possible maintainer trees even more by looking at the mailling list or lists the patch was submitted to. Before we talk about adding anything more I think we need a clear picture of what you have tried with what already exists. A decade ago part of the problem was that not everyone used git. At best it will take a little while before everyone upgrades to a version of git diff containing your changes, and if possibly even longer if they have to start specifying an additional option when a diff is generated. Eric -- 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/5] README.md: add hyperlinks on filenames
Junio C Hamano writes: > Matthieu Moy writes: > >> Signed-off-by: Matthieu Moy >> --- >> README.md | 19 +-- >> 1 file changed, 13 insertions(+), 6 deletions(-) > > Makes sense, provided if we want to do Markdown. I'd say it the other way around: declaring README as markdown costs almost nothing and doesn't harm source code readability. This patch slightly decreases the source's readability so we may want to drop it. > If I were pushing this topic (i.e. cater to those who browse at > GitHub, not with "less" in the source tree), I'd have further made > these links to the preformatted documentation at git-scm.com; I > expected the later steps in this series to do that, but it seems you > stopped short of it for some reason. I tried to keep the spirit of the README as "this is the entry point to the source code" rather than "this is the new homepage of Git" (as some project do on GitHub, but we have such a nice git-scm.com ...). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
Fengguang Wu writes: >> >> I have a mixed feeling about this one, primarily because this was >> >> already tried quite early in the life of "format-patch" command. >> >> >> >> http://thread.gmane.org/gmane.comp.version-control.git/9694/focus=9757 >> >> >> >> Only the name is different (it was called "applies-to" and named a >> >> tree object). >> > >> > Either commit or tree object will work for us. We can use it in >> > v2 if you prefer tree object. >> >> Sorry, I think you misunderstood. By "only the name is different", I >> didn't mean to say that the tree object name should be shown as the >> old proposal did. What I meant but didn't explicitly say, as I >> thought it was sufficient to point at an old discussion thread, was >> that this was already tried and rejected. This round uses different >> name but does essentially the same thing as the old proposal, and I >> do not think I heard anything new that supports this patch against >> earlier rejection by Linus. That is what gave me a mixed feeling. > > I can understand the rejection by Linus in development process POV. > > However we are facing a new situation: in test robot POV, IMHO there > are values to test exactly the same tree as the patch submitter. > Otherwise the robot risks > > - false negative: failing to apply and test some patches > - false positive: sending wrong bug reports due to guessed wrong base tree I always get negatives and positives confused, so let me think aloud with an example. Let's say that somebody worked on adding a new feature based on v4.2 codebase and sent in a patch series. The series touched files in quiescent part of the system, these files are identical between v4.2 and the current codebase at v4.5-rc5, and the series applies cleanly to a "wrong" base tree at the tip of 'master'. But it turns out that the series uses an old API that was removed in the meantime. The test robot may say "the result of applying the series does not even build" and the developer would complain to you saying "You tested with a wrong version". I've already said that I can see the value this approach has for you. By having the developer state which commit the series was based on, it will shield you from such a complaint, because you would not use closer-to-tip 'master' as the base, but instead use v4.2 codebase for the test. As I said, what is unclear to me is what value this apporach gives to the project. >> I can see that recording the exact commit object name allows you to >> claim that you identified the exact commit to apply the patch, and >> that you tested the exact tree contents. It however is unclear what >> the value of such a claim would be to the project or to the >> integrator. > > The value of base commit info is: providing a solid ground to the > tester, to reliably avoid false positive/negatives. It is valuable for a testing organization to say "We tested this series on top of version X. We know it works, we have tested on a lot more hardware than the original developer had, we know this is good to go." It is a valuable service. But that is valuable only if version X is still relevant, isn't it? Is the relevance of a version something that is decided by a developer who submits a patch series, or is it more of an attribute of the project and where the current integration is happening? Judging from the responses from Dan to this thread, I think the answer is the latter, and for the purpose of identifying the relevant version(s), the project does not even care about the exact commit, but it wants to know more about which branch the series is targetted to. With that understanding, I find it hard to believe that it buys the project much for the "base" commit to be recorded in a patch series and automated testing is done by applying the patches to that exact commit, which possibly is no-longer-relevant, even though it may help shielding the testing machinery from "you tested with a wrong version" complaints. Isn't it more valuable for the test robot to say "this may or may not have worked well with whatever old version the patch series was based on, but it no longer is useful to the current tip of the 'master'"? If you consider what benefit the project would gain by having such a robot, that is the conclusion I have to draw. So I still am not convinced that this "record base commit" is a useful thing to 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: [PATCH 1/5] README: use markdown syntax
Junio C Hamano writes: > Matthieu Moy writes: > >> This allows repository browsers like GitHub to display the content of >> the file nicely formatted. >> >> Signed-off-by: Matthieu Moy >> --- > > To be honest, I have the most problem with this step in the whole > series. > > Markdown when rendered may be easier to read, but plain text is even > easier, and it somehow feels backward to cater to those who browse > at GitHub sacrificing those who use "less" in the source tree. Well, actually almost all the page was already in markdown. The real change done by this patch is a rename, and change the asciiart in the title: >> --- a/README >> +++ b/README.md >> @@ -1,8 +1,4 @@ >> - >> - >> -Git - the stupid content tracker >> - >> - >> +# Git - the stupid content tracker Markdown would also accept ascii-art underlining. Git - the stupid content tracker we can use this if people think it's easier to read in the source. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: shorten "push.default is unset" warning message
Junio C Hamano writes: > The punchline of that question is: > > I can obviously set it to one of the values mentioned, but what do > they mean? What's the difference between simple and matching? > > It tells us that "See 'git help config'" is not such an effective > message to help such a user. [ The teacher inside me speaks ] Don't underestimate the ability to ignore any pointer to the doc from many users. In many case, when a student comes to me scared about an error or warning message, I just ask the student to read the message to me. If it's in english, I sometimes have to add "so, what does this mean in French", and in many cases it's sufficient. >> Shorten the warning and mention only the way to remove the warning >> without changing the behavior. Keep a pointer to the documentation so >> that people willing to learn can still find the alternative behaviors >> easily. > > While I admit that I usually am the most cautious one when dealing > with any change, I am not sure if this rephrasing helps very much. > As we saw, the sentence you kept, "See 'git help config'", is not > effective in helping those stackoverflow users. Right. But assuming someone who reads the complete message, I found that keeping only the first lines without a pointer to the doc make the text kind of mysterious: warning: push.default is unset; its default value has changed in Git 2.0 from 'matching' to 'simple'. To squelch this message and adopt the new behavior, use: git config --global push.default simple Alone, this really looks like a magic formula like "I'm showing you this warning just to bug you, but you can get rid of it with ...". > If most people are happy with "simple" (and certainly that was the > assumption and hope behind the transtion we made at 2.0), we may be > better off removing the warning altogether. To me, this is the plan. I have no strong objection in removing it completely, but I think it makes sense to keep a lightweight one for a while: if people use different machines with different Git versions (especially if they ever use a version in the interval 2.0 to 2.3 which claims to use simple but actually do not), then these people may appreciate an incentive to set push.default. OTOH, if they use different Git versions, they will eventually get the message. > After all, push.default configuration is hardly the only case where > there are other ways to use Git that may match the user's situation > better, and we do not advertise "Oh by the way you can do things > differently, study the manual" for any of them with a warning > message like this. Those who want to do different things know to > seek settings to tweak. I completely agree with this. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] activate diff.renames by default
Matthieu Moy writes: > I have always wondered why diff.renames was not activated by default. > I've had it to true in my configuration for 9 years, and I've been > teaching newbies to set it for a while without issue. I think it's > time to activate it by default, but please let me know if I missed a > reason to keep it to false. > > In any case, the first 3 patches are useful cleanups. It's a long time coming since I heard "I love how I can just say 'oh, keep in mind that we might want to..' and 24 hours later you did it." [*1*] I can hardly be an impartial judge for this series, though. [Reference] *1* http://thread.gmane.org/gmane.comp.version-control.git/3541/focus=3702 > Matthieu Moy (5): > Documentation/diff-config: fix description of diff.renames > t4001-diff-rename: wrap file creations in a test > t: add tests for diff.renames (true/false/unset) > log: introduce init_log_defaults() > diff: activate diff.renames by default > > Documentation/diff-config.txt | 7 ++-- > builtin/commit.c | 1 + > builtin/diff.c| 1 + > builtin/log.c | 16 --- > builtin/merge.c | 1 + > diff.c| 5 +++ > diff.h| 1 + > t/t4001-diff-rename.sh| 97 > +++ > t/t4013-diff-various.sh | 2 + > t/t4014-format-patch.sh | 4 +- > t/t4047-diff-dirstat.sh | 3 +- > t/t4202-log.sh| 8 ++-- > 12 files changed, 114 insertions(+), 32 deletions(-) -- 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 4/5] README.md: don't call git stupid in the title
Matthieu Moy writes: > "the stupid content tracker" was true in the early days of Git, but > hardly applicable these days. "fast, scalable, distributed" describes > Git more accuralety. > > Also, "stupid" can be seen as offensive by some people. Let's not use it > in the very first words of the README. > > The new formulation is taken from the description of the Debian package. > > Signed-off-by: Matthieu Moy > --- This self-derogatory reference shouldn't offend those who didn't help write it. Having said that, I agree with the spirit of 4/5 and 5/5; but it is sad that this line is not resurrected by 5/5 in some way. > README.md | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/README.md b/README.md > index 1625352..c11c3e2 100644 > --- a/README.md > +++ b/README.md > @@ -1,4 +1,4 @@ > -# Git - the stupid content tracker > +# Git - fast, scalable, distributed revision control system > > "git" can mean anything, depending on your mood. -- 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/5] README.md: add hyperlinks on filenames
Matthieu Moy writes:
> Signed-off-by: Matthieu Moy
> ---
> README.md | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
Makes sense, provided if we want to do Markdown.
If I were pushing this topic (i.e. cater to those who browse at
GitHub, not with "less" in the source tree), I'd have further made
these links to the preformatted documentation at git-scm.com; I
expected the later steps in this series to do that, but it seems you
stopped short of it for some reason.
> diff --git a/README.md b/README.md
> index 907eb3b..750fdda 100644
> --- a/README.md
> +++ b/README.md
> @@ -20,17 +20,17 @@ License version 2 (some parts of it are under different
> licenses,
> compatible with the GPLv2). It was originally written by Linus
> Torvalds with help of a group of hackers around the net.
>
> -Please read the file INSTALL for installation instructions.
> +Please read the file [INSTALL][] for installation instructions.
>
> -See Documentation/gittutorial.txt to get started, then see
> -Documentation/giteveryday.txt for a useful minimum set of commands, and
> -Documentation/git-commandname.txt for documentation of each command.
> +See [Documentation/gittutorial.txt][] to get started, then see
> +[Documentation/giteveryday.txt][] for a useful minimum set of commands, and
> +[Documentation/git-commandname.txt][] for documentation of each command.
> If git has been correctly installed, then the tutorial can also be
> read with "man gittutorial" or "git help tutorial", and the
> documentation of each command with "man git-commandname" or "git help
> commandname".
>
> -CVS users may also want to read Documentation/gitcvs-migration.txt
> +CVS users may also want to read [Documentation/gitcvs-migration.txt][]
> ("man gitcvs-migration" or "git help cvs-migration" if git is
> installed).
>
> @@ -40,7 +40,7 @@ including full documentation and Git related tools.
> The user discussion and development of Git take place on the Git
> mailing list -- everyone is welcome to post bug reports, feature
> requests, comments and patches to git@vger.kernel.org (read
> -Documentation/SubmittingPatches for instructions on patch submission).
> +[Documentation/SubmittingPatches][] for instructions on patch submission).
> To subscribe to the list, send an email with just "subscribe git" in
> the body to majord...@vger.kernel.org. The mailing list archives are
> available at http://news.gmane.org/gmane.comp.version-control.git/,
> @@ -50,3 +50,10 @@ The maintainer frequently sends the "What's cooking"
> reports that
> list the current status of various development topics to the mailing
> list. The discussion following them give a good reference for
> project status, development direction and remaining tasks.
> +
> +[INSTALL]: INSTALL
> +[Documentation/gittutorial.txt]: Documentation/gittutorial.txt
> +[Documentation/giteveryday.txt]: Documentation/giteveryday.txt
> +[Documentation/git-commandname.txt]: Documentation/git-commandname.txt
> +[Documentation/gitcvs-migration.txt]: Documentation/gitcvs-migration.txt
> +[Documentation/SubmittingPatches]: Documentation/SubmittingPatches
--
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/5] README: use markdown syntax
Matthieu Moy writes: > This allows repository browsers like GitHub to display the content of > the file nicely formatted. > > Signed-off-by: Matthieu Moy > --- To be honest, I have the most problem with this step in the whole series. Markdown when rendered may be easier to read, but plain text is even easier, and it somehow feels backward to cater to those who browse at GitHub sacrificing those who use "less" in the source tree. > README => README.md | 6 +- > t/t7001-mv.sh | 2 +- > 2 files changed, 2 insertions(+), 6 deletions(-) > rename README => README.md (93%) > > diff --git a/README b/README.md > similarity index 93% > rename from README > rename to README.md > index 1083735..907eb3b 100644 > --- a/README > +++ b/README.md > @@ -1,8 +1,4 @@ > - > - > - Git - the stupid content tracker > - > - > +# Git - the stupid content tracker > > "git" can mean anything, depending on your mood. > > diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh > index 51dd2b4..4008fae 100755 > --- a/t/t7001-mv.sh > +++ b/t/t7001-mv.sh > @@ -102,7 +102,7 @@ test_expect_success \ > > test_expect_success \ > 'adding another file' \ > -'cp "$TEST_DIRECTORY"/../README path0/README && > +'cp "$TEST_DIRECTORY"/../README.md path0/README && > git add path0/README && > git commit -m add2 -a' -- 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: shorten "push.default is unset" warning message
Matthieu Moy writes:
> The warning is mostly seen by beginners, who have not set their
> push.default configuration (yet). For many of them, the warning is
> confusing because it talks about concepts that they have not learned and
> asks them a choice that they are not able to make yet. See for example
>
>
> http://stackoverflow.com/questions/13148066/warning-push-default-is-unset-its-implicit-value-is-changing-in-git-2-0
>
> (1260 votes for the question, 1824 for the answer as of writing)
The punchline of that question is:
I can obviously set it to one of the values mentioned, but what do
they mean? What's the difference between simple and matching?
It tells us that "See 'git help config'" is not such an effective
message to help such a user.
> Shorten the warning and mention only the way to remove the warning
> without changing the behavior. Keep a pointer to the documentation so
> that people willing to learn can still find the alternative behaviors
> easily.
While I admit that I usually am the most cautious one when dealing
with any change, I am not sure if this rephrasing helps very much.
As we saw, the sentence you kept, "See 'git help config'", is not
effective in helping those stackoverflow users. Removal of the
other parts of the message the patch does does make sense, as we
know these users do not read, so they are merely noisy black pixels
on the screen.
If most people are happy with "simple" (and certainly that was the
assumption and hope behind the transtion we made at 2.0), we may be
better off removing the warning altogether. Keeping "and adopt the
new behaviour" part pretends to be offering a chance to make an
informed choice, but it will forever be unclear to the non-reader
what the implication of not adopting the new behaviour is anyway, so
overall we won't see reduced hits at stackoverflow with this change.
After all, push.default configuration is hardly the only case where
there are other ways to use Git that may match the user's situation
better, and we do not advertise "Oh by the way you can do things
differently, study the manual" for any of them with a warning
message like this. Those who want to do different things know to
seek settings to tweak.
The above analysis considers _only_ those who go to stackoverflow.
For those who do read, perhaps "See 'git config help'" may have some
value, but again, many aspects of the system can be tweaked, and we
do not advertise that everywhere, so...
> Eventually, the warning should be removed completely, but this can wait
> a couple more releases or years.
> Signed-off-by: Matthieu Moy
> ---
> builtin/push.c | 20 +++-
> 1 file changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 960ffc3..00eba2f 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -205,26 +205,12 @@ static void setup_push_current(struct remote *remote,
> struct branch *branch)
> }
>
> static char warn_unspecified_push_default_msg[] =
> -N_("push.default is unset; its implicit value has changed in\n"
> - "Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
> - "and maintain the traditional behavior, use:\n"
> - "\n"
> - " git config --global push.default matching\n"
> - "\n"
> - "To squelch this message and adopt the new behavior now, use:\n"
> +N_("push.default is unset; its default value has changed in Git 2.0 from\n"
> + "'matching' to 'simple'. To squelch this message and adopt the new
> behavior, use:\n"
> "\n"
> " git config --global push.default simple\n"
> "\n"
> - "When push.default is set to 'matching', git will push local branches\n"
> - "to the remote branches that already exist with the same name.\n"
> - "\n"
> - "Since Git 2.0, Git defaults to the more conservative 'simple'\n"
> - "behavior, which only pushes the current branch to the corresponding\n"
> - "remote branch that 'git pull' uses to update the current branch.\n"
> - "\n"
> - "See 'git help config' and search for 'push.default' for further
> information.\n"
> - "(the 'simple' mode was introduced in Git 1.7.11. Use the similar mode\n"
> - "'current' instead of 'simple' if you sometimes use older versions of
> Git)");
> + "See 'git help config' and search for 'push.default' for further
> information.");
>
> static void warn_unspecified_push_default_configuration(void)
> {
>
> --
> https://github.com/git/git/pull/201
--
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
Invalid initial git gui message encoding
Hello.
Working with Git for Windows on git gui I noticed an issue on reading
initial message to git gui message prompt.
Steps to reproduce:
git init .
echo 'a' > a && git add . && git commit -m 'added A'
git checkout -b devel
echo 'b' > b && git add . && git commit -m 'added B (ęóąśłżźćń)'
echo 'bbb' >> b && git add . && git commit -m 'changed B (żźćńąśłóę)'
git checkout master
git merge --squash devel
after above commands I run git gui where I get initial message with
invalid encoding. I described it more widely on github:
https://github.com/git-for-windows/git/issues/664#issuecomment-187664072.
I prepared a fix on https://github.com/git-for-windows/git/pull/665.
Before I send a patch based on git://repo.or.cz/git-gui.git may I ask
you to review my changes? I also attach them to this message below.
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 1834f00..5771973 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1599,11 +1599,13 @@ proc run_prepare_commit_msg_hook {} {
if {[file isfile [gitdir MERGE_MSG]]} {
set pcm_source "merge"
set fd_mm [open [gitdir MERGE_MSG] r]
+ fconfigure $fd_mm -encoding utf-8
puts -nonewline $fd_pcm [read $fd_mm]
close $fd_mm
} elseif {[file isfile [gitdir SQUASH_MSG]]} {
set pcm_source "squash"
set fd_sm [open [gitdir SQUASH_MSG] r]
+ fconfigure $fd_sm -encoding utf-8
puts -nonewline $fd_pcm [read $fd_sm]
close $fd_sm
} else {
---
Yours sincerely,
Jarek Z.
--
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 v5 23/27] svn: learn ref-storage argument
On Sat, 2016-02-20 at 23:55 +, Eric Wong wrote:
> David Turner wrote:
> > +++ b/git-svn.perl
>
> > + if (defined $_ref_storage) {
> > + push @init_db, "--ref-storage=" .
> > $_ref_storage;
> > + }
>
> Minor nit: git-svn uses tabs for indentation.
> Otherwise, if we go this route, consider it:
Got it, thanks.
> Signed-off-by: Eric Wong
>
> Thanks.
>
> I would favor Shawn's RefTree or similar to reuse existing
> code + commands and avoid the external dependency, though.
It's an interesting idea. I'm not sure how good the performance would
be in the case where there are a large number of refs in a single
directory, but it would be worth a try.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] diff: activate diff.renames by default
Rename detection is a very convenient feature, and new users shouldn't
have to dig in the documentation to benefit from it.
Potential objections to activating rename detection are that it
sometimes fail, and it is sometimes slow. But rename detection is
already activated by default in several cases like "git status" and "git
merge", so activating diff.renames does not fundamentally change the
situation. When the rename detection fails, it now fails consistently
between "git diff" and "git status".
This setting does not affect plumbing commands, hence well-written
scripts will not be affected.
Signed-off-by: Matthieu Moy
---
Documentation/diff-config.txt | 2 +-
builtin/commit.c | 1 +
builtin/diff.c| 1 +
builtin/log.c | 1 +
builtin/merge.c | 1 +
diff.c| 5 +
diff.h| 1 +
t/t4001-diff-rename.sh| 2 +-
t/t4013-diff-various.sh | 2 ++
t/t4014-format-patch.sh | 4 ++--
t/t4047-diff-dirstat.sh | 3 ++-
t/t4202-log.sh| 8
12 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 1acd203..fdf5a79 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -111,7 +111,7 @@ diff.renames::
Whether and how Git detects renames. If set to "false",
rename detection is disabled. If set to "true", basic rename
detection is enable. If set to "copies" or "copy", Git will
- detect copies, as well. Defaults to false.
+ detect copies, as well. Defaults to true.
diff.suppressBlankEmpty::
A boolean to inhibit the standard behavior of printing a space
diff --git a/builtin/commit.c b/builtin/commit.c
index b3bd2d4..3cb4843 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -186,6 +186,7 @@ static void status_init_config(struct wt_status *s,
config_fn_t fn)
gitmodules_config();
git_config(fn, s);
determine_whence(s);
+ git_diff_ui_default_config();
s->hints = advice_status_hints; /* must come after git_config() */
}
diff --git a/builtin/diff.c b/builtin/diff.c
index 52c98a9..8bd1fd5 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -318,6 +318,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
if (!no_index)
gitmodules_config();
+ git_diff_ui_default_config();
git_config(git_diff_ui_config, NULL);
init_revisions(&rev, prefix);
diff --git a/builtin/log.c b/builtin/log.c
index 7f96c64..6e34df3 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -103,6 +103,7 @@ static int log_line_range_callback(const struct option
*option, const char *arg,
static void init_log_defaults()
{
init_grep_defaults();
+ git_diff_ui_default_config();
}
static void cmd_log_init_defaults(struct rev_info *rev)
diff --git a/builtin/merge.c b/builtin/merge.c
index b98a348..cf297d4 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1187,6 +1187,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
else
head_commit = lookup_commit_or_die(head_sha1, "HEAD");
+ git_diff_ui_default_config();
git_config(git_merge_config, NULL);
if (branch_mergeoptions)
diff --git a/diff.c b/diff.c
index 2136b69..d5db898 100644
--- a/diff.c
+++ b/diff.c
@@ -168,6 +168,11 @@ long parse_algorithm_value(const char *value)
* never be affected by the setting of diff.renames
* the user happens to have in the configuration file.
*/
+void git_diff_ui_default_config()
+{
+ diff_detect_rename_default = 1;
+}
+
int git_diff_ui_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
diff --git a/diff.h b/diff.h
index 70b2d70..75686d5 100644
--- a/diff.h
+++ b/diff.h
@@ -266,6 +266,7 @@ extern int parse_long_opt(const char *opt, const char
**argv,
const char **optarg);
extern int git_diff_basic_config(const char *var, const char *value, void *cb);
+extern void git_diff_ui_default_config();
extern int git_diff_ui_config(const char *var, const char *value, void *cb);
extern void diff_setup(struct diff_options *);
extern int diff_opt_parse(struct diff_options *, const char **, int, const
char *);
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 6844906..15d99a3 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -124,7 +124,7 @@ test_expect_success 'test diff.renames=false' '
test_expect_success 'test diff.renames unset' '
git diff --cached $tree >current &&
- compare_diff_patch current no-rename
+ compare_diff_patch current expected
'
test_expect_success 'favour same basenames over different ones' '
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 6ec6072..94ef500 100755
--- a/t/t4013-diff-var
[PATCH 2/5] t4001-diff-rename: wrap file creations in a test
Signed-off-by: Matthieu Moy --- t/t4001-diff-rename.sh | 38 +- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh index 2f327b7..bfb364c 100755 --- a/t/t4001-diff-rename.sh +++ b/t/t4001-diff-rename.sh @@ -9,7 +9,9 @@ test_description='Test rename detection in diff engine. . ./test-lib.sh . "$TEST_DIRECTORY"/diff-lib.sh -echo >path0 'Line 1 +test_expect_success 'setup' ' + cat >path0 <<\EOF && +Line 1 Line 2 Line 3 Line 4 @@ -24,6 +26,23 @@ Line 12 Line 13 Line 14 Line 15 +EOF + cat >expected <<\EOF +diff --git a/path0 b/path1 +rename from path0 +rename to path1 +--- a/path0 b/path1 +@@ -8,7 +8,7 @@ Line 7 + Line 8 + Line 9 + Line 10 +-line 11 ++Line 11 + Line 12 + Line 13 + Line 14 +EOF ' test_expect_success \ @@ -43,22 +62,7 @@ test_expect_success \ test_expect_success \ 'git diff-index -p -M after rename and editing.' \ 'git diff-index -p -M $tree >current' -cat >expected <<\EOF -diff --git a/path0 b/path1 -rename from path0 -rename to path1 a/path0 -+++ b/path1 -@@ -8,7 +8,7 @@ Line 7 - Line 8 - Line 9 - Line 10 --line 11 -+Line 11 - Line 12 - Line 13 - Line 14 -EOF + test_expect_success \ 'validate the output.' \ -- 2.7.2.334.g35ed2ae.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] log: introduce init_log_defaults()
This is currently a wrapper around init_grep_defaults(), but will allow
adding more initialization in further patches.
Signed-off-by: Matthieu Moy
---
builtin/log.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 0d738d6..7f96c64 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -100,6 +100,11 @@ static int log_line_range_callback(const struct option
*option, const char *arg,
return 0;
}
+static void init_log_defaults()
+{
+ init_grep_defaults();
+}
+
static void cmd_log_init_defaults(struct rev_info *rev)
{
if (fmt_pretty)
@@ -416,7 +421,7 @@ int cmd_whatchanged(int argc, const char **argv, const char
*prefix)
struct rev_info rev;
struct setup_revision_opt opt;
- init_grep_defaults();
+ init_log_defaults();
git_config(git_log_config, NULL);
init_revisions(&rev, prefix);
@@ -527,7 +532,7 @@ int cmd_show(int argc, const char **argv, const char
*prefix)
struct pathspec match_all;
int i, count, ret = 0;
- init_grep_defaults();
+ init_log_defaults();
git_config(git_log_config, NULL);
memset(&match_all, 0, sizeof(match_all));
@@ -608,7 +613,7 @@ int cmd_log_reflog(int argc, const char **argv, const char
*prefix)
struct rev_info rev;
struct setup_revision_opt opt;
- init_grep_defaults();
+ init_log_defaults();
git_config(git_log_config, NULL);
init_revisions(&rev, prefix);
@@ -647,7 +652,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)
struct rev_info rev;
struct setup_revision_opt opt;
- init_grep_defaults();
+ init_log_defaults();
git_config(git_log_config, NULL);
init_revisions(&rev, prefix);
@@ -1280,7 +1285,7 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
extra_hdr.strdup_strings = 1;
extra_to.strdup_strings = 1;
extra_cc.strdup_strings = 1;
- init_grep_defaults();
+ init_log_defaults();
git_config(git_format_config, NULL);
init_revisions(&rev, prefix);
rev.commit_format = CMIT_FMT_EMAIL;
--
2.7.2.334.g35ed2ae.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] Documentation/diff-config: fix description of diff.renames
The description was misleading, since "set to any boolean value" include "set to false", and diff.renames=false does not enable basic detection, but actually disables it. Signed-off-by: Matthieu Moy --- Documentation/diff-config.txt | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 6eaa452..1acd203 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -108,9 +108,10 @@ diff.renameLimit:: detection; equivalent to the 'git diff' option '-l'. diff.renames:: - Tells Git to detect renames. If set to any boolean value, it - will enable basic rename detection. If set to "copies" or - "copy", it will detect copies, as well. + Whether and how Git detects renames. If set to "false", + rename detection is disabled. If set to "true", basic rename + detection is enable. If set to "copies" or "copy", Git will + detect copies, as well. Defaults to false. diff.suppressBlankEmpty:: A boolean to inhibit the standard behavior of printing a space -- 2.7.2.334.g35ed2ae.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] t: add tests for diff.renames (true/false/unset)
The underlying machinery is well-tested, but the configuration option itself was tested only in t3400-rebase.sh. Signed-off-by: Matthieu Moy --- t/t4001-diff-rename.sh | 61 +- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh index bfb364c..6844906 100755 --- a/t/t4001-diff-rename.sh +++ b/t/t4001-diff-rename.sh @@ -27,7 +27,7 @@ Line 13 Line 14 Line 15 EOF - cat >expected <<\EOF + cat >expected <<\EOF && diff --git a/path0 b/path1 rename from path0 rename to path1 @@ -43,6 +43,50 @@ rename to path1 Line 13 Line 14 EOF + cat >no-rename <<\EOF +diff --git a/path0 b/path0 +deleted file mode 100644 +index fdbec44..000 +--- a/path0 /dev/null +@@ -1,15 +0,0 @@ +-Line 1 +-Line 2 +-Line 3 +-Line 4 +-Line 5 +-Line 6 +-Line 7 +-Line 8 +-Line 9 +-Line 10 +-line 11 +-Line 12 +-Line 13 +-Line 14 +-Line 15 +diff --git a/path1 b/path1 +new file mode 100644 +index 000..752c50e +--- /dev/null b/path1 +@@ -0,0 +1,15 @@ ++Line 1 ++Line 2 ++Line 3 ++Line 4 ++Line 5 ++Line 6 ++Line 7 ++Line 8 ++Line 9 ++Line 10 ++Line 11 ++Line 12 ++Line 13 ++Line 14 ++Line 15 +EOF ' test_expect_success \ @@ -68,6 +112,21 @@ test_expect_success \ 'validate the output.' \ 'compare_diff_patch current expected' +test_expect_success 'test diff.renames=true' ' + git -c diff.renames=true diff --cached $tree >current && + compare_diff_patch current expected +' + +test_expect_success 'test diff.renames=false' ' + git -c diff.renames=false diff --cached $tree >current && + compare_diff_patch current no-rename +' + +test_expect_success 'test diff.renames unset' ' + git diff --cached $tree >current && + compare_diff_patch current no-rename +' + test_expect_success 'favour same basenames over different ones' ' cp path1 another-path && git add another-path && -- 2.7.2.334.g35ed2ae.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 0/5] activate diff.renames by default
I have always wondered why diff.renames was not activated by default. I've had it to true in my configuration for 9 years, and I've been teaching newbies to set it for a while without issue. I think it's time to activate it by default, but please let me know if I missed a reason to keep it to false. In any case, the first 3 patches are useful cleanups. Matthieu Moy (5): Documentation/diff-config: fix description of diff.renames t4001-diff-rename: wrap file creations in a test t: add tests for diff.renames (true/false/unset) log: introduce init_log_defaults() diff: activate diff.renames by default Documentation/diff-config.txt | 7 ++-- builtin/commit.c | 1 + builtin/diff.c| 1 + builtin/log.c | 16 --- builtin/merge.c | 1 + diff.c| 5 +++ diff.h| 1 + t/t4001-diff-rename.sh| 97 +++ t/t4013-diff-various.sh | 2 + t/t4014-format-patch.sh | 4 +- t/t4047-diff-dirstat.sh | 3 +- t/t4202-log.sh| 8 ++-- 12 files changed, 114 insertions(+), 32 deletions(-) -- 2.7.2.334.g35ed2ae.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] README.md: don't call git stupid in the title
"the stupid content tracker" was true in the early days of Git, but hardly applicable these days. "fast, scalable, distributed" describes Git more accuralety. Also, "stupid" can be seen as offensive by some people. Let's not use it in the very first words of the README. The new formulation is taken from the description of the Debian package. Signed-off-by: Matthieu Moy --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1625352..c11c3e2 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Git - the stupid content tracker +# Git - fast, scalable, distributed revision control system "git" can mean anything, depending on your mood. -- 2.7.2.334.g35ed2ae.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] README.md: move down historical explanation about the name
The explanations about why the name was chosen are secondary compared to the description and link to the documentation. Some consider these explanations as good computer scientists joke, but other see it as needlessly offensive vocabulary. This patch preserves the historical joke, but gives it less importance by moving it to the end of the README, and makes it clear that it is a historical explanation, that does not necessarily reflect the state of mind of current developers. Signed-off-by: Matthieu Moy --- README.md | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index c11c3e2..40de78e 100644 --- a/README.md +++ b/README.md @@ -1,16 +1,5 @@ # Git - fast, scalable, distributed revision control system -"git" can mean anything, depending on your mood. - - - random three-letter combination that is pronounceable, and not - actually used by any common UNIX command. The fact that it is a - mispronunciation of "get" may or may not be relevant. - - stupid. contemptible and despicable. simple. Take your pick from the - dictionary of slang. - - "global information tracker": you're in a good mood, and it actually - works for you. Angels sing, and a light suddenly fills the room. - - "goddamn idiotic truckload of sh*t": when it breaks - Git is a fast, scalable, distributed revision control system with an unusually rich command set that provides both high-level operations and full access to internals. @@ -51,6 +40,18 @@ list the current status of various development topics to the mailing list. The discussion following them give a good reference for project status, development direction and remaining tasks. +The name "git" was given by Linus Torvalds when he wrote the very +first version. He described it as (depending on your mood): + + - random three-letter combination that is pronounceable, and not + actually used by any common UNIX command. The fact that it is a + mispronunciation of "get" may or may not be relevant. + - stupid. contemptible and despicable. simple. Take your pick from the + dictionary of slang. + - "global information tracker": you're in a good mood, and it actually + works for you. Angels sing, and a light suddenly fills the room. + - "goddamn idiotic truckload of sh*t": when it breaks + [INSTALL]: INSTALL [Documentation/gittutorial.txt]: Documentation/gittutorial.txt [Documentation/giteveryday.txt]: Documentation/giteveryday.txt -- 2.7.2.334.g35ed2ae.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
[RFC/PATCH 0/5] Make README more pleasant to read
This patch series was inspired by a discussion I had with Emma Jane after Git Merge last year. It tries both to make the README file less agressive and generally more pleasant to read. To get a quick overview, compare the old one: https://github.com/git/git#readme and my proposal: https://github.com/moy/git/tree/git-readme#readme Matthieu Moy (5): README: use markdown syntax README.md: add hyperlinks on filenames README.md: move the link to git-scm.com up README.md: don't call git stupid in the title README.md: move down historical explanation about the name README => README.md | 54 - t/t7001-mv.sh | 2 +- 2 files changed, 30 insertions(+), 26 deletions(-) rename README => README.md (67%) -- 2.7.2.334.g35ed2ae.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 2/5] README.md: add hyperlinks on filenames
Signed-off-by: Matthieu Moy
---
README.md | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/README.md b/README.md
index 907eb3b..750fdda 100644
--- a/README.md
+++ b/README.md
@@ -20,17 +20,17 @@ License version 2 (some parts of it are under different
licenses,
compatible with the GPLv2). It was originally written by Linus
Torvalds with help of a group of hackers around the net.
-Please read the file INSTALL for installation instructions.
+Please read the file [INSTALL][] for installation instructions.
-See Documentation/gittutorial.txt to get started, then see
-Documentation/giteveryday.txt for a useful minimum set of commands, and
-Documentation/git-commandname.txt for documentation of each command.
+See [Documentation/gittutorial.txt][] to get started, then see
+[Documentation/giteveryday.txt][] for a useful minimum set of commands, and
+[Documentation/git-commandname.txt][] for documentation of each command.
If git has been correctly installed, then the tutorial can also be
read with "man gittutorial" or "git help tutorial", and the
documentation of each command with "man git-commandname" or "git help
commandname".
-CVS users may also want to read Documentation/gitcvs-migration.txt
+CVS users may also want to read [Documentation/gitcvs-migration.txt][]
("man gitcvs-migration" or "git help cvs-migration" if git is
installed).
@@ -40,7 +40,7 @@ including full documentation and Git related tools.
The user discussion and development of Git take place on the Git
mailing list -- everyone is welcome to post bug reports, feature
requests, comments and patches to git@vger.kernel.org (read
-Documentation/SubmittingPatches for instructions on patch submission).
+[Documentation/SubmittingPatches][] for instructions on patch submission).
To subscribe to the list, send an email with just "subscribe git" in
the body to majord...@vger.kernel.org. The mailing list archives are
available at http://news.gmane.org/gmane.comp.version-control.git/,
@@ -50,3 +50,10 @@ The maintainer frequently sends the "What's cooking" reports
that
list the current status of various development topics to the mailing
list. The discussion following them give a good reference for
project status, development direction and remaining tasks.
+
+[INSTALL]: INSTALL
+[Documentation/gittutorial.txt]: Documentation/gittutorial.txt
+[Documentation/giteveryday.txt]: Documentation/giteveryday.txt
+[Documentation/git-commandname.txt]: Documentation/git-commandname.txt
+[Documentation/gitcvs-migration.txt]: Documentation/gitcvs-migration.txt
+[Documentation/SubmittingPatches]: Documentation/SubmittingPatches
--
2.7.2.334.g35ed2ae.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: interactive rebase results across shared histories
On Sun, 21 Feb 2016 03:12:49 +0100, Moritz Neeb wrote: > Hi Seb, > On 02/20/2016 11:58 PM, Seb wrote: >> Hello, >> I've recently learnt how to consolidate and clean up the master >> branch's commit history. I've squashed/fixuped many commits thinking >> these would propagate to the children branches with whom it shares >> the earlier parts of the its history. However, this is not the case; >> switching to the child branch still shows the non-rebased (dirty) >> commit history from master. Am I misunderstanding something with >> this? > I am not sure what you meand by "child branch". If I understand > corretly, you have something like: [...] > Maybe, to get a better understanding, you could use visualization tool > like "tig" or "gitk" to observe what happens to your commits (hashes) > and branches (labels) and just play around with some of these > operations. OK, I've followed this advice and looked at the dependency graphs in gitk before and after rebasing, I've managed to obtain what I was after. The repository now has two branches: master and topic. However, Gitk reveals a problem with a string of commits that are not part of any branch: A---B---H---I (master) \ C---D---E (loose string of commits) \ D'---E'---F---G (topic) How do I remove these loose commits (C, D, E)? Thanks for your feedback, -- Seb -- 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/5] README: use markdown syntax
This allows repository browsers like GitHub to display the content of the file nicely formatted. Signed-off-by: Matthieu Moy --- README => README.md | 6 +- t/t7001-mv.sh | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) rename README => README.md (93%) diff --git a/README b/README.md similarity index 93% rename from README rename to README.md index 1083735..907eb3b 100644 --- a/README +++ b/README.md @@ -1,8 +1,4 @@ - - - Git - the stupid content tracker - - +# Git - the stupid content tracker "git" can mean anything, depending on your mood. diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 51dd2b4..4008fae 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -102,7 +102,7 @@ test_expect_success \ test_expect_success \ 'adding another file' \ -'cp "$TEST_DIRECTORY"/../README path0/README && +'cp "$TEST_DIRECTORY"/../README.md path0/README && git add path0/README && git commit -m add2 -a' -- 2.7.2.334.g35ed2ae.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] README.md: move the link to git-scm.com up
The documentation available on git-scm.com is nicely formatted. It's
better to point users to it than to the source code of the
documentation.
Signed-off-by: Matthieu Moy
---
README.md | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/README.md b/README.md
index 750fdda..1625352 100644
--- a/README.md
+++ b/README.md
@@ -22,6 +22,9 @@ Torvalds with help of a group of hackers around the net.
Please read the file [INSTALL][] for installation instructions.
+Many Git online resources are accessible from http://git-scm.com/
+including full documentation and Git related tools.
+
See [Documentation/gittutorial.txt][] to get started, then see
[Documentation/giteveryday.txt][] for a useful minimum set of commands, and
[Documentation/git-commandname.txt][] for documentation of each command.
@@ -34,9 +37,6 @@ CVS users may also want to read
[Documentation/gitcvs-migration.txt][]
("man gitcvs-migration" or "git help cvs-migration" if git is
installed).
-Many Git online resources are accessible from http://git-scm.com/
-including full documentation and Git related tools.
-
The user discussion and development of Git take place on the Git
mailing list -- everyone is welcome to post bug reports, feature
requests, comments and patches to git@vger.kernel.org (read
--
2.7.2.334.g35ed2ae.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
