Re: patch for fix build git on Haiku
Hi, diger wrote: This patch fixes build git on Haiku Lovely. May we have your sign-off? (See Documentation/SubmittingPatches for what this means.) Does make test pass? (Just curious --- it's fine if it doesn't, though in that case a list of test failures would be helpful.) Thanks, Jonathan (patch left unsnipped for reference) --- Makefile.orig 2012-10-21 21:32:15.0 + +++ Makefile @@ -1211,6 +1204,16 @@ ifeq ($(uname_S),HP-UX) endif GIT_TEST_CMP = cmp endif +ifeq ($(uname_S),Haiku) +NO_R_TO_GCC_LINKER = YesPlease +NO_LIBGEN_H = YesPlease +NO_MEMMEM = YesPlease +NO_MKSTEMPS = YesPlease +NEEDS_LIBICONV = YesPlease +DEFAULT_EDITOR = nano +PTHREAD_LIBS =-lroot +NO_CROSS_DIRECTORY_HARDLINKS = YesPlease +endif ifeq ($(uname_S),Windows) GIT_VERSION := $(GIT_VERSION).MSVC pathsep = ; -- 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/2] update-index: list supported idx versions and their features
Nguyễn Thái Ngọc Duy wrote: --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -145,7 +145,15 @@ you will need to handle the situation manually. --index-version n:: Write the resulting index out in the named on-disk format version. - The current default version is 2. + Supported versions are 2, 3 and 4. The current default version is 2 + or 3, depending on whether extra features are used, such as + `git add -N`. ++ + Version 4 performs a simple pathname compression that could + reduce index size by 30%-50% on large repositories, which + results in faster load time. Version 4 is relatively young + (first released in 1.8.0 in October 2012). Other Git + implementations may not support it yet. Markup nit: the second paragraph needs to be unindented, or asciidoc will treat it as literal text, including line breaks. Usage nit: s/could/can/ Clarity nit: something like such as JGit and libgit2 after Other Git implementations would make it clearer, at least to my eyes. Aside from that, this looks like a good change. Thanks, Jonathan -- 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 3/2] update-index: list supported idx versions and their features
Nguyễn Thái Ngọc Duy wrote: Oops, bogus indentation in the first 3/2 Oops, I missed this. Ok, ignore the comment on indentation on v1. :) With or without the other changes I suggested, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/2] update-index: list supported idx versions and their features
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Nguyễn Thái Ngọc Duy wrote: + Version 4 performs a simple pathname compression that could + reduce index size by 30%-50% on large repositories, which + results in faster load time. Version 4 is relatively young + (first released in 1.8.0 in October 2012). Other Git + implementations may not support it yet. Usage nit: s/could/can/ I think s/could reduce/reduces/ is even simpler. Yes, true. 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 01/16] git-sh-setup: make usage text consistent
David Aguilar wrote: mergetool, bisect, and other commands that use git-sh-setup print a usage string that is inconsistent with the rest of Git when they are invoked as git $cmd -h. The compiled builtins use the lowercase usage: string but these commands say Usage:. Adjust the shell library to make these consistent. Scripts could be grepping for Usage:, but they are asking for trouble and if anything should check for status 129 instead. And luckily we have been careful about not checking for this in tests. Thanks for the fix. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/16] make usage text consistent in git commands
David Aguilar wrote: git-sh-setup: make usage text consistent git-svn: make usage text consistent git-relink: make usage text consistent [...] Micronit: titles like git-relink: use a lowercase usage: string would make the effect of these patches easier to see in the shortlog. With or without that change, all of these except patches 11 and 12 are Reviewed-by: Jonathan Nieder jrnie...@gmail.com I have no opinion about patches 11 and 12. :) Thanks, Jonathan -- 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 01/19] spell checking
Paul Campbell wrote: Four of the eight original authors now have dead email addresses. As I found out when I started getting the mail bounces when I started sending these patches out. Would it be acceptable for those patches to leave the From line, add a Based-on-patch-by and then sign of myself? It's always nice to get the original author's sign-off, but if you can certify what's stated in the DCO1.1 (from Documentation/SubmittingPatches) then just adding your sign-off is fine. Please still keep the original authorship in that case, and no need to add a Based-on-patch-by line. Thanks, Jonathan -- 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/2] shell: allow 'no-interactive-login' command to disable interactive shell
Hi again, Here's a reroll along the lines described at http://thread.gmane.org/gmane.comp.version-control.git/216229 As before, this series is meant to give users of basic 'git shell' setups a chance to imitate some nice behaviors that GitHub and gitolite offer in more complicated ways. Thanks for your help on it so far. Jonathan Nieder (2): shell doc: emphasize purpose and security model shell: allow customization of interactive login disabled message Documentation/git-shell.txt | 86 + shell.c | 13 +++ 2 files changed, 84 insertions(+), 15 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
[PATCH 1/2] shell doc: emphasize purpose and security model
The original git-shell(1) manpage emphasized that the shell supports only git transport commands. As the shell gained features, that emphasis and focus in the manual has been lost. Bring it back by splitting the manpage into a few short sections and fleshing out each: - SYNOPSIS, describing how the shell gets used in practice - DESCRIPTION, which gives an overview of the purpose and guarantees provided by this restricted shell - COMMANDS, listing supported commands and restrictions on the arguments they accept - INTERACTIVE USE, describing the interactive mode Also add a see also section with related reading. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Changes since v2: - use command -v instead of which in synopsis to subtly reinforce good habits - use user instead of hardcoding git username in synopsis - give up on typesetting git in monospace, since the toolchain doesn't seem to like lonely backticks :/ - clarify change description The actual text is pretty much the same. Documentation/git-shell.txt | 66 ++--- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt index 9b925060..544b21aa 100644 --- a/Documentation/git-shell.txt +++ b/Documentation/git-shell.txt @@ -9,25 +9,61 @@ git-shell - Restricted login shell for Git-only SSH access SYNOPSIS [verse] -'git shell' [-c command argument] +'chsh' -s $(command -v git-shell) user +'git clone' user`@localhost:/path/to/repo.git` +'ssh' user`@localhost` DESCRIPTION --- -A login shell for SSH accounts to provide restricted Git access. When -'-c' is given, the program executes command non-interactively; -command can be one of 'git receive-pack', 'git upload-pack', 'git -upload-archive', 'cvs server', or a command in COMMAND_DIR. The shell -is started in interactive mode when no arguments are given; in this -case, COMMAND_DIR must exist, and any of the executables in it can be -invoked. - -'cvs server' is a special command which executes git-cvsserver. - -COMMAND_DIR is the path $HOME/git-shell-commands. The user must have -read and execute permissions to the directory in order to execute the -programs in it. The programs are executed with a cwd of $HOME, and -argument is parsed as a command-line string. +This is a login shell for SSH accounts to provide restricted Git access. +It permits execution only of server-side Git commands implementing the +pull/push functionality, plus custom commands present in a subdirectory +named `git-shell-commands` in the user's home directory. + +COMMANDS + + +'git shell' accepts the following commands after the '-c' option: + +'git receive-pack argument':: +'git upload-pack argument':: +'git upload-archive argument':: + Call the corresponding server-side command to support + the client's 'git push', 'git fetch', or 'git archive --remote' + request. +'cvs server':: + Imitate a CVS server. See linkgit:git-cvsserver[1]. + +If a `~/git-shell-commands` directory is present, 'git shell' will +also handle other, custom commands by running +`git-shell-commands/command arguments` from the user's home +directory. + +INTERACTIVE USE +--- + +By default, the commands above can be executed only with the '-c' +option; the shell is not interactive. + +If a `~/git-shell-commands` directory is present, 'git shell' +can also be run interactively (with no arguments). If a `help` +command is present in the `git-shell-commands` directory, it is +run to provide the user with an overview of allowed actions. Then a +git prompt is presented at which one can enter any of the +commands from the `git-shell-commands` directory, or `exit` to close +the connection. + +Generally this mode is used as an administrative interface to allow +users to list repositories they have access to, create, delete, or +rename repositories, or change repository descriptions and +permissions. + +SEE ALSO + +ssh(1), +linkgit:git-daemon[1], +contrib/git-shell-commands/README GIT --- -- 1.8.2.rc3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] shell: new no-interactive-login command to print a custom message
If I disable git-shell's interactive mode by removing the
~/git-shell-commands directory, attempts to use 'ssh' in produce a
message intended for the administrator:
$ ssh git@myserver
fatal: Interactive git shell is not enabled.
hint: ~/git-shell-commands should exist and have read and execute
access.
$
That is helpful for the new admin who is wondering What? Why isn't
the git-shell I just set up working?, but once the site setup is
complete, it would be better to give the user a friendly hint that she
is on the right track, like GitHub does.
Hi username! You've successfully authenticated, but
GitHub does not provide shell access.
An appropriate greeting might even include more complex dynamic
information, like gitolite's list of repositories the user has access
to. Add support for a ~/git-shell-commands/no-interactive-login
command that generates an arbitrary greeting. When the user tries to
log in:
* If the file ~/git-shell-commands/no-interactive-login exists,
run no-interactive-login to let the server say what it likes,
then hang up.
* Otherwise, if ~/git-shell-commands/ is present, start an
interactive read-eval-print loop.
* Otherwise, print the usual configuration hint and hang up.
Reported-by: Ethan Reesor firelizz...@gmail.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
Improved-by: Jeff King p...@peff.net
---
v2 jammed this functionality into the help command, which was kind
of silly. Hopefully this version's better.
This is not urgent at all. If it looks like a good change, I'd be
happy to see it be a part of the 1.8.3 cycle.
Thoughts?
Jonathan
Documentation/git-shell.txt | 20
shell.c | 13 +
2 files changed, 33 insertions(+)
diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
index 544b21aa..c35051ba 100644
--- a/Documentation/git-shell.txt
+++ b/Documentation/git-shell.txt
@@ -59,6 +59,26 @@ users to list repositories they have access to, create,
delete, or
rename repositories, or change repository descriptions and
permissions.
+If a `no-interactive-login` command exists, then it is run and the
+interactive shell is aborted.
+
+EXAMPLE
+---
+
+To disable interactive logins, displaying a greeting instead:
++
+
+$ chsh -s /usr/bin/git-shell
+$ mkdir $HOME/git-shell-commands
+$ cat $HOME/git-shell-commands/no-interactive-login \EOF
+#!/bin/sh
+printf '%s\n' Hi $USER! You've successfully authenticated, but I do not
+printf '%s\n' provide interactive shell access.
+exit 128
+EOF
+$ chmod +x $HOME/git-shell-commands/no-interactive-login
+
+
SEE ALSO
ssh(1),
diff --git a/shell.c b/shell.c
index 84b237fe..1429870a 100644
--- a/shell.c
+++ b/shell.c
@@ -6,6 +6,7 @@
#define COMMAND_DIR git-shell-commands
#define HELP_COMMAND COMMAND_DIR /help
+#define NOLOGIN_COMMAND COMMAND_DIR /no-interactive-login
static int do_generic_cmd(const char *me, char *arg)
{
@@ -65,6 +66,18 @@ static void run_shell(void)
{
int done = 0;
static const char *help_argv[] = { HELP_COMMAND, NULL };
+
+ if (!access(NOLOGIN_COMMAND, F_OK)) {
+ /* Interactive login disabled. */
+ const char *argv[] = { NOLOGIN_COMMAND, NULL };
+ int status;
+
+ status = run_command_v_opt(argv, 0);
+ if (status 0)
+ exit(127);
+ exit(status);
+ }
+
/* Print help if enabled */
run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
--
1.8.2.rc3
--
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] git.c: Remove unnecessary new line
Hi,
Michael Fallows wrote:
--- a/git.c
+++ b/git.c
@@ -316,8 +316,7 @@ static void handle_internal_command(int argc, const char
**argv)
{ check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE
},
{ check-ref-format, cmd_check_ref_format },
{ checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
- { checkout-index, cmd_checkout_index,
- RUN_SETUP | NEED_WORK_TREE},
+ { checkout-index, cmd_checkout_index, RUN_SETUP |
NEED_WORK_TREE },
This wrapped line was introduced a while ago (4465f410, checkout-index
needs a working tree, 2007-08-04). It was the first line to wrap, but
it was also the longest line at the time.
Now the longest line is
{ merge-recursive-theirs, cmd_merge_recursive, RUN_SETUP |
NEED_WORK_TREE },
(94 columns), so you are right that consistency would suggest dropping
the line wrapping for checkout-index.
But I find it hard to convince myself that alone is worth the churn.
In what context did you notice this? Is the intent to help scripts to
parse the commands[] list, or to manipulate it while preserving
formatting to avoid distractions? Did you notice the broken line
while reading through and get distracted, or did some syntax
highlighting tool notice the oddity, or something else?
Hope that helps,
Jonathan
--
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] shell: new no-interactive-login command to print a custom message
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: If I disable git-shell's interactive mode by removing the ~/git-shell-commands directory, attempts to use 'ssh' in produce a message intended for the administrator: Sorry, but -ECANTPARSE. s/in produce/produces/ perhaps? Or if you meant ssh in as a verb, then attempts to ssh in to the service produces a message. I dunno. Sloppy of me. Yes, it should say something like this: If I disable git-shell's interactive mode by removing the ~/git-shell-commands directory, attempts to ssh in to the service produce a message intended for the administrator: -- 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] shell: new no-interactive-login command to print a custom message
Ramkumar Ramachandra wrote: Jonathan Nieder wrote: * If the file ~/git-shell-commands/no-interactive-login exists, run no-interactive-login to let the server say what it likes, then hang up. [...] If no-interactive-login doesn't have execute permissions, we'll get an error from here: fatal: cannot exec 'git-shell-commands/no-interactive-login': Permission denied Yep. Intended. Thanks for looking it over, Jonathan -- 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] push test: use test_config when appropriate
Configuration from test_config does not last beyond the end of the current test assertion, making each test easier to think about in isolation. This changes the meaning of some of the tests. For example, currently push with insteadOf passes even if the line setting url.$TRASH.pushInsteadOf is dropped because an url.$TRASH.insteadOf setting leaks in from a previous test. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- t/t5516-fetch-push.sh | 27 ++- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index c31e5c1c..5b89c111 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -203,7 +203,7 @@ test_expect_success 'push with wildcard' ' test_expect_success 'push with insteadOf' ' mk_empty TRASH=$(pwd)/ - git config url.$TRASH.insteadOf trash/ + test_config url.$TRASH.insteadOf trash/ git push trash/testrepo refs/heads/master:refs/remotes/origin/master ( cd testrepo @@ -217,7 +217,7 @@ test_expect_success 'push with insteadOf' ' test_expect_success 'push with pushInsteadOf' ' mk_empty TRASH=$(pwd)/ - git config url.$TRASH.pushInsteadOf trash/ + test_config url.$TRASH.pushInsteadOf trash/ git push trash/testrepo refs/heads/master:refs/remotes/origin/master ( cd testrepo @@ -231,9 +231,9 @@ test_expect_success 'push with pushInsteadOf' ' test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' ' mk_empty TRASH=$(pwd)/ - git config url.trash2/.pushInsteadOf trash/ - git config remote.r.url trash/wrong - git config remote.r.pushurl $TRASH/testrepo + test_config url.trash2/.pushInsteadOf trash/ + test_config remote.r.url trash/wrong + test_config remote.r.pushurl $TRASH/testrepo git push r refs/heads/master:refs/remotes/origin/master ( cd testrepo @@ -489,31 +489,24 @@ test_expect_success 'push with config remote.*.push = HEAD' ' git checkout local git reset --hard $the_first_commit ) - git config remote.there.url testrepo - git config remote.there.push HEAD - git config branch.master.remote there + test_config remote.there.url testrepo + test_config remote.there.push HEAD + test_config branch.master.remote there git push check_push_result $the_commit heads/master check_push_result $the_first_commit heads/local ' -# clean up the cruft left with the previous one -git config --remove-section remote.there -git config --remove-section branch.master - test_expect_success 'push with config remote.*.pushurl' ' mk_test heads/master git checkout master - git config remote.there.url test2repo - git config remote.there.pushurl testrepo + test_config remote.there.url test2repo + test_config remote.there.pushurl testrepo git push there check_push_result $the_commit heads/master ' -# clean up the cruft left with the previous one -git config --remove-section remote.there - test_expect_success 'push with dry-run' ' mk_test heads/master -- 1.8.2.rc3 -- 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] push test: rely on -chaining instead of 'if bad; then echo Oops; fi'
When it is unclear which command from a test has failed, usual
practice these days is to debug by running the test again with sh -x
instead of relying on debugging 'echo' statements.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
t/t5516-fetch-push.sh | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2f1255d4..0695d570 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -22,10 +22,8 @@ mk_test () {
(
for ref in $@
do
- git push testrepo $the_first_commit:refs/$ref || {
- echo Oops, push refs/$ref failure
- exit 1
- }
+ git push testrepo $the_first_commit:refs/$ref ||
+ exit
done
cd testrepo
for ref in $@
@@ -328,13 +326,8 @@ test_expect_success 'push with weak ambiguity (2)' '
test_expect_success 'push with ambiguity' '
mk_test heads/frotz tags/frotz
- if git push testrepo master:frotz
- then
- echo Oops, should have failed
- false
- else
- check_push_result $the_first_commit heads/frotz tags/frotz
- fi
+ test_must_fail git push testrepo master:frotz
+ check_push_result $the_first_commit heads/frotz tags/frotz
'
--
1.8.2.rc3
--
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: Alias pushurl from push rewrites
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Test nits: ... Hope that helps, Jonathan Nieder (3): push test: use test_config where appropriate push test: simplify check of push result push test: rely on -chaining instead of 'if bad; then echo Oops; fi' Are these supposed to be follow-up patches? Preparatory steps that Rob can reroll on top? Something else? Preparatory steps. -- 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] make pathless 'add [-u|-A]' warning less noisy
Hi, Jeff King wrote: The config option added by this patch gives them such an option. I suspect the need for this config option is a sign that the warning is too eager. After all, the whole idea of the change being safe is that it shouldn't make a difference the way people usually use git, no? In other words, how about the following patches? With them applied, hopefully no one would mind even if the warning becomes a fatal error. Looking forward to your thoughts, Jonathan Nieder (4): add: make pathless 'add [-u|-A]' warning a file-global function add: make warn_pathless_add() a no-op after first call add -u: only show pathless 'add -u' warning when changes exist outside cwd add -A: only show pathless 'add -A' warning when changes exist outside cwd builtin/add.c | 142 -- 1 file changed, 99 insertions(+), 43 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
[PATCH 1/4] add: make pathless 'add [-u|-A]' warning a file-global function
Currently warn_pathless_add() is only called directly by cmd_add(),
but that is about to change. Move its definition higher in the file
and pass the --update or --all option name used in its message
through globals instead of function arguments to make it easier to
call without passing values that will not change through the call
chain.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
builtin/add.c | 69 +++
1 file changed, 36 insertions(+), 33 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index ab1c9e8f..a4028eea 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -28,6 +28,41 @@ struct update_callback_data {
int add_errors;
};
+static const char *option_with_implicit_dot;
+static const char *short_option_with_implicit_dot;
+
+static void warn_pathless_add(void)
+{
+ assert(option_with_implicit_dot short_option_with_implicit_dot);
+
+ /*
+* To be consistent with git add -p and most Git
+* commands, we should default to being tree-wide, but
+* this is not the original behavior and can't be
+* changed until users trained themselves not to type
+* git add -u or git add -A. For now, we warn and
+* keep the old behavior. Later, the behavior can be changed
+* to tree-wide, keeping the warning for a while, and
+* eventually we can drop the warning.
+*/
+ warning(_(The behavior of 'git add %s (or %s)' with no path argument
from a\n
+ subdirectory of the tree will change in Git 2.0 and should
not be used anymore.\n
+ To add content for the whole tree, run:\n
+ \n
+ git add %s :/\n
+ (or git add %s :/)\n
+ \n
+ To restrict the command to the current directory, run:\n
+ \n
+ git add %s .\n
+ (or git add %s .)\n
+ \n
+ With the current Git version, the command is restricted to
the current directory.),
+ option_with_implicit_dot, short_option_with_implicit_dot,
+ option_with_implicit_dot, short_option_with_implicit_dot,
+ option_with_implicit_dot, short_option_with_implicit_dot);
+}
+
static int fix_unmerged_status(struct diff_filepair *p,
struct update_callback_data *data)
{
@@ -321,35 +356,6 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
}
-static void warn_pathless_add(const char *option_name, const char *short_name)
{
- /*
-* To be consistent with git add -p and most Git
-* commands, we should default to being tree-wide, but
-* this is not the original behavior and can't be
-* changed until users trained themselves not to type
-* git add -u or git add -A. For now, we warn and
-* keep the old behavior. Later, the behavior can be changed
-* to tree-wide, keeping the warning for a while, and
-* eventually we can drop the warning.
-*/
- warning(_(The behavior of 'git add %s (or %s)' with no path argument
from a\n
- subdirectory of the tree will change in Git 2.0 and should
not be used anymore.\n
- To add content for the whole tree, run:\n
- \n
- git add %s :/\n
- (or git add %s :/)\n
- \n
- To restrict the command to the current directory, run:\n
- \n
- git add %s .\n
- (or git add %s .)\n
- \n
- With the current Git version, the command is restricted to
the current directory.),
- option_name, short_name,
- option_name, short_name,
- option_name, short_name);
-}
-
int cmd_add(int argc, const char **argv, const char *prefix)
{
int exit_status = 0;
@@ -360,8 +366,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
- const char *option_with_implicit_dot = NULL;
- const char *short_option_with_implicit_dot = NULL;
git_config(add_config, NULL);
@@ -392,8 +396,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (option_with_implicit_dot !argc) {
static const char *here[2] = { ., NULL };
if (prefix)
- warn_pathless_add(option_with_implicit_dot,
- short_option_with_implicit_dot);
+ warn_pathless_add();
argc = 1;
argv = here;
}
--
1.8.2.rc3
--
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
[PATCH 2/4] add: make warn_pathless_add() a no-op after first call
Make warn_pathless_add() print its warning the first time it is called
and do nothing if called again. This will make it easier to show the
warning on the fly when a relevant condition is detected without
risking showing it multiple times when multiple such conditions hold.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
builtin/add.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/builtin/add.c b/builtin/add.c
index a4028eea..a424e69d 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -33,8 +33,13 @@ static const char *short_option_with_implicit_dot;
static void warn_pathless_add(void)
{
+ static int shown;
assert(option_with_implicit_dot short_option_with_implicit_dot);
+ if (shown)
+ return;
+ shown = 1;
+
/*
* To be consistent with git add -p and most Git
* commands, we should default to being tree-wide, but
--
1.8.2.rc3
--
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] add -u: only show pathless 'add -u' warning when changes exist outside cwd
A common workflow in large projects is to chdir into a subdirectory of
interest and only do work there:
cd src
vi foo.c
make test
git add -u
git commit
The upcoming change to 'git add -u' behavior would not affect such a
workflow: when the only changes present are in the current directory,
'git add -u' will add all changes, and whether that happens via an
implicit . or implicit :/ parameter is an unimportant
implementation detail.
The warning about use of 'git add -u' with no pathspec is annoying
because it serves no purpose in this case. So suppress the warning
unless there are changes outside the cwd that are not being added.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
builtin/add.c | 51 ---
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index a424e69d..f05ec1c1 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -89,6 +89,22 @@ static int fix_unmerged_status(struct diff_filepair *p,
return DIFF_STATUS_MODIFIED;
}
+static void warn_if_outside_pathspec(struct diff_queue_struct *q,
+struct diff_options *opt, void *cbdata)
+{
+ int i;
+ const char **pathspec = cbdata;
+
+ for (i = 0; i q-nr; i++) {
+ const char *path = q-queue[i]-one-path;
+
+ if (!match_pathspec(pathspec, path, strlen(path), 0, NULL)) {
+ warn_pathless_add();
+ return;
+ }
+ }
+}
+
static void update_callback(struct diff_queue_struct *q,
struct diff_options *opt, void *cbdata)
{
@@ -121,20 +137,26 @@ static void update_callback(struct diff_queue_struct *q,
}
}
-int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+static void diff_files_with_callback(const char *prefix, const char **pathspec,
+diff_format_fn_t callback, void *data)
{
- struct update_callback_data data;
struct rev_info rev;
init_revisions(rev, prefix);
setup_revisions(0, NULL, rev, NULL);
init_pathspec(rev.prune_data, pathspec);
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
- rev.diffopt.format_callback = update_callback;
- data.flags = flags;
- data.add_errors = 0;
- rev.diffopt.format_callback_data = data;
+ rev.diffopt.format_callback = callback;
+ rev.diffopt.format_callback_data = data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
run_diff_files(rev, DIFF_RACY_IS_MODIFIED);
+}
+
+int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
+{
+ struct update_callback_data data;
+ data.flags = flags;
+ data.add_errors = 0;
+ diff_files_with_callback(prefix, pathspec, update_callback, data);
return !!data.add_errors;
}
@@ -371,6 +393,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
+ int implicit_dot = 0;
git_config(add_config, NULL);
@@ -400,10 +423,11 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
}
if (option_with_implicit_dot !argc) {
static const char *here[2] = { ., NULL };
- if (prefix)
+ if (prefix addremove)
warn_pathless_add();
argc = 1;
argv = here;
+ implicit_dot = 1;
}
add_new_files = !take_worktree_changes !refresh_only;
@@ -450,6 +474,19 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
goto finish;
}
+ /*
+* Check if git add -A or git add -u was run from a
+* subdirectory with a modified file outside that directory,
+* and warn if so.
+*
+* git add -u will behave like git add -u :/ instead of
+* git add -u . in the future. This warning prepares for
+* that change.
+*/
+ if (implicit_dot)
+ diff_files_with_callback(prefix, NULL,
+ warn_if_outside_pathspec, pathspec);
+
if (pathspec) {
int i;
struct path_exclude_check check;
--
1.8.2.rc3
--
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] add -A: only show pathless 'add -A' warning when changes exist outside cwd
In the spirit of the recent similar change for 'git add -u', avoid
pestering users that restrict their attention to a subdirectory and
will not be affected by the coming change in the behavior of pathless
'git add -A'.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
builtin/add.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index f05ec1c1..56ac4519 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -160,7 +160,9 @@ int add_files_to_cache(const char *prefix, const char
**pathspec, int flags)
return !!data.add_errors;
}
-static char *prune_directory(struct dir_struct *dir, const char **pathspec,
int prefix)
+#define WARN_IMPLICIT_DOT (1u 0)
+static char *prune_directory(struct dir_struct *dir, const char **pathspec,
+int prefix, unsigned flag)
{
char *seen;
int i, specs;
@@ -177,6 +179,16 @@ static char *prune_directory(struct dir_struct *dir, const
char **pathspec, int
if (match_pathspec(pathspec, entry-name, entry-len,
prefix, seen))
*dst++ = entry;
+ else if (flag WARN_IMPLICIT_DOT)
+ /*
+* git add -A was run from a subdirectory with a
+* new file outside that directory.
+*
+* git add -A will behave like git add -A :/
+* instead of git add -A . in the future.
+* Warn about the coming behavior change.
+*/
+ warn_pathless_add();
}
dir-nr = dst - dir-entries;
add_pathspec_matches_against_index(pathspec, seen, specs);
@@ -423,8 +435,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
}
if (option_with_implicit_dot !argc) {
static const char *here[2] = { ., NULL };
- if (prefix addremove)
- warn_pathless_add();
argc = 1;
argv = here;
implicit_dot = 1;
@@ -464,9 +474,10 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
}
/* This picks up the paths that are not tracked */
- baselen = fill_directory(dir, pathspec);
+ baselen = fill_directory(dir, implicit_dot ? NULL : pathspec);
if (pathspec)
- seen = prune_directory(dir, pathspec, baselen);
+ seen = prune_directory(dir, pathspec, baselen,
+ implicit_dot ? WARN_IMPLICIT_DOT : 0);
}
if (refresh_only) {
--
1.8.2.rc3
--
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/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd
Junio C Hamano wrote:
The implemenation appears to run an extra diff_files() in addition
to the one we already have to run in order to implement the add -u
to collect modified/deleted paths. Is that the best we can do?
I am wondering if we can special case the no-pathspec case to have
add_files_to_cache() call underlying run_diff_files() without any
pathspec, inspect the paths that are modified and/or deleted in the
update_callback, add ones that are under the $prefix while noticing
the ones outside as warning worthy events.
Yes, that can work, for example like this (replacing the patch you're
replying to).
-- 8 --
Subject: add -u: only show pathless 'add -u' warning when changes exist outside
cwd
A common workflow in large projects is to chdir into a subdirectory of
interest and only do work there:
cd src
vi foo.c
make test
git add -u
git commit
The upcoming change to 'git add -u' behavior would not affect such a
workflow: when the only changes present are in the current directory,
'git add -u' will add all changes, and whether that happens via an
implicit . or implicit :/ parameter is an unimportant
implementation detail.
The warning about use of 'git add -u' with no pathspec is annoying
because it serves no purpose in this case. So suppress the warning
unless there are changes outside the cwd that are not being added.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
builtin/add.c | 41 +
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index a424e69d..e3ed5d93 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,6 +26,7 @@ static int take_worktree_changes;
struct update_callback_data {
int flags;
int add_errors;
+ const char **implicit_dot;
};
static const char *option_with_implicit_dot;
@@ -94,10 +95,26 @@ static void update_callback(struct diff_queue_struct *q,
{
int i;
struct update_callback_data *data = cbdata;
+ const char **implicit_dot = data-implicit_dot;
for (i = 0; i q-nr; i++) {
struct diff_filepair *p = q-queue[i];
const char *path = p-one-path;
+
+ /*
+* Check if git add -A or git add -u was run from a
+* subdirectory with a modified file outside that directory,
+* and warn if so.
+*
+* git add -u will behave like git add -u :/ instead of
+* git add -u . in the future. This warning prepares for
+* that change.
+*/
+ if (implicit_dot
+ !match_pathspec(implicit_dot, path, strlen(path), 0, NULL))
{
+ warn_pathless_add();
+ continue;
+ }
switch (fix_unmerged_status(p, data)) {
default:
die(_(unexpected diff status %c), p-status);
@@ -121,17 +138,30 @@ static void update_callback(struct diff_queue_struct *q,
}
}
+#define ADD_CACHE_IMPLICIT_DOT 32
int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
{
struct update_callback_data data;
struct rev_info rev;
+
+ data.flags = flags ~ADD_CACHE_IMPLICIT_DOT;
+ data.add_errors = 0;
+ data.implicit_dot = NULL;
+ if (flags ADD_CACHE_IMPLICIT_DOT) {
+ /*
+* Check for modified files throughout the worktree so
+* update_callback has a chance to warn about changes
+* outside the cwd.
+*/
+ data.implicit_dot = pathspec;
+ pathspec = NULL;
+ }
+
init_revisions(rev, prefix);
setup_revisions(0, NULL, rev, NULL);
init_pathspec(rev.prune_data, pathspec);
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = update_callback;
- data.flags = flags;
- data.add_errors = 0;
rev.diffopt.format_callback_data = data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
run_diff_files(rev, DIFF_RACY_IS_MODIFIED);
@@ -371,6 +401,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
+ int implicit_dot = 0;
git_config(add_config, NULL);
@@ -400,10 +431,11 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
}
if (option_with_implicit_dot !argc) {
static const char *here[2] = { ., NULL };
- if (prefix)
+ if (prefix addremove)
warn_pathless_add();
argc = 1;
argv = here;
+ implicit_dot = 1;
}
add_new_files = !take_worktree_changes !refresh_only;
@@ -416,7 +448,8 @@ int cmd_add
Re: [PATCH 3/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd
Junio C Hamano wrote: The implemenation appears to run an extra diff_files() in addition to the one we already have to run in order to implement the add -u to collect modified/deleted paths. Is that the best we can do? At least the following should be squashed in to avoid wasted effort in the easy case (cwd at top of worktree). Thanks for catching it. diff --git i/builtin/add.c w/builtin/add.c index f05ec1c1..8e4cdfae 100644 --- i/builtin/add.c +++ w/builtin/add.c @@ -483,7 +483,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) * git add -u . in the future. This warning prepares for * that change. */ - if (implicit_dot) + if (prefix implicit_dot) diff_files_with_callback(prefix, NULL, warn_if_outside_pathspec, pathspec); -- 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/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd
Duy Nguyen wrote: My concern is run full-tree diff can be expensive on large repos (one of the reasons the user may choose to work from within a subdirectory). We can exit as soon as we find a difference outside $prefix. But in case we find nothing, we need to diff the whole worktree. Yes. This is an argument for the single-diff approach that Junio suggested, since at least it's not significantly slower than the future default behavior (git add -u :/). Sysadmins and others helping to train users will need to make sure people working on large repos understand that they can use git add -u . to avoid a speed penalty. Thanks for looking it over, Jonathan -- 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/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd
Junio C Hamano wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: No time to review the code now. I thought about implementing something like that, but did not do it because I didn't want the change in the code to be too big. At some point, we'll have to remove the warning and it's easier with my version than with yours. But the damage to the code do not seem too big, so that's probably OK and will actually reduce the pain for some users. Getting these warnings is a *good* thing. You may happen to have no changed path outside the current directory with this particular invocation of git add -u, or you may do, or you may not *even* remember if you touched the paths outside. Training your fingers to type git add -u . without having to even think, is primarily to help the last case. The problem is that these warnings are triggering way too often. It is like the story of the boy who cried wolf: instead of training people to type git add -u ., we are training them to ignore warnings. I personally often find myself in the following situation: $ cd repowithdeepsubdirs/third_party/git $ ... hack hack hack ... $ git add -u The result is a pile of warning text that I cannot convince myself not to ignore because I already *knew* that the only changes present were under the cwd. The old and new git add -u behaviors always have the same effect in this case, so the warning is not relevant to me. So I find myself being trained to ignore the warning. Presumably habitual Java hackers that do their work in a com/long/package/name subdirectory of the toplevel would find this even more annoying. One important exception is that if git add -u :/ is slow, users need to learn to run git add -u . to speed the operation up. But I think that is intuitive already. Running a full-tree diff which slows down the code that decides whether to print a warning is a good way to train people regarding how long to expect a plain git add -u to take. Hoping that clarifies, Jonathan -- 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/4] add -u: only show pathless 'add -u' warning when changes exist outside cwd
Junio C Hamano wrote: Incidentally, I am rebuilding the 'next' by kicking many of the topics back to 'pu' (essentially, only the ones marked as Safe in the latest issue of What's cooking are kept in 'next'), so perhaps we can rebuild the jc/add-2.0-u-A-sans-pathspec topic with your series at the bottom, or something? I do not think I have time to get around to it myself until later in the evening, though. Sounds good. I'll reroll with the use-prefix-not-pathspec tweak and incorporate the current patches from jc/add-2.0-u-A-sans-pathspec. -- 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/6] make pathless 'add [-u|-A]' warning less noisy
As promised, here is a rerolled version of the make pathless 'add [-u|-A]' warning less noisy, incorporating patches from jc/add-2.0-u-A-sans-pathspec. Thanks for your help so far. Just like jc/add-2.0-u-A-sans-pathspec, the only transition in this series is the pull the bandaid kind. That is, there are two steps: 0. the current state, where the warning is a little too noisy 1. the current state but with the warning only firing in cases where the user will be affected by the change to default 'git add -u' behavior 2. no more warning, 'git add -u' defaulting to 'git add -u :/' Patch 1 is the same as in jc/add-2.0-u-A-sans-pathspec, included for reference. Patches 2-5 correspond to the original patches 1-4. Any changes are described after the '---' in each patch. Patch 6 is just the patch from the tip of jc/add-2.0-u-A-sans-pathspec, rebased. It is meant to be applied in the 2.0 cycle. Jeff King (1): t2200: check that add -u limits itself to subdirectory Jonathan Nieder (4): add: make pathless 'add [-u|-A]' warning a file-global function add: make warn_pathless_add() a no-op after first call add -u: only show pathless 'add -u' warning when changes exist outside cwd add -A: only show pathless 'add -A' warning when changes exist outside cwd Junio C Hamano (1): git add: -u/-A now affects the entire working tree Documentation/git-add.txt | 16 +++--- builtin/add.c | 56 --- t/t2200-add-update.sh | 11 ++ 3 files changed, 28 insertions(+), 55 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
[PATCH 1/6] t2200: check that add -u limits itself to subdirectory
From: Jeff King p...@peff.net Date: Thu, 14 Mar 2013 02:44:04 -0400 This behavior is due to change in the future, but let's test it anyway. That helps make sure we do not accidentally switch the behavior too soon while we are working in the area, and it means that we can easily verify the change when we do make it. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Unchanged, only included for reference. t/t2200-add-update.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 4cdebda..c317254 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -80,6 +80,22 @@ test_expect_success 'change gets noticed' ' ' +# Note that this is scheduled to change in Git 2.0, when +# git add -u will become full-tree by default. +test_expect_success 'non-limited update in subdir leaves root alone' ' + ( + cd dir1 + echo even more sub2 + git add -u + ) + cat expect -\EOF + check + top + EOF + git diff-files --name-only actual + test_cmp expect actual +' + test_expect_success SYMLINKS 'replace a file with a symlink' ' rm foo -- 1.8.2.rc3 -- 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/6] add: make pathless 'add [-u|-A]' warning a file-global function
Currently warn_pathless_add() is only called directly by cmd_add(),
but that is about to change. Move its definition higher in the file
and pass the --update or --all option name used in its message
through globals instead of function arguments to make it easier to
call without passing values that will not change through the call
chain.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Unchanged.
builtin/add.c | 69 +++
1 file changed, 36 insertions(+), 33 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index ab1c9e8..a4028ee 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -28,6 +28,41 @@ struct update_callback_data {
int add_errors;
};
+static const char *option_with_implicit_dot;
+static const char *short_option_with_implicit_dot;
+
+static void warn_pathless_add(void)
+{
+ assert(option_with_implicit_dot short_option_with_implicit_dot);
+
+ /*
+* To be consistent with git add -p and most Git
+* commands, we should default to being tree-wide, but
+* this is not the original behavior and can't be
+* changed until users trained themselves not to type
+* git add -u or git add -A. For now, we warn and
+* keep the old behavior. Later, the behavior can be changed
+* to tree-wide, keeping the warning for a while, and
+* eventually we can drop the warning.
+*/
+ warning(_(The behavior of 'git add %s (or %s)' with no path argument
from a\n
+ subdirectory of the tree will change in Git 2.0 and should
not be used anymore.\n
+ To add content for the whole tree, run:\n
+ \n
+ git add %s :/\n
+ (or git add %s :/)\n
+ \n
+ To restrict the command to the current directory, run:\n
+ \n
+ git add %s .\n
+ (or git add %s .)\n
+ \n
+ With the current Git version, the command is restricted to
the current directory.),
+ option_with_implicit_dot, short_option_with_implicit_dot,
+ option_with_implicit_dot, short_option_with_implicit_dot,
+ option_with_implicit_dot, short_option_with_implicit_dot);
+}
+
static int fix_unmerged_status(struct diff_filepair *p,
struct update_callback_data *data)
{
@@ -321,35 +356,6 @@ static int add_files(struct dir_struct *dir, int flags)
return exit_status;
}
-static void warn_pathless_add(const char *option_name, const char *short_name)
{
- /*
-* To be consistent with git add -p and most Git
-* commands, we should default to being tree-wide, but
-* this is not the original behavior and can't be
-* changed until users trained themselves not to type
-* git add -u or git add -A. For now, we warn and
-* keep the old behavior. Later, the behavior can be changed
-* to tree-wide, keeping the warning for a while, and
-* eventually we can drop the warning.
-*/
- warning(_(The behavior of 'git add %s (or %s)' with no path argument
from a\n
- subdirectory of the tree will change in Git 2.0 and should
not be used anymore.\n
- To add content for the whole tree, run:\n
- \n
- git add %s :/\n
- (or git add %s :/)\n
- \n
- To restrict the command to the current directory, run:\n
- \n
- git add %s .\n
- (or git add %s .)\n
- \n
- With the current Git version, the command is restricted to
the current directory.),
- option_name, short_name,
- option_name, short_name,
- option_name, short_name);
-}
-
int cmd_add(int argc, const char **argv, const char *prefix)
{
int exit_status = 0;
@@ -360,8 +366,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
- const char *option_with_implicit_dot = NULL;
- const char *short_option_with_implicit_dot = NULL;
git_config(add_config, NULL);
@@ -392,8 +396,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (option_with_implicit_dot !argc) {
static const char *here[2] = { ., NULL };
if (prefix)
- warn_pathless_add(option_with_implicit_dot,
- short_option_with_implicit_dot);
+ warn_pathless_add();
argc = 1;
argv = here;
}
--
1.8.2.rc3
--
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
[PATCH 3/6] add: make warn_pathless_add() a no-op after first call
Make warn_pathless_add() print its warning the first time it is called
and do nothing if called again. This will make it easier to show the
warning on the fly when a relevant condition is detected without
risking showing it multiple times when multiple such conditions hold.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
As before.
builtin/add.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/builtin/add.c b/builtin/add.c
index a4028ee..a424e69 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -33,8 +33,13 @@ static const char *short_option_with_implicit_dot;
static void warn_pathless_add(void)
{
+ static int shown;
assert(option_with_implicit_dot short_option_with_implicit_dot);
+ if (shown)
+ return;
+ shown = 1;
+
/*
* To be consistent with git add -p and most Git
* commands, we should default to being tree-wide, but
--
1.8.2.rc3
--
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/6] add -u: only show pathless 'add -u' warning when changes exist outside cwd
A common workflow in large projects is to chdir into a subdirectory of
interest and only do work there:
cd src
vi foo.c
make test
git add -u
git commit
The upcoming change to 'git add -u' behavior would not affect such a
workflow: when the only changes present are in the current directory,
'git add -u' will add all changes, and whether that happens via an
implicit . or implicit :/ parameter is an unimportant
implementation detail.
The warning about use of 'git add -u' with no pathspec is annoying
because it seemingly serves no purpose in this case. So suppress the
warning unless there are changes outside the cwd that are not being
added.
A previous version of this patch ran two I/O-intensive diff-files
passes: one to find changes outside the cwd, and another to find
changes to add to the index within the cwd. This version runs one
full-tree diff and decides for each change whether to add it or warn
and suppress it in update_callback. As a result, even on very large
repositories git add -u will not be significantly slower than the
future default behavior (git add -u :/), and the slowdown relative
to git add -u . should be a useful clue to users of such
repositories to get into the habit of explicitly passing '.'.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
Acked-by: Jeff King p...@peff.net
Improved-by: Junio C Hamano gits...@pobox.com
---
This is the interesting one.
Changes since v1:
* run only one diff-files pass, as explained in the log message
* use strncmp_icase, not match_pathspec, to check if paths are
under cwd
* expand log message with performance implications
* summarized Peff's review with an Ack. I hope that's ok.
Thanks for your help getting this into good shape.
builtin/add.c | 43 +++
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index a424e69..4d8d441 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,6 +26,8 @@ static int take_worktree_changes;
struct update_callback_data {
int flags;
int add_errors;
+ const char *implicit_dot;
+ size_t implicit_dot_len;
};
static const char *option_with_implicit_dot;
@@ -94,10 +96,27 @@ static void update_callback(struct diff_queue_struct *q,
{
int i;
struct update_callback_data *data = cbdata;
+ const char *implicit_dot = data-implicit_dot;
+ size_t implicit_dot_len = data-implicit_dot_len;
for (i = 0; i q-nr; i++) {
struct diff_filepair *p = q-queue[i];
const char *path = p-one-path;
+
+ /*
+* Check if git add -A or git add -u was run from a
+* subdirectory with a modified file outside that directory,
+* and warn if so.
+*
+* git add -u will behave like git add -u :/ instead of
+* git add -u . in the future. This warning prepares for
+* that change.
+*/
+ if (implicit_dot
+ strncmp_icase(path, implicit_dot, implicit_dot_len)) {
+ warn_pathless_add();
+ continue;
+ }
switch (fix_unmerged_status(p, data)) {
default:
die(_(unexpected diff status %c), p-status);
@@ -121,17 +140,30 @@ static void update_callback(struct diff_queue_struct *q,
}
}
+#define ADD_CACHE_IMPLICIT_DOT 32
int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
{
struct update_callback_data data;
struct rev_info rev;
+
+ memset(data, 0, sizeof(data));
+ data.flags = flags ~ADD_CACHE_IMPLICIT_DOT;
+ if ((flags ADD_CACHE_IMPLICIT_DOT) prefix) {
+ /*
+* Check for modified files throughout the worktree so
+* update_callback has a chance to warn about changes
+* outside the cwd.
+*/
+ data.implicit_dot = prefix;
+ data.implicit_dot_len = strlen(prefix);
+ pathspec = NULL;
+ }
+
init_revisions(rev, prefix);
setup_revisions(0, NULL, rev, NULL);
init_pathspec(rev.prune_data, pathspec);
rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = update_callback;
- data.flags = flags;
- data.add_errors = 0;
rev.diffopt.format_callback_data = data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
run_diff_files(rev, DIFF_RACY_IS_MODIFIED);
@@ -371,6 +403,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
+ int implicit_dot = 0;
git_config(add_config, NULL);
@@ -400,10 +433,11 @@ int cmd_add(int argc, const char **argv, const char
[PATCH 5/6] add -A: only show pathless 'add -A' warning when changes exist outside cwd
In the spirit of the recent similar change for 'git add -u', avoid
pestering users that restrict their attention to a subdirectory and
will not be affected by the coming change in the behavior of pathless
'git add -A'.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
As before.
builtin/add.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 4d8d441..2493493 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -170,7 +170,9 @@ int add_files_to_cache(const char *prefix, const char
**pathspec, int flags)
return !!data.add_errors;
}
-static char *prune_directory(struct dir_struct *dir, const char **pathspec,
int prefix)
+#define WARN_IMPLICIT_DOT (1u 0)
+static char *prune_directory(struct dir_struct *dir, const char **pathspec,
+int prefix, unsigned flag)
{
char *seen;
int i, specs;
@@ -187,6 +189,16 @@ static char *prune_directory(struct dir_struct *dir, const
char **pathspec, int
if (match_pathspec(pathspec, entry-name, entry-len,
prefix, seen))
*dst++ = entry;
+ else if (flag WARN_IMPLICIT_DOT)
+ /*
+* git add -A was run from a subdirectory with a
+* new file outside that directory.
+*
+* git add -A will behave like git add -A :/
+* instead of git add -A . in the future.
+* Warn about the coming behavior change.
+*/
+ warn_pathless_add();
}
dir-nr = dst - dir-entries;
add_pathspec_matches_against_index(pathspec, seen, specs);
@@ -433,8 +445,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
}
if (option_with_implicit_dot !argc) {
static const char *here[2] = { ., NULL };
- if (prefix addremove)
- warn_pathless_add();
argc = 1;
argv = here;
implicit_dot = 1;
@@ -475,9 +485,10 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
}
/* This picks up the paths that are not tracked */
- baselen = fill_directory(dir, pathspec);
+ baselen = fill_directory(dir, implicit_dot ? NULL : pathspec);
if (pathspec)
- seen = prune_directory(dir, pathspec, baselen);
+ seen = prune_directory(dir, pathspec, baselen,
+ implicit_dot ? WARN_IMPLICIT_DOT : 0);
}
if (refresh_only) {
--
1.8.2.rc3
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] git add: -u/-A now affects the entire working tree
From: Junio C Hamano gits...@pobox.com
As promised in 0fa2eb530fb7 (add: warn when -u or -A is used without
pathspec, 2013-01-28), in Git 2.0, git add -u/-A that is run
without pathspec in a subdirectory updates all updated paths in the
entire working tree, not just the current directory and its
subdirectories.
Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Thanks for reading.
Documentation/git-add.txt | 16 +++
builtin/add.c | 110 --
t/t2200-add-update.sh | 9 +---
3 files changed, 19 insertions(+), 116 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index b0944e5..02b99cb 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -104,10 +104,10 @@ apply to the index. See EDITING PATCHES below.
pathspec. This removes as well as modifies index entries to
match the working tree, but adds no new files.
+
-If no pathspec is given, the current version of Git defaults to
-.; in other words, update all tracked files in the current directory
-and its subdirectories. This default will change in a future version
-of Git, hence the form without pathspec should not be used.
+If no pathspec is given when `-u` option is used, all
+tracked files in the entire working tree are updated (old versions
+of Git used to limit the update to the current directory and its
+subdirectories).
-A::
--all::
@@ -116,10 +116,10 @@ of Git, hence the form without pathspec should not be
used.
entry. This adds, modifies, and removes index entries to
match the working tree.
+
-If no pathspec is given, the current version of Git defaults to
-.; in other words, update all files in the current directory
-and its subdirectories. This default will change in a future version
-of Git, hence the form without pathspec should not be used.
+If no pathspec is given when `-A` option is used, all
+files in the entire working tree are updated (old versions
+of Git used to limit the update to the current directory and its
+subdirectories).
-N::
--intent-to-add::
diff --git a/builtin/add.c b/builtin/add.c
index 2493493..5c0a869 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -26,50 +26,8 @@ static int take_worktree_changes;
struct update_callback_data {
int flags;
int add_errors;
- const char *implicit_dot;
- size_t implicit_dot_len;
};
-static const char *option_with_implicit_dot;
-static const char *short_option_with_implicit_dot;
-
-static void warn_pathless_add(void)
-{
- static int shown;
- assert(option_with_implicit_dot short_option_with_implicit_dot);
-
- if (shown)
- return;
- shown = 1;
-
- /*
-* To be consistent with git add -p and most Git
-* commands, we should default to being tree-wide, but
-* this is not the original behavior and can't be
-* changed until users trained themselves not to type
-* git add -u or git add -A. For now, we warn and
-* keep the old behavior. Later, the behavior can be changed
-* to tree-wide, keeping the warning for a while, and
-* eventually we can drop the warning.
-*/
- warning(_(The behavior of 'git add %s (or %s)' with no path argument
from a\n
- subdirectory of the tree will change in Git 2.0 and should
not be used anymore.\n
- To add content for the whole tree, run:\n
- \n
- git add %s :/\n
- (or git add %s :/)\n
- \n
- To restrict the command to the current directory, run:\n
- \n
- git add %s .\n
- (or git add %s .)\n
- \n
- With the current Git version, the command is restricted to
the current directory.),
- option_with_implicit_dot, short_option_with_implicit_dot,
- option_with_implicit_dot, short_option_with_implicit_dot,
- option_with_implicit_dot, short_option_with_implicit_dot);
-}
-
static int fix_unmerged_status(struct diff_filepair *p,
struct update_callback_data *data)
{
@@ -96,27 +54,11 @@ static void update_callback(struct diff_queue_struct *q,
{
int i;
struct update_callback_data *data = cbdata;
- const char *implicit_dot = data-implicit_dot;
- size_t implicit_dot_len = data-implicit_dot_len;
for (i = 0; i q-nr; i++) {
struct diff_filepair *p = q-queue[i];
const char *path = p-one-path;
- /*
-* Check if git add -A or git add -u was run from a
-* subdirectory with a modified file outside that directory,
-* and warn if so.
-*
-* git add -u will behave like git add -u :/ instead
Re: [PATCH 1/6] remote.c: simplify a bit of code using git_config_string()
Ramkumar Ramachandra wrote:
--- a/remote.c
+++ b/remote.c
@@ -356,9 +356,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
return 0;
branch = make_branch(name, subkey - name);
if (!strcmp(subkey, .remote)) {
- if (!value)
- return config_error_nonbool(key);
- branch-remote_name = xstrdup(value);
+ git_config_string(branch-remote_name, key, value);
Shouldn't this say
if (git_config_string(branch-remote_name, key, value))
return -1;
or something?
Thanks,
Jonathan
--
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/6] t5516 (fetch-push): update test description
Ramkumar Ramachandra wrote: --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='fetching and pushing, with or without wildcard' +test_description='fetching and pushing' I'm not thrilled with the description before or after. Would it make sense to do something like the following? test_description='Tests of basic fetch/push functionality. These tests create small test repositories and fetch from and push to them, testing: * commandline syntax * refspecs and default refspecs * fast-forward detection and overriding fast-forward detection * configuration (insteadOf, pushInsteadOf, [remote name] push, etc) * hooks * --porcelain output format * hiderefs ' -- 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/6] t5516 (fetch-push): introduce mk_test_with_name()
Ramkumar Ramachandra wrote:
mk_test() creates a repository with the constant name testrepo, and
this may be limiting for tests that need to create more than one
repository for testing. To fix this, create a new mk_test_with_name()
which accepts the repository name as $1. Reimplement mk_test() as a
special case of this function, making sure that no tests need to be
rewritten.
Why not give mk_test an optional parameter?
repo_name=${1:-testrepo}
Oh, it is because mk_test already takes arguments naming refs to push.
I suppose the change description could make this clearer.
Why not use mk_test and then rename, like so?
mk_test ...refs...
mv testrepo testrepo-a
mk_test ...refs...
mv testrepo testrepo-b
...
I dunno. The helper functions at the top of this test are already
intimidating, so I guess I am looking for a way to avoid making that
problem worse. One way would be to add an opening comment before
the function definition explaining how it is meant to be used. See
t/test-lib-functions.sh for examples, such as test_cmp.
Hope that helps,
Jonathan
--
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/6] remote.c: introduce a way to have different remotes for fetch/push
Ramkumar Ramachandra wrote: Currently, do_push() in push.c calls remote_get(), which gets the configured remote for fetching and pushing. Replace this call with a call to pushremote_get() instead, a new function that will return the remote configured specifically for pushing. This function tries to work with the string pushremote_name, before falling back to the codepath of remote_get(). This patch has no visible impact, but serves to enable future patches to introduce configuration variables to set this variable. The above description does not make the impact of this change clear to me. Could you give a before-and-after example? How will this internal API change make my life easier as a developer? Hope that helps, Jonathan -- 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/6] remote.c: introduce remote.pushdefault
Ramkumar Ramachandra wrote: This new configuration variable defines the default remote to push to, and overrides `branch.name.remote` for all branches. Micronit: I think this would be easier to explain if it came after patch 6, since then you could say In other words, it is a default for branch.name.pushremote for all branches and readers would not have to wonder Why does the more generic configuration override the more specific one?. My two cents, Jonathan -- 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/6] t5516 (fetch-push): update test description
Ramkumar Ramachandra wrote: When I want to add a test for branch.name.pushremote, I grep for branch.*.pushurl, and open files with sensible names; I'm not going to open up the file and read a long description of what tests it already contains. Huh? The test_description is output for ./t5516-* --help and is supposed to help people hacking on the test to understand its setup and its purpose. -- 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] t: don't redefine test_config() in various places
Ramkumar Ramachandra wrote: Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- Thanks, Junio. t/t4018-diff-funcname.sh | 5 - t/t7810-grep.sh | 5 - t/t7811-grep-open.sh | 5 - 3 files changed, 15 deletions(-) Yeah, that looks like all of them. FWIW, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
Ramkumar Ramachandra wrote: Jonathan Nieder wrote: I dunno. The helper functions at the top of this test are already intimidating, so I guess I am looking for a way to avoid making that problem worse. [...] My patch does not make the situation worse in any way: Um, yes it does. It adds another function to learn to an already intimidating list. Hoping that clarifies, Jonathan -- 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] Documentation: merging a tag is a special case
Yann Droneaud wrote: When asking Git to merge a tag (such as a signed tag or annotated tag), it will always create a merge commit even if fast-forward was possible. It's like having --no-ff present on the command line. Thanks. This looks good, modulo some nitpicks. [...] --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -170,6 +170,15 @@ happens: If you tried a merge which resulted in complex conflicts and want to start over, you can recover with `git merge --abort`. +MERGING TAG +--- + +When merging a tag (annotated or signed), Git will create a merge commit How about something like When merging an annotated or signed tag or When merging an annotated (and possibly signed) tag? The above text can be misread as meaning When merging any tag, no matter whether it is annotated or signed, which is needlessly confusing for people who don't know about unannotated tags. [...] --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -26,7 +26,7 @@ set to `no` at the beginning of them. --ff:: When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default - behavior. + behavior (except when merging a tag). s/a tag/an annotated tag/ here as well. By the way, what about the possibility of dropping this implicit --no-ff? I think Linus could get used to passing --no-ff explicitly when responding to pull requests. I could go either way on it. It is certainly useful to document the current state before considering changing it, so with the tweaks mentioned above, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
Jeff King wrote: I tend to read the tests in a top-down manner: a test is interesting (usually because it fails), and then I want to see what it is doing, so I look at any functions it calls, and so forth. What I usually find _much_ harder to debug is when there is hidden state leftover from other tests. Thanks for articulating this. I agree that keeping track of state is the hardest part of working with git's tests. To clarify my earlier comment, I was reading the test script from the point of view of someone who wants to add an additional test, rather than someone debugging an existing one. That person has a difficult task: she needs to understand * What do the existing tests in the script do? This information is important in deciding whether the new test will be redundant. * How do I work with the particular dialect used in the script, including helpers? A new test should fit in with the style of its surroundings to avoid contributing to an unmaintainable mess. * What is the intended scope of the test script? Does my new test belong in this script? * What is the logical progression of the script? What story does this script tell? Where should I insert my test to maintain a logical ordering? * What state that later tests rely on do I need to avoid clobbering? Generally the best way to help such a person is to make the script very simple. In particular, using standard helpers instead of custom functions when appropriate and documenting helpers used to give new readers a quick introduction to the dialect can help a lot. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] rebase -i: edit versus unmerged changes
Ramkumar Ramachandra wrote: Andrew Wong wrote: You can actually rely on rebase to run the appropriate command. Didn't Junio explicitly mention that this is undesirable earlier (from the point of view of `rebase -i` design)? I missed the earlier discussion. Does the documentation (e.g., git-rebase(1)) cover this? I can't think of any reason off-hand not to rely on the DWIMery involved in git rebase --continue. Curious, Jonathan -- 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/4] wt-status: fix possible use of uninitialized variable
Jeff King wrote:
Instead of using the x = x hack, let's handle the default
case in the switch() statement with a die(BUG). That tells
the compiler and any readers of the code exactly what the
function's input assumptions are.
Sounds reasonable.
We could also convert the flag to an enum, which would
provide a compile-time check on the function input.
Unfortunately C permits out-of-bounds values for enums.
[...]
--- a/wt-status.c
+++ b/wt-status.c
@@ -264,7 +264,7 @@ static void wt_status_print_change_data(struct wt_status
*s,
{
struct wt_status_change_data *d = it-util;
const char *c = color(change_type, s);
- int status = status;
+ int status;
char *one_name;
char *two_name;
const char *one, *two;
@@ -292,6 +292,9 @@ static void wt_status_print_change_data(struct wt_status
*s,
}
status = d-worktree_status;
break;
+ default:
+ die(BUG: unhandled change_type %d in
wt_status_print_change_data,
+ change_type);
Micronit: s/unhandled/invalid/.
Thanks,
Jonathan
--
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] Documentation: merging a tag is a special case
Junio C Hamano wrote: +Consequently a request `git merge --ff-only v1.2.3` to merge such a +tag would fail. + +When you want to just integrate with the work leading to the commit +that happens to be tagged, e.g. synchronizing with an upstream +release point, you may not want to make an unnecessary merge commit +especially when you do not have any work on your own. In such a +case, you can unwrap the tag yourself before feeding it to `git +merge`, e.g. + +--- +git fetch origin +git merge [--ff-only] v1.2.3^0 +--- Nice and clear, but doesn't this contradict b5c9f1c1b0ed (merge: do not create a signed tag merge under --ff-only option, 2012-02-05)? -- 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/4] wt-status: fix possible use of uninitialized variable
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: Jeff King wrote: + default: + die(BUG: unhandled change_type %d in wt_status_print_change_data, + change_type); Micronit: s/unhandled/invalid/. I actually think unhandled is more correct for this one; we may add new change_type later in the caller, and we do not want to forget to add a new case arm that handles the new value. Ok. Makes sense. -- 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] Documentation: merging a tag is a special case
Junio C Hamano wrote: Here is a replacement. Looks good. Thanks for taking care of this. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] fast-import: use pointer-to-pointer to keep list tail
Jeff King wrote: This is shorter, idiomatic, and it means the compiler does not get confused about whether our e pointer is valid, letting us drop the e = e hack. Signed-off-by: Jeff King p...@peff.net --- And it fixes an instance of Linus's people do not understand pointers Heh. Yes, looks correct. For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] drop some obsolete x = x compiler warning hacks
Jeff King wrote: And 4.3 was old enough for me to say I do not care if you can run with -Wall -Werror or not, let alone 4.2. Changes like this can only reveal bugs (in git or optimizers) that were hidden before, without regressing actual runtime behavior, so for what it's worth I like them. I think perhaps we should encourage people to use -Wno-error=uninitialized, in addition to cleaning up our code where reasonably recent optimizers reveal it to be confusing. Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] transport: drop int cmp = cmp hack
Jeff King wrote: We probably _don't_ want to apply this one right now. I think we should. gcc 4.6.y warning bugs should be fixed --- there's no need for git to work around them. And anyone affected can easily stop using -Werror (-Werror is not meant for use by non-developers in production). So fwiw Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] drop some int x = x hacks to silence gcc warnings
Jeff King wrote: Two patches to follow. [5/4]: fast-import: clarify inline logic in file_change_m This one is clearly a bug / missing feature in gcc's control flow analysis, but your workaround looks reasonable. [6/4]: run-command: always set failed_errno in start_command Very sane. 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] Documentation: add a README file
Yann Droneaud wrote: Anyway, having a README at the Documentation/ level could also help to explain what to be found in this directory: - user-manual - howto - technical - RelNote - SubmittingPatches - CodingGuidelines - etc. A Documentation/README or Documentation/INDEX in the spirit of Linux's Documentation/00-INDEX could be interesting if it does not bitrot (unlike Linux's 00-INDEX). Presumably that means it would have to be pretty brief. Thanks, Jonathan -- 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] Documentation: merging a tag is a special case
Yann Droneaud wrote: Reviewed-by: Jonathan Nieder jrnie...@gmail.com Yes, I think this is in good shape now. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: feature request - have git honor nested .gitconfig files
Hi Josh, Josh Sharpe wrote: For example, I have my ~/.gitconfig that has one email address in it, but I also have multiple repos inside ~/dev which I want to use a different email address for. The only way to do that now is to edit all of these: ~/dev/*/.git/conf -- and there are lots of them, and new repos get added all the time - and I forget. A couple of ideas using existing git features: - A wrapper script around git init can take care of setting up the shared configuration appropriately based on the repository path. - The extra configuration can be applied on a per-cwd instead of a per-repository basis. Some shells provide a PROMPT_COMMAND facility that can be used to run a command (for example set up environment) each time the prompt is displayed. A PROMPT_COMMAND could set the environment variable EMAIL or GIT_EMAIL based on the value of $PWD. Room for improvement: * A new repository can be created by git init or git clone and the path where the repository will live is not immediately obvious from the command line, so setting up thorough wrappers is not actually that easy. So this sounds like a good place to provide a hook. (It could be called new-repository or something.) * Maintaining configuration per repository to record a rather simple is more complicated than ideal. It would be easier to understand the configuration if ~/.gitconfig could spell out the rule explicitly: [include] path = cond(starts_with($GIT_DIR, ~/dev/), ~/.config/git/dev-config, ~/.config/git/nondev-config) This means supporting an extension language in the config file. It sounds hard to do right, especially considering use cases like User runs into trouble, asks a privileged sysadmin to try running a command in her untrusted repository, but it is worth thinking about how to do. * The Includes facility is annoyingly close to being helpful. An include.path setting from ~/.gitconfig cannot refer to $GIT_DIR by name. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] rebase -i: edit versus unmerged changes
Ramkumar Ramachandra wrote: I'd really to have that final 'git continue' in Git 2.0. Can someone attempt to break up the migration path into manageable logical steps that we can start working on? Is there any deadline or migration path needed? Depending on the design, it might be possible to do without backward-incompatible changes, meaning the migration path is whatever someone feels like implementing first lands first. Hope that helps, Jonathan -- 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/6] remote.c: simplify a bit of code using git_config_string()
Ramkumar Ramachandra wrote: A small segment where handle_config() parses the branch.remote configuration variable can be simplified using git_config_string(). Looks correct. -- 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/6] t5516 (fetch-push): update test description
Ramkumar Ramachandra wrote: --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='fetching and pushing, with or without wildcard' +test_description='fetching and pushing' The description before and after are equally useless. You might as well use the following description: test_description='t5516-fetch-push.sh' (Please don't actually do that.) I gave a sketch of what I think might be a more useful description and got shot down without an explanation I could understand in reply. So, this one needs more work I guess. Hope that helps, Jonathan -- 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/6] t5516 (fetch-push): introduce mk_test_with_name()
Junio C Hamano wrote: I would prefer to see a preparatory patch to teach mk_test/mk_empty to _always_ take the new name (i.e. the result of your patch) and then do whatever new things on top. Yes, that sounds like a good way to go. By the way, I am planning to _not_ look at new stuff today. I'd rather see known recent regressions addressed (and unknown ones discovered and squashed) before we move forward, and I would appreciate if regular contributors did the same. I've been flushing out my thoughts to avoid forgetting them. ;-) Agreed, though. 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 4/6] remote.c: introduce a way to have different remotes for fetch/push
Ramkumar Ramachandra wrote:
This patch has no visible impact, but
serves to enable future patches to introduce configuration variables
to set pushremote_name. For example, you can now do the following in
handle_config():
if (!strcmp(key, remote.pushdefault))
git_config_string(pushremote_name, key, value);
Thanks.
[...]
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport,
int flags)
static int do_push(const char *repo, int flags)
{
int i, errs;
- struct remote *remote = remote_get(repo);
+ struct remote *remote = pushremote_get(repo);
struct remote has url and pushurl fields. What do they mean in the
context of these two accessors? /me is confused.
Is the idea that now I should not use pushurl any more, and that I
should use pushremote_get and use url instead?
Hope that helps,
Jonathan
--
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] merge/pull: verify GPG signatures of commits being merged
Hi, Sebastian Götte wrote: git merge/pull: When --verify-signatures is specified on the command-line of git-merge or git-pull, check whether the commits being merged have good gpg signatures and abort the merge in case they do not. This allows e.g. auto-deployment from untrusted repo hosts. This leaves me pretty nervous. Is there an argument to pass in to specify a keyring with public keys to trust? Without that, it is presumably using ~/.gnupg/trustdb.gpg, which is about trust of identity rather than trust to provide code to run on my machine. :( If there's a good way to avoid that, this looks like a good thing to do, though. Hope that helps, Jonathan -- 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/6] remote.c: introduce a way to have different remotes for fetch/push
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: - struct remote *remote = remote_get(repo); + struct remote *remote = pushremote_get(repo); struct remote has url and pushurl fields. What do they mean in the context of these two accessors? /me is confused. Is the idea that now I should not use pushurl any more, and that I should use pushremote_get and use url instead? [...] At the programming level, you would still ask what the URL to be pushed to to the remote obtained here, and would use pushurl if defined, or url otherwise. Ah, I think I see. It might be more convenient to the caller if pushremote_get returned a remote with url set to the pushurl, but that would prevent sharing the struct with other callers that want that remote for fetching. So instead, the idea is something like remote: support a different default remote for pushing Teach remote_get() to accept an argument FOR_FETCH or FOR_PUSH that determines, when no remote is passed to it, whether to use the default remote for fetching or the default for pushing. The default remote for fetching is stored in the static var default_remote_name, while the default for pushing, if set, is in default_push_remote_name. Currently there is never a different default for pushing set but later patches will change that. If remote_get() gained a new required parameter, that would force all call sites to be examined (even any potential call sites added by new patches in flight) and there would no longer be need for the remote_get_1() function. What do you think? Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: feature request - have git honor nested .gitconfig files
Jeff King wrote: On Fri, Mar 22, 2013 at 11:22:11AM -0700, Jonathan Nieder wrote: It would be easier to understand the configuration if ~/.gitconfig could spell out the rule explicitly: [...] It sounds hard to do right, especially considering use cases like User runs into trouble, asks a privileged sysadmin to try running a command in her untrusted repository, but it is worth thinking about how to do. I'd rather not invent a new language. It will either not be featureful enough, or will end up bloated. Or both. How about something like: [include] exec = case \$GIT_DIR\ in) */dev/*) cat ~/.config/git/dev-config ;; *) cat ~/.config/git/nondev-config ;; esac Yes, an existing language like shell or lua would presumably be the way to go. The scary aspect of shell is the Prankster sets up a malicious configuration, asks a sysadmin to help in her repository scenario. Of course we already have that problem with command aliases, but if the sysadmin has perfect spelling then aliases would never trip, so... Hopefully that's enough information for anyone interested to start and understand the relevant variables at play. Thanks, Jonathan -- 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] checkout: add --sparse for restoring files in sparse checkout mode
Hi, Nguyễn Thái Ngọc Duy wrote: --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -180,6 +180,13 @@ branch by running git rm -rf . from the top level of the working tree. Afterwards you will be ready to prepare your new files, repopulating the working tree, by copying them from elsewhere, extracting a tarball, etc. + +--sparse:: + In sparse checkout mode, `git checkout -- paths` would + update all entries matched by paths regardless sparse + patterns. This option only updates entries matched by paths + and sparse patterns. Hm, should this be the default? In principle, I would expect git checkout -- . to make the worktree match the index, respecting the sparse checkout. And something like git checkout --widen -- . to change the sparse checkout pattern. But of course it is easily possible that I am missing some details of how sparse checkout is used in practice. What do you think? Jonathan -- 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] checkout: add --sparse for restoring files in sparse checkout mode
Duy Nguyen wrote: On Mon, Mar 25, 2013 at 1:17 AM, Jonathan Nieder jrnie...@gmail.com wrote: Hm, should this be the default? In principle, I would expect git checkout -- . to make the worktree match the index, respecting the sparse checkout. And something like git checkout --widen -- . to change the sparse checkout pattern. [...] Changing the default may involve a painful transition phase (e.g. add -u). I don't think it needs to. There aren't many people using sparse checkout even these days, and I think they'd generally be happy about the change. But if we want to be conservative until some later point (like 2.1), perhaps --sparse should be named something like --no-widen? That way, I can do git checkout --no-widen -- . to make the worktree match the index, respecting the sparse checkout. And I can do git checkout --widen -- . to change the sparse checkout pattern. Meanwhile the confusing command git checkout -- . would be ill-defined for sparse checkouts --- in past git versions, if I understand you correctly it acted like --widen, while in some unspecified future version it may change to mean --no-widen. No need for warnings because I doubt anyone is relying on either behavior. Would that work? Jonathan -- 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: Why does 'submodule add' stage the relevant portions?
Ramkumar Ramachandra wrote: Junio C Hamano wrote: Ramkumar Ramachandra artag...@gmail.com writes: Git 2.0 is coming soon, so I'm excited about breaking a lot of backward compatibility ;) Don't. push.default is the necessary exception? A while ago there was a discussion of changes of the If we were starting over today, it would be obvious we should have made it work the other way kind and potential transition plans for them leading up to 2.0. It's way too late to throw new breakage in. More generally, it doesn't make a lot of sense to save thinking about such questions for the last minute before a new major release. If an important change will require breaking compatibility and can only be done using a painful 5-year transition, start today (and send patches to the ML when an appropriate quiet moment comes to get review) so the 5-year counter can start ticking. Hoping that clarifies, Jonathan -- 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: Why does 'submodule add' stage the relevant portions?
Ramkumar Ramachandra wrote: Okay, I'm confused: why are you waiting for 2.0 to change push.default then? Isn't that just a slightly better default at the porcelain level and hence a regular enhancement? No. Among other aspects, git push is used heavily by scripts. -- 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: Why does 'submodule add' stage the relevant portions?
Ramkumar Ramachandra wrote: This whole backward compatibility thing is not black-or-white: it's shades of gray. Can I ask about how backward incompatible the other suggestions I listed were, just so I get a rough idea of your scale? It depends on how important the change is and how painful the proposed transition is. Please don't gratuitously break things. If there is a smooth way to accomplish the intended effect without much downside, that is generally preferred, even if it is harder to write the code. There are no absolutes here. It is about helping people in the real world who never asked for such-and-such feature to avoid suffering real breakage. Hope that helps, Jonathan -- 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 6/9] streaming_write_entry: propagate streaming errors
Jeff King wrote:
When we are streaming an index blob to disk, we store the
error from stream_blob_to_fd in the result variable, and
then immediately overwrite that with the return value of
close.
Good catch.
[...]
--- a/entry.c
+++ b/entry.c
@@ -126,8 +126,10 @@ static int streaming_write_entry(struct cache_entry *ce,
char *path,
fd = open_output_fd(path, ce, to_tempfile);
if (0 = fd) {
result = stream_blob_to_fd(fd, ce-sha1, filter, 1);
- *fstat_done = fstat_output(fd, state, statbuf);
- result = close(fd);
+ if (!result) {
+ *fstat_done = fstat_output(fd, state, statbuf);
+ result = close(fd);
+ }
Should this do something like
{
int fd, result = 0;
fd = open_output_fd(path, ce, to_tempfile);
if (fd 0)
return -1;
result = stream_blob_to_fd(fd, ce-sha1, filter, 1);
if (result)
goto close_fd;
*fstat_done = fstat_output(fd, state, statbuf);
close_fd:
result |= close(fd);
unlink_path:
if (result)
unlink(path);
return result;
}
to avoid leaking the file descriptor?
@@ -31,4 +40,20 @@ test_expect_success 'streaming a corrupt blob fails' '
)
'
+test_expect_success 'read-tree -u detects bit-errors in blobs' '
+ (
+ cd bit-error
+ rm content.t
+ test_must_fail git read-tree --reset -u FETCH_HEAD
+ )
Makes sense. Might make sense to use rm -f instead of rm to avoid
failures if content.t is removed already.
+'
+
+test_expect_success 'read-tree -u detects missing objects' '
+ (
+ cd missing
+ rm content.t
Especially here.
Thanks,
Jonathan
--
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 ate my home directory :-(
Hi, Richard Weinberger wrote: In my scripts I'm setting GIT_DIR to use git-fetch and git-reset without changing the current working directory all the time. Yeah, for historical reasons GIT_WORK_TREE defaults to $(pwd) when GIT_DIR is explicitly set. In git versions including the patch 2cd83d10bb6b (setup: suppress implicit . work-tree for bare repos, 2013-03-08, currently in next but not master), you can set GIT_IMPLICIT_WORK_TREE=0 to avoid this behavior. Thanks for a useful example, and sorry for the trouble. Sincerely, Jonathan -- 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 ate my home directory :-(
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: In git versions including the patch 2cd83d10bb6b (setup: suppress implicit . work-tree for bare repos, 2013-03-08, currently in next but not master), you can set GIT_IMPLICIT_WORK_TREE=0 to avoid this behavior. WAT? Is that false? If I understand the history correctly, the ability to set the GIT_DIR envvar was meant to allow a person to keep their .git directory outside the worktree. So you can do: git init my-favorite-repo cd my-favorite-repo ...work as usual... # cleaning time! mv .git ~/my-favorite-repo-metadata.git GIT_DIR=$HOME/my-favorite-repo-metadata.git; export GIT_DIR ... work as usual... If you want to set GIT_DIR and treat it as a bare repository, the sane way to do that is simply cd ~/my-favorite-bare-repository.git ... use git as usual ... But if something (for example relative paths used by your script) ties your cwd somewhere else, you might really want to do GIT_DIR=~/my-favorite-bare-repository.git; export GIT_DIR ... work as usual ... and as a side effect of Jeff's patch there is now a mechanism to do that: GIT_IMPLICIT_WORK_TREE=0; export GIT_IMPLICIT_WORK_TREE GIT_DIR=~/my-favorite-bare-repository.git; export GIT_DIR ... work as usual ... This is of course unsafe because it ties your usage to a specific version of git. And the variable is not advertised in the documentation. -- 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 ate my home directory :-(
Junio C Hamano wrote: I do not know how things will break when the end user sets and exports it to the environment, and I do not think we would want to make any promise on how it works. That's a reasonable desire, and it means it's a good thing we noticed this before the envvar escaped to master. People *will* use such exposed interfaces unless they are clearly marked as internal. That's just a fact of life. Here's a rough patch to hopefully improve matters. Longer term, it would be nice to have something like GIT_IMPLICIT_WORK_TREE exposed to let scripts cache the result of the search for .git. Maybe something like GIT_BARE=(arbitrary value) would be a good interface. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- diff --git a/cache.h b/cache.h index 59e5b53..8f92b6d 100644 --- a/cache.h +++ b/cache.h @@ -377,7 +377,7 @@ static inline enum object_type object_type(unsigned int mode) * of this, but we use it internally to communicate to sub-processes that we * are in a bare repo. If not set, defaults to true. */ -#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_INTERNAL_IMPLICIT_WORK_TREE /* * Repository-local GIT_* environment variables; these will be cleared -- 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 ate my home directory :-(
Richard Weinberger wrote: Okay, I have to set GIT_DIR _and_ GIT_WORK_TREE to make my scripts safe again? I've always set only GIT_DIR because it just worked (till today...). chdir-ing into the git repo without setting any GIT_* vars is probably the simplest way to go. -- 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 6/9] streaming_write_entry: propagate streaming errors
Jeff King wrote: Both fixed in my re-roll. Thanks! This and the rest of the patches up to and including patch 8 look good to me. I haven't decided what to think about patch 9 yet, but I suspect it would be good, too. In the long term I suspect git clone --worktree-only repo (or some other standard interface for git-new-workdir functionality) is a better way to provide a convenient lightweight same-machine clone anyway. Jonathan -- 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: [DONOTAPPLY PATCH 1/3] environment: set GIT_WORK_TREE when we figure out work tree
Jeff King wrote: --- a/environment.c +++ b/environment.c @@ -194,6 +194,7 @@ void set_git_work_tree(const char *new_work_tree) } git_work_tree_initialized = 1; work_tree = xstrdup(real_path(new_work_tree)); + setenv(GIT_WORK_TREE_ENVIRONMENT, work_tree, 1); } There's no rush, but I think this is a good change. It makes the rest of the codebase more resilient to running commands from a subdir of the top level of the worktree. -- 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: [DONOTAPPLY PATCH 2/3] setup: warn about implicit worktree with $GIT_DIR
Jeff King wrote: On Tue, Mar 26, 2013 at 01:21:42PM -0700, Jonathan Nieder wrote: If we want this warning, would something like the following do? warning: You have set GIT_DIR without setting GIT_WORK_TREE hint: In this case, GIT_WORK_TREE defaults to '.' hint: To suppress this message, set GIT_WORK_TREE='.' That can help by teaching people how GIT_DIR behaves in general. Yes, I think it would have helped in this case. If I understand correctly then for a while Richard was habitually setting GIT_DIR to mean act on this repository and thought the worktree was automatically being set to the containing directory. I think patch 3 is a bad direction to go because there will always be old scripts that follow what used to be the recommended way to use GIT_DIR. In the long term a warning like this that doesn't break them (or a fatal error that at least doesn't confuse them) might be a good way to go. Thanks for your thoughtful work, as always. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/9] clone: leave repo in place after checkout errors
Jeff King wrote:
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -377,10 +377,40 @@ static void remove_junk(void)
static const char *junk_work_tree;
static const char *junk_git_dir;
static pid_t junk_pid;
+enum {
+ JUNK_LEAVE_NONE,
+ JUNK_LEAVE_REPO,
+ JUNK_LEAVE_ALL
+} junk_mode = JUNK_LEAVE_NONE;
Neat.
+
+static const char junk_leave_repo_msg[] =
+N_(The remote repository was cloned successfully, but there was\n
+ an error checking out the HEAD branch. The repository has been left in\n
+ place but the working tree may be in an inconsistent state. You can\n
+ can inspect the contents with:\n
+ \n
+ git status\n
+ \n
+ and retry the checkout with\n
+ \n
+ git checkout -f HEAD\n
+ \n);
Can this be made more precise? I don't know what it means for the
working tree to be in an inconsistent state: do you mean that some files
might be partially checked out or not have been checked out at all yet?
error: Clone succeeded, but checkout failed.
hint: You can inspect what was checked out with git status.
hint: To retry the checkout, run git checkout -f HEAD.
Aside from that, this looks very nice.
Thanks,
Jonathan
--
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: propagating repo corruption across clone
Hi, Rich Fromm wrote: The host executing the clone command is different than the the host on which the remote repository lives, and I am using ssh as a transport protocol. If there is corruption, can I or can I not expect the clone operation to fail and return a non-zero exit value? If I can not expect this, is the workaround to run `git fsck` on the resulting clone? Is the [transfer] fsckObjects configuration on the host executing the clone set to true? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] push: Alias pushurl from push rewrites
Rob Hoelz wrote:
--- a/remote.c
+++ b/remote.c
@@ -465,7 +465,11 @@ static void alias_all_urls(void)
if (!remotes[i])
continue;
for (j = 0; j remotes[i]-pushurl_nr; j++) {
- remotes[i]-pushurl[j] =
alias_url(remotes[i]-pushurl[j], rewrites);
+ char *copy = xstrdup(remotes[i]-pushurl[j]);
+ remotes[i]-pushurl[j] =
alias_url(remotes[i]-pushurl[j], rewrites_push);
+ if (!strcmp(copy, remotes[i]-pushurl[j]))
+ remotes[i]-pushurl[j] =
alias_url(remotes[i]-pushurl[j], rewrites);
+ free(copy);
Interesting.
Suppose I configure
[url git://anongit.myserver.example.com/]
insteadOf = myserver.example.com:
[url myserver:]
pushInsteadOf = myserver.example.com:
The above code would make the insteadOf rule apply instead of
pushInsteadOf, even when pushing. Perhaps something like the
following would work?
const char *url = remotes[i]-pushurl[j];
remotes[i]-pushurl[j] = alias_url(url, rewrites_push);
if (remotes[i]-pushurl[j] == url)
/* No url.*.pushinsteadof configuration
matched. */
remotes[i]-pushurl[j] = alias_url(url,
rewrites);
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -244,6 +244,83 @@ test_expect_success 'push with pushInsteadOf and
explicit pushurl (pushInsteadOf
)
'
+test_expect_success 'push with pushInsteadOf and explicit pushurl (pushurl +
pushInsteadOf does rewrite in this case)' '
+ mk_empty
+ rm -rf ro rw
+ TRASH=$(pwd)/
+ mkdir ro
+ mkdir rw
+ git init --bare rw/testrepo
+ test_config url.file://$TRASH/ro/.insteadOf ro:
+ test_config url.file://$TRASH/rw/.pushInsteadOf rw:
+ test_config remote.r.url ro:wrong
+ test_config remote.r.pushurl rw:testrepo
+ git push r refs/heads/master:refs/remotes/origin/master
+ (
+ cd rw/testrepo
+ echo $the_commit commitrefs/remotes/origin/master
expected
+ git for-each-ref refs/remotes/origin actual
+ test_cmp expected actual
+ )
Looks good. The usual style in git tests is to include no space
after redirection operators:
git for-each-ref refs/remotes/origin actual
Hope that helps,
Jonathan
--
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 and GSoC 2013
Jeff King wrote: There was a big thread about a month ago on whether Git should do Google Summer of Code this year[1]. [...] In my opinion, a lot of the issues come down to project selection; Let me throw in some other issues. :) * I think the git project has been very disorganized in vetting candidate students. Other organizations have formal requirements (for example, must submit at least one properly formatted patch to qualify) but we seem to rely on a candidate's good sense, independence, and general sense of trustworthiness without providing guidance beyond that. At first glance that wouldn't seem to be a problem --- the accepted students have been very good anyway --- but I think that if we could communicate more clearly what we need, we might find there are more qualified students that we have been missing, and promising students might end up working a little in advance of GSoC to adapt themselves to the project. * Similarly, we are not very good at making clear the expectations for students during the program and making sure they are met. At least I know I was lousy about this as a mentor. For example, students delay too long before posting patches on-list and do not ask for help quickly when they are stuck. By the end of the summer they may start to get a sense of the usual contribution workflow when they could have been more effective by following it from the start. Some organizations require (as a non-negotiable rule) regular blog posts from their students, as a way of advertising to others what work they are doing and how to help them out. That could help here. * We didn't plan in advance for What happens when summer ends and the students don't have free time any more? * We don't advertise any good recourse available to students if a mentor is unexpectedly too busy or hard to contact. I don't know if that's happened in practice. Matthieu Moy's summer projects worked better in all these respects, I think. I don't think we should apply. Better to take a break and prepare for next time. My two cents, Jonathan -- 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: Composing git repositories
Ramkumar Ramachandra wrote: Even then, working with one worktree embedded inside another is something git never designed for: it explains why I have to literally fight with git when using submodules Do you mean that you wish you could ignore subrepository boundaries and use commands like git clone --recurse-submodules http://git.zx2c4.com/cgit cd cgit vi git/cache.h ... edit edit edit ... git add --recurse-submodules git/cache.h git commit --recurse-submodules git push --recurse-submodules , possibly with configuration to allow the --recurse-submodules to be implied, and have everything work out well? I think something like that is a goal for submodules in the long term, with a caveat that there are complications in that different projects (the parent project and subproject) can have different contribution guidelines, review and release schedules, and so on. If submodules are not working for you today, you may find some of Jens's submodule improvement patches interesting, or you may want to look into alternatives that make different assumptions, such as entirely independent repositories and tools like mr that iterate over them. Hope that helps, Jonathan -- 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
jn/add-2.0-u-A-sans-pathspec (Re: What's cooking in git.git (Mar 2013, #07; Tue, 26))
Junio C Hamano wrote:
On Tue, Mar 26, 2013 at 03:40:00PM -0700, Junio C Hamano wrote:
* jn/add-2.0-u-A-sans-pathspec (2013-03-20) 5 commits
- git add: -u/-A now affects the entire working tree
- add -A: only show pathless 'add -A' warning when changes exist outside cwd
- add -u: only show pathless 'add -u' warning when changes exist outside cwd
- add: make warn_pathless_add() a no-op after first call
- add: make pathless 'add [-u|-A]' warning a file-global function
Replaces jc/add-2.0-u-A-sans-pathspec topic by not warning against
add -u/-A that is ran without pathspec when there is no change
outside the current directory.
I recall we had a lengthy discussion on this, but how committed are
we on the progression of this series? Are the bottom four ready to
be merged to 1.8.3, or do they need more polishing?
I wanted to add tests and then other tasks took over. Sorry. Probably
best to get the bottom four in next and add tests on top later.
I have the following squashed in locally.
-- 8 --
Subject: fixup! add -u: only show pathless 'add -u' warning when changes exist
outside cwd
Define ADD_CACHE_IMPLICIT_DOT in cache.h alongside the other add_to_index
flags. This way, authors of patches adding new flags that might want to
use the same bit can know to be careful.
Requested-by: Jeff King p...@peff.net
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Thanks,
Jonathan
builtin/add.c | 1 -
cache.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/add.c b/builtin/add.c
index ad59182..9f35df7 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -139,7 +139,6 @@ static void update_callback(struct diff_queue_struct *q,
}
}
-#define ADD_CACHE_IMPLICIT_DOT 32
int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
{
struct update_callback_data data;
diff --git a/cache.h b/cache.h
index e493563..5de3480 100644
--- a/cache.h
+++ b/cache.h
@@ -459,6 +459,7 @@ extern int remove_file_from_index(struct index_state *,
const char *path);
#define ADD_CACHE_IGNORE_ERRORS4
#define ADD_CACHE_IGNORE_REMOVAL 8
#define ADD_CACHE_INTENT 16
+#define ADD_CACHE_IMPLICIT_DOT 32 /* internal to git add -u/-A */
extern int add_to_index(struct index_state *, const char *path, struct stat *,
int flags);
extern int add_file_to_index(struct index_state *, const char *path, int
flags);
extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned
char *sha1, const char *path, int stage, int refresh);
--
1.8.2.rc3
--
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: Alias pushurl from push rewrites
Rob Hoelz wrote: My mistake; I had not seen it! I thought you may have found a bug in my implementation, so I wanted to double check. =) Well, I had found an unfortunate consequence of the implementation that uses an unnecessary copy. :) Will follow up to the updated patch. -- 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: Alias pushurl from push rewrites
Josh Triplett wrote: I have a .gitconfig in my git-managed home directory which sets pushInsteadOf so that I can clone via git:// and immediately have working push. I work with a number of systems that don't have inbound access to each other but do have outbound access to the network; on some of these satellite boxes, I can't push changes directly to the server pushInsteadOf points to, so I can explicitly set pushurl in .git/config for that repository, which overrides the pushInsteadOf. This change would break that configuration. Would it? As long as your pushurl does not start with git://, I think your configuration would still work fine. After this patch, neither pushInsteadOf nor pushUrl overrides the other one. The rule is: 1. First, get the URL from the remote's configuration, based on whether you are fetching or pushing. (At this step, in your setup git chooses the URL specified with pushurl in your .git/config.) 2. Next, apply the most appropriate url.*.insteadOf or url.*.pushInsteadOf rule, based on whether you are fetching or pushing. (At this step, no rewrite rules apply, so the URL is used as is.) -- 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] branch: add -u as a shortcut for --set-upstream
Hi Carlos,
Carlos Martín Nieto wrote:
Add this shortcut just like git-push has it.
[...]
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -724,7 +724,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT__QUIET(quiet, suppress informational messages),
OPT_SET_INT('t', track, track, set up tracking mode (see
git-pull(1)),
BRANCH_TRACK_EXPLICIT),
- OPT_SET_INT( 0, set-upstream, track, change upstream info,
+ OPT_SET_INT('u', set-upstream, track, change upstream
info,
BRANCH_TRACK_OVERRIDE),
I think this is a bad idea. The --set-upstream option is confusing:
$ git branch --set-upstream=foo
error: option 'set-upstream' takes no value
$ ???
-u to set the corresponding upstream branch at the same time as
creating a branch, analagous to git push -u might seem intuitive:
# create foo branch, setting its upstream at the same time
git branch -u foo origin/foo
But that is what --track does and is not what --set-upstream is for.
Unlike --track, I don't think --set-upstream is a common enough
operation to deserve a one-letter synonym.
Instead, I'd suggest the following changes:
1) Add support for
# change upstream info
git branch --set-upstream=origin/foo foo
for existing branches only.
2) Introduce an --unset-upstream option which removes the
corresponding upstream branch configuration for an existing
branch.
3) Warn on
# acts just like --track
git branch --set-upstream foo origin/foo
for branches that do not exist yet. Plan to make this a hard
error in the future.
4) Warn on
# sets upstream for foo to the current branch
git branch --set-upstream foo
and plan to make it a hard error in the future.
5) Warn on
git branch --set-upstream origin/foo foo
which is almost certainly a typo for
git branch --set-upstream=origin/foo foo
6) Perhaps, make -u a synonym for -t for consistency with git push.
What do you think?
Jonathan
--
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 PULL] vcs-svn housekeeping
Hi Junio, The following changes since commit 58ebd9865d2bb9d42842fbac5a1c4eae49e92859: vcs-svn/svndiff.c: squelch false unused warning from gcc (2012-01-27 11:58:56 -0800) are available at: git://repo.or.cz/git/jrn.git svn-fe The first three commits duplicate changes that are already in master but were committed independently on the svn-fe branch last February. The rest are David's db/vcs-svn series which aims to address various nits noticed when merging the code back into svn-dump-fast-export: unnecessary use of git-specific functions (prefixcmp, memmem) and warnings reported by clang. Some of the patches had to change a little since v2 of db/vcs-svn, so I'll be replying with a copy of the patches for reference. David has looked the branch over and acked and tested it. Thoughts welcome, as usual. I think these are ready for pulling into master. Sorry to be so slow at this. David Barr (7): vcs-svn: drop no-op reset methods vcs-svn: avoid self-assignment in dummy initialization of pre_off vcs-svn: simplify cleanup in apply_one_window vcs-svn: use constcmp instead of prefixcmp vcs-svn: use strstr instead of memmem vcs-svn: suppress signed/unsigned comparison warnings vcs-svn: suppress a signed/unsigned comparison warning Jonathan Nieder (4): vcs-svn: allow import of 4GiB files vcs-svn: suppress -Wtype-limits warning vcs-svn: suppress a signed/unsigned comparison warning vcs-svn: allow 64-bit Prop-Content-Length Ramsay Allan Jones (1): vcs-svn: rename check_overflow and its arguments for clarity test-line-buffer.c |1 - test-svn-fe.c|2 -- vcs-svn/fast_export.c| 26 -- vcs-svn/fast_export.h|5 ++--- vcs-svn/line_buffer.c|4 vcs-svn/line_buffer.h|1 - vcs-svn/sliding_window.c | 16 vcs-svn/svndiff.c| 15 --- vcs-svn/svndump.c| 46 -- 9 files changed, 58 insertions(+), 58 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
[PATCH 1/9] vcs-svn: drop no-op reset methods
From: David Barr davidb...@google.com
Date: Fri, 1 Jun 2012 00:41:30 +1000
Since v1.7.5~42^2~6 (vcs-svn: remove buffer_read_string)
buffer_reset() does nothing thus fast_export_reset() also.
Signed-off-by: David Barr davidb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Incorporates fixes from $gmane/198955. No other changes.
test-line-buffer.c|1 -
test-svn-fe.c |2 --
vcs-svn/fast_export.c |5 -
vcs-svn/fast_export.h |1 -
vcs-svn/line_buffer.c |4
vcs-svn/line_buffer.h |1 -
vcs-svn/svndump.c |2 --
7 files changed, 16 deletions(-)
diff --git a/test-line-buffer.c b/test-line-buffer.c
index 7ec9b13c..ef1d7bae 100644
--- a/test-line-buffer.c
+++ b/test-line-buffer.c
@@ -87,6 +87,5 @@ int main(int argc, char *argv[])
die(input error);
if (ferror(stdout))
die(output error);
- buffer_reset(stdin_buf);
return 0;
}
diff --git a/test-svn-fe.c b/test-svn-fe.c
index 332a5f71..83633a21 100644
--- a/test-svn-fe.c
+++ b/test-svn-fe.c
@@ -31,9 +31,7 @@ static int apply_delta(int argc, char *argv[])
die_errno(cannot close preimage);
if (buffer_deinit(delta))
die_errno(cannot close delta);
- buffer_reset(preimage);
strbuf_release(preimage_view.buf);
- buffer_reset(delta);
return 0;
}
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index b823b851..b4be91cc 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -42,11 +42,6 @@ void fast_export_deinit(void)
die_errno(error closing fast-import feedback stream);
}
-void fast_export_reset(void)
-{
- buffer_reset(report_buffer);
-}
-
void fast_export_delete(const char *path)
{
putchar('D');
diff --git a/vcs-svn/fast_export.h b/vcs-svn/fast_export.h
index aa629f54..8823aca1 100644
--- a/vcs-svn/fast_export.h
+++ b/vcs-svn/fast_export.h
@@ -6,7 +6,6 @@ struct line_buffer;
void fast_export_init(int fd);
void fast_export_deinit(void);
-void fast_export_reset(void);
void fast_export_delete(const char *path);
void fast_export_modify(const char *path, uint32_t mode, const char *dataref);
diff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c
index 01fcb842..57cc1cec 100644
--- a/vcs-svn/line_buffer.c
+++ b/vcs-svn/line_buffer.c
@@ -124,7 +124,3 @@ off_t buffer_skip_bytes(struct line_buffer *buf, off_t
nbytes)
}
return done;
}
-
-void buffer_reset(struct line_buffer *buf)
-{
-}
diff --git a/vcs-svn/line_buffer.h b/vcs-svn/line_buffer.h
index 8901f214..ee23b4f4 100644
--- a/vcs-svn/line_buffer.h
+++ b/vcs-svn/line_buffer.h
@@ -14,7 +14,6 @@ struct line_buffer {
int buffer_init(struct line_buffer *buf, const char *filename);
int buffer_fdinit(struct line_buffer *buf, int fd);
int buffer_deinit(struct line_buffer *buf);
-void buffer_reset(struct line_buffer *buf);
int buffer_tmpfile_init(struct line_buffer *buf);
FILE *buffer_tmpfile_rewind(struct line_buffer *buf); /* prepare to write. */
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index 644fdc71..f6c0d4c8 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -499,8 +499,6 @@ void svndump_deinit(void)
void svndump_reset(void)
{
- fast_export_reset();
- buffer_reset(input);
strbuf_release(dump_ctx.uuid);
strbuf_release(dump_ctx.url);
strbuf_release(rev_ctx.log);
--
1.7.10.4
--
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/9] vcs-svn: simplify cleanup in apply_one_window
From: David Barr davidb...@google.com
Date: Fri, 1 Jun 2012 00:41:26 +1000
Currently the cleanup code looks like this:
free resources
return 0;
error_out:
free resources
return -1;
Avoid duplicating the free resources part by keeping the return
value in a variable and sharing code between the success and
exceptional case:
ret = 0;
out:
free resources
return ret;
Noticed in the svn-dump-fast-export project, where using the error()
macro in void context produces a warning.
Signed-off-by: David Barr davidb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Changes since v2:
- new description
vcs-svn/svndiff.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c
index c89d9623..e810d0c3 100644
--- a/vcs-svn/svndiff.c
+++ b/vcs-svn/svndiff.c
@@ -258,6 +258,7 @@ static int apply_window_in_core(struct window *ctx)
static int apply_one_window(struct line_buffer *delta, off_t *delta_len,
struct sliding_view *preimage, FILE *out)
{
+ int rv = -1;
struct window ctx = WINDOW_INIT(preimage);
size_t out_len;
size_t instructions_len;
@@ -275,16 +276,15 @@ static int apply_one_window(struct line_buffer *delta,
off_t *delta_len,
if (apply_window_in_core(ctx))
goto error_out;
if (ctx.out.len != out_len) {
- error(invalid delta: incorrect postimage length);
+ rv = error(invalid delta: incorrect postimage length);
goto error_out;
}
if (write_strbuf(ctx.out, out))
goto error_out;
- window_release(ctx);
- return 0;
+ rv = 0;
error_out:
window_release(ctx);
- return -1;
+ return rv;
}
int svndiff0_apply(struct line_buffer *delta, off_t delta_len,
--
1.7.10.4
--
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/9] vcs-svn: use constcmp instead of prefixcmp
From: David Barr davidb...@google.com
Date: Fri, 1 Jun 2012 00:41:27 +1000
Since the length of t is already known, we can simplify a little by
using memcmp() instead of strncmp() to carry out a prefix comparison.
All nearby code already does this.
Noticed in the standalone svn-dump-fast-export project which has not
needed to implement prefixcmp() yet.
Signed-off-by: David Barr davidb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Description clarified. No other change.
vcs-svn/svndump.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index f6c0d4c8..c5d07a66 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -361,7 +361,7 @@ void svndump_read(const char *url)
reset_rev_ctx(atoi(val));
break;
case sizeof(Node-path):
- if (prefixcmp(t, Node-))
+ if (constcmp(t, Node-))
continue;
if (!constcmp(t + strlen(Node-), path)) {
if (active_ctx == NODE_CTX)
--
1.7.10.4
--
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/9] vcs-svn: use strstr instead of memmem
From: David Barr davidb...@google.com Date: Fri, 1 Jun 2012 00:41:28 +1000 memmem is a GNU extension. Avoiding it makes the code clearer and makes it easier for projects that don't share git's compat/ code, such as the standalone svn-dump-fast-export project, to reuse the vcs-svn/ library. Signed-off-by: David Barr davidb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Clarified description. No other change since v2. vcs-svn/fast_export.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c index b4be91cc..854b328d 100644 --- a/vcs-svn/fast_export.c +++ b/vcs-svn/fast_export.c @@ -158,7 +158,7 @@ static int parse_cat_response_line(const char *header, off_t *len) if (ends_with(header, headerlen, missing)) return error(cat-blob reports missing blob: %s, header); - type = memmem(header, headerlen, blob , strlen( blob )); + type = strstr(header, blob ); if (!type) return error(cat-blob header has wrong object type: %s, header); n = strtoumax(type + strlen( blob ), (char **) end, 10); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] vcs-svn: suppress signed/unsigned comparison warnings
From: David Barr davidb...@google.com Date: Fri, 1 Jun 2012 00:41:29 +1000 These are already safe because both sides of the comparison are nonnegative. This would normally not be important because Git is not -Wsign-compare clean anyway, but we like to keep the vcs-svn/ lib to a higher standard for convenience using it in other projects. Signed-off-by: David Barr davidb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- This is the easy part of what is a larger patch[1] in v2, split out and given a new description for easier review. [1] http://thread.gmane.org/gmane.comp.version-control.git/198909/focus=198914 vcs-svn/fast_export.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c index 854b328d..1f046978 100644 --- a/vcs-svn/fast_export.c +++ b/vcs-svn/fast_export.c @@ -254,7 +254,7 @@ static int parse_ls_response(const char *response, uint32_t *mode, } /* Mode. */ - if (response_end - response strlen(100644) || + if (response_end - response (signed) strlen(100644) || response[strlen(100644)] != ' ') die(invalid ls response: missing mode: %s, response); *mode = 0; @@ -267,7 +267,7 @@ static int parse_ls_response(const char *response, uint32_t *mode, } /* ' blob ' or ' tree ' */ - if (response_end - response strlen( blob ) || + if (response_end - response (signed) strlen( blob ) || (response[1] != 'b' response[1] != 't')) die(unexpected ls response: not a tree or blob: %s, response); response += strlen( blob ); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] vcs-svn: suppress a signed/unsigned comparison warning
From: David Barr davidb...@google.com Date: Fri, 1 Jun 2012 00:41:29 +1000 The preceding code checks that view-max_off is nonnegative and (off + width) fits in an off_t, so this code is already safe. Signed-off-by: David Barr davidb...@google.com Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Another unobjectionable piece from v2's patch 5. The new change description explains why it's unobjectionable to save future readers some time. vcs-svn/sliding_window.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vcs-svn/sliding_window.c b/vcs-svn/sliding_window.c index ec2707c9..f11d4909 100644 --- a/vcs-svn/sliding_window.c +++ b/vcs-svn/sliding_window.c @@ -54,7 +54,7 @@ int move_window(struct sliding_view *view, off_t off, size_t width) return -1; if (off view-off || off + width view-off + view-width) return error(invalid delta: window slides left); - if (view-max_off = 0 view-max_off off + width) + if (view-max_off = 0 view-max_off off + (off_t) width) return error(delta preimage ends early); file_offset = view-off + view-buf.len; -- 1.7.10.4 -- 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 8/9] vcs-svn: suppress a signed/unsigned comparison warning
From: Jonathan Nieder jrnie...@gmail.com
Date: Thu, 5 Jul 2012 22:21:09 -0500
All callers pass a nonnegative delta_len, so the code is already safe.
Add an assertion to ensure that remains so and add a cast to keep
clang and gcc -Wsign-compare from worrying.
Reported-by: David Barr davidb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
v2 suppressed the warning by casting len to an off_t, producing an
unintentional change (breakage) in functionality on 64-bit systems
when len is large.
This version is longer but more conservative.
vcs-svn/svndiff.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/vcs-svn/svndiff.c b/vcs-svn/svndiff.c
index e810d0c3..74c97c45 100644
--- a/vcs-svn/svndiff.c
+++ b/vcs-svn/svndiff.c
@@ -77,8 +77,9 @@ static int error_short_read(struct line_buffer *input)
static int read_chunk(struct line_buffer *delta, off_t *delta_len,
struct strbuf *buf, size_t len)
{
+ assert(*delta_len = 0);
strbuf_reset(buf);
- if (len *delta_len ||
+ if (len (uintmax_t) *delta_len ||
buffer_read_binary(delta, buf, len) != len)
return error_short_read(delta);
*delta_len -= buf-len;
@@ -290,7 +291,7 @@ error_out:
int svndiff0_apply(struct line_buffer *delta, off_t delta_len,
struct sliding_view *preimage, FILE *postimage)
{
- assert(delta preimage postimage);
+ assert(delta preimage postimage delta_len = 0);
if (read_magic(delta, delta_len))
return -1;
--
1.7.10.4
--
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 9/9] vcs-svn: allow 64-bit Prop-Content-Length
Date: Thu, 5 Jul 2012 22:47:47 -0500
Currently the vcs-svn/ library only pays attention to the presence of
the Prop-Content-Length field and doesn't care about its value, but
some day we might care about the value. Parse it as an off_t instead
of arbitrarily limiting to 32 bits for intuitiveness.
So now you can import from a dump with more than 2 GiB of properties
for a node. In practice that isn't likely to happen often, and this
is mostly meant as a cleanup.
Based-on-patch-by: David Barr davidb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Another change that was mixed into v2's signedness warnings patch.
In v2 it changed the type of propLength without changing its name.
This version of the patch is more thorough about consistently using
the intuitive type (off_t instead of a 32-bit integer).
That's the end of the series. Thanks for your patience.
vcs-svn/svndump.c | 33 ++---
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index c5d07a66..a7f3ea64 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -34,14 +34,13 @@
#define NODE_CTX 2 /* node metadata */
#define INTERNODE_CTX 3/* between nodes */
-#define LENGTH_UNKNOWN (~0)
#define DATE_RFC2822_LEN 31
static struct line_buffer input = LINE_BUFFER_INIT;
static struct {
- uint32_t action, propLength, srcRev, type;
- off_t text_length;
+ uint32_t action, srcRev, type;
+ off_t prop_length, text_length;
struct strbuf src, dst;
uint32_t text_delta, prop_delta;
} node_ctx;
@@ -61,7 +60,7 @@ static void reset_node_ctx(char *fname)
{
node_ctx.type = 0;
node_ctx.action = NODEACT_UNKNOWN;
- node_ctx.propLength = LENGTH_UNKNOWN;
+ node_ctx.prop_length = -1;
node_ctx.text_length = -1;
strbuf_reset(node_ctx.src);
node_ctx.srcRev = 0;
@@ -209,7 +208,7 @@ static void read_props(void)
static void handle_node(void)
{
const uint32_t type = node_ctx.type;
- const int have_props = node_ctx.propLength != LENGTH_UNKNOWN;
+ const int have_props = node_ctx.prop_length != -1;
const int have_text = node_ctx.text_length != -1;
/*
* Old text for this node:
@@ -273,7 +272,7 @@ static void handle_node(void)
if (have_props) {
if (!node_ctx.prop_delta)
node_ctx.type = type;
- if (node_ctx.propLength)
+ if (node_ctx.prop_length)
read_props();
}
@@ -409,22 +408,26 @@ void svndump_read(const char *url)
node_ctx.srcRev = atoi(val);
break;
case sizeof(Text-content-length):
- if (!constcmp(t, Text-content-length)) {
+ if (constcmp(t, Text) constcmp(t, Prop))
+ continue;
+ if (constcmp(t + 4, -content-length))
+ continue;
+ {
char *end;
- uintmax_t textlen;
+ uintmax_t len;
- textlen = strtoumax(val, end, 10);
+ len = strtoumax(val, end, 10);
if (!isdigit(*val) || *end)
die(invalid dump: non-numeric length
%s, val);
- if (textlen
maximum_signed_value_of_type(off_t))
+ if (len maximum_signed_value_of_type(off_t))
die(unrepresentable length in dump:
%s, val);
- node_ctx.text_length = (off_t) textlen;
+
+ if (*t == 'T')
+ node_ctx.text_length = (off_t) len;
+ else
+ node_ctx.prop_length = (off_t) len;
break;
}
- if (constcmp(t, Prop-content-length))
- continue;
- node_ctx.propLength = atoi(val);
- break;
case sizeof(Text-delta):
if (!constcmp(t, Text-delta)) {
node_ctx.text_delta = !strcmp(val, true);
--
1.7.10.4
--
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: [PATCHv3 02/11] Makefile: fold MISC_H into LIB_H
Hi, I finally found some moments to revisit this series, so I'm starting here. I think the justification for this patch is something like this: Keeping track of what files include each header is an error-prone chore. On top of that, for l10n, these days we have to keep a master list of all headers, too, which is double work when adding a new header that adds insult to injury. Active Makefile hackers tend to use compilers like gcc that support automatic dependency generation with -MMD. The precise header deps aren't even used when building with these compilers, so the people maintaining the precise header deps do not benefit from them at all. Unfair! Non-developers who can't fend for themselves do not rebuild after a small header change very often, so they would not be hurt much by a change one header, rebuild everything rule when automatic dependency generation is disabled, either. That leaves at least one important category of people to be hurt by this change: the glorious MSVC hackers. MSVC supports the appropriate magic to compute header dependencies, but no one's gotten around to teaching the Makefile to use it yet. So let's stop delaying the inevitable and drop the detailed dependencies. If anyone complains then we can work with them to finish support for computing header dependencies for the relevant compiler. Fair enough. Two details puzzle me: Jeff King wrote: The original point of LIB_H was that it would force recompilation of C files when any of the library headers changed. LIB_H was introduced by commit e590d694 (Add more header dependencies, 2005-04-18). It only lists cache.h object.h even though some translation units included tree.h, commit.h, or blob.h already. So at least back then, it seems to have been about library headers and not about all headers (and all headers was puzzlingly not worth worrying about at all). So isn't this a fundamentally new thing, rather than a return to the state of nature? The other remaining question is why we don't use something like $(wildcard *.h) and avoid listing individual headers altogether. Is the fear that some stray non-git header will find its way into the cwd and poison the translation files? (If so, I'd like to document that as well to help readers understand why we keep doing the work we do.) Ciao, Jonathan -- 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 02.5/11] Makefile: fold XDIFF_H and VCSSVN_H into LIB_H
Just like MISC_H (see previous commit), there is no reason to track xdiff and vcs-svn headers separately from the rest of the headers. The only purpose of these variables is to keep track of recompilation dependencies. As a pleasant side effect, folding these into LIB_H lets us stop tracking GIT_OBJS and VCSSVN_TEST_OBJS separately from the list of all OBJECTS. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Jeff King wrote: On Wed, Jun 20, 2012 at 04:07:30PM -0500, Jonathan Nieder wrote: Should XDIFF_H and VCSSVN_H be folded into STATIC_HEADERS, too? I stopped short of that, but I'd be tempted to do so. Here goes. Makefile | 60 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/Makefile b/Makefile index 500966b1..b24ca20d 100644 --- a/Makefile +++ b/Makefile @@ -392,11 +392,8 @@ BUILTIN_OBJS = BUILT_INS = COMPAT_CFLAGS = COMPAT_OBJS = -XDIFF_H = XDIFF_OBJS = -VCSSVN_H = VCSSVN_OBJS = -VCSSVN_TEST_OBJS = GENERATED_H = EXTRA_CPPFLAGS = LIB_H = @@ -558,21 +555,21 @@ LIB_FILE=libgit.a XDIFF_LIB=xdiff/lib.a VCSSVN_LIB=vcs-svn/lib.a -XDIFF_H += xdiff/xinclude.h -XDIFF_H += xdiff/xmacros.h -XDIFF_H += xdiff/xdiff.h -XDIFF_H += xdiff/xtypes.h -XDIFF_H += xdiff/xutils.h -XDIFF_H += xdiff/xprepare.h -XDIFF_H += xdiff/xdiffi.h -XDIFF_H += xdiff/xemit.h +LIB_H += xdiff/xinclude.h +LIB_H += xdiff/xmacros.h +LIB_H += xdiff/xdiff.h +LIB_H += xdiff/xtypes.h +LIB_H += xdiff/xutils.h +LIB_H += xdiff/xprepare.h +LIB_H += xdiff/xdiffi.h +LIB_H += xdiff/xemit.h -VCSSVN_H += vcs-svn/line_buffer.h -VCSSVN_H += vcs-svn/sliding_window.h -VCSSVN_H += vcs-svn/repo_tree.h -VCSSVN_H += vcs-svn/fast_export.h -VCSSVN_H += vcs-svn/svndiff.h -VCSSVN_H += vcs-svn/svndump.h +LIB_H += vcs-svn/line_buffer.h +LIB_H += vcs-svn/sliding_window.h +LIB_H += vcs-svn/repo_tree.h +LIB_H += vcs-svn/fast_export.h +LIB_H += vcs-svn/svndiff.h +LIB_H += vcs-svn/svndump.h GENERATED_H += common-cmds.h @@ -2110,13 +2107,6 @@ version.o git.spec \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ : GIT-VERSION-FILE -TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) -GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ - git.o -ifndef NO_CURL - GIT_OBJS += http.o http-walker.o remote-curl.o -endif - XDIFF_OBJS += xdiff/xdiffi.o XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o @@ -2132,9 +2122,14 @@ VCSSVN_OBJS += vcs-svn/fast_export.o VCSSVN_OBJS += vcs-svn/svndiff.o VCSSVN_OBJS += vcs-svn/svndump.o -VCSSVN_TEST_OBJS += test-line-buffer.o - -OBJECTS := $(GIT_OBJS) $(XDIFF_OBJS) $(VCSSVN_OBJS) +TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) +OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ + $(XDIFF_OBJS) \ + $(VCSSVN_OBJS) \ + git.o +ifndef NO_CURL + OBJECTS += http.o http-walker.o remote-curl.o +endif dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS @@ -2233,15 +2228,8 @@ else # Dependencies on automatically generated headers such as common-cmds.h # should _not_ be included here, since they are necessary even when # building an object for the first time. -# -# XXX. Please check occasionally that these include all dependencies -# gcc detects! -$(GIT_OBJS): $(LIB_H) - -xdiff-interface.o $(XDIFF_OBJS): $(XDIFF_H) - -$(VCSSVN_OBJS) $(VCSSVN_TEST_OBJS): $(LIB_H) $(VCSSVN_H) +$(OBJECTS): $(LIB_H) endif exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ @@ -2334,7 +2322,7 @@ XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ --keyword=_ --keyword=N_ --keyword=Q_:1,2 XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --keyword=__ --language=Perl -LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(XDIFF_H) $(VCSSVN_H) $(GENERATED_H) +LOCALIZED_C := $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) LOCALIZED_SH := $(SCRIPT_SH) LOCALIZED_PERL := $(SCRIPT_PERL) -- 1.7.10.4 -- 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/RFC] Makefile: document ground rules for target-specific dependencies
When a source file makes use of a makefile variable, there should be a corresponding dependency on a file that changes when that variable changes to ensure the build output is not left stale when the variable changes. Document this, even though we are not following the rule perfectly yet. Based on an explanation from Jeff King. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Jeff King wrote: On Wed, Jun 20, 2012 at 04:12:25PM -0500, Jonathan Nieder wrote: Wouldn't it be simpler to put the ground rules in a comment or a document somewhere under Documentation/ where they can be easily found? I think a comment in the Makefile might make sense (especially if it introduces the section as and this is the place to put weird target-specific cppflags and dependencies). How about something like this? Makefile | 34 ++ 1 file changed, 34 insertions(+) diff --git a/Makefile b/Makefile index 3f82b51b..542856f0 100644 --- a/Makefile +++ b/Makefile @@ -1970,6 +1970,40 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell strip: $(PROGRAMS) git$X $(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X + +### Target-specific flags and dependencies + +# The generic compilation pattern rule and automatically +# computed header dependencies (falling back to a dependency on +# LIB_H) are enough to describe how most targets should be built, +# but some targets are special enough to need something a little +# different. +# +# - When a source file foo.c #includes a generated header file, +# we need to list that dependency for the foo.o target. +# +# We also list it from other targets that are built from foo.c +# like foo.sp and foo.s, even though that is easy to forget +# to do because the generated header is already present around +# after a regular build attempt. +# +# - Some code depends on configuration kept in makefile +# variables. The target-specific variable EXTRA_CPPFLAGS can +# be used to convey that information to the C preprocessor +# using -D options. +# +# The foo.o target should have a corresponding dependency on +# a file that changes when the value of the makefile variable +# changes. For example, targets making use of the +# $(GIT_VERSION) variable depend on GIT-VERSION-FILE. +# +# Technically the .sp and .s targets do not need this +# dependency because they are force-built, but they get the +# same dependency for consistency. This way, you do not have to +# know how each target is implemented. And it means the +# dependencies here will not need to change if the force-build +# details change some day. + git.sp git.s git.o: EXTRA_CPPFLAGS = \ '-DGIT_HTML_PATH=$(htmldir_SQ)' \ '-DGIT_MAN_PATH=$(mandir_SQ)' \ -- 1.7.10.4 -- 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 v4 06/11] Makefile: split GIT_USER_AGENT from GIT-CFLAGS
The default user-agent depends on the GIT_VERSION, which means that anytime you switch versions, it causes a full rebuild. Instead, let's split it out into its own file and restrict the dependency to version.o. To avoid noise during builds, unlike the GIT-CFLAGS rule which prints * new build flags or prefix so the operator knows why all files are being rebuilt when it changes, GIT-USER-AGENT generation is silent. If this code breaks and a target depending on GIT-USER-AGENT ends up being rebuilt when it shouldn't be, the full dependency chain can be retrieved with make --debug=b. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- Jeff King wrote: I am tempted to get rid of the informative message altogether. Like this? .gitignore |1 + Makefile | 10 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index bf66648e..7329cfe5 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /GIT-CFLAGS /GIT-LDFLAGS /GIT-GUI-VARS +/GIT-USER-AGENT /GIT-VERSION-FILE /bin-wrappers/ /git diff --git a/Makefile b/Makefile index 7148cadd..2a84cd8b 100644 --- a/Makefile +++ b/Makefile @@ -1922,7 +1922,10 @@ endif GIT_USER_AGENT_SQ = $(subst ','\'',$(GIT_USER_AGENT)) GIT_USER_AGENT_CQ = $(subst ,\,$(subst \,\\,$(GIT_USER_AGENT))) GIT_USER_AGENT_CQ_SQ = $(subst ','\'',$(GIT_USER_AGENT_CQ)) -BASIC_CFLAGS += -DGIT_USER_AGENT='$(GIT_USER_AGENT_CQ_SQ)' +GIT-USER-AGENT: FORCE + @if test x'$(GIT_USER_AGENT_SQ)' != x`cat GIT-USER-AGENT 2/dev/null`; then \ + echo '$(GIT_USER_AGENT_SQ)' GIT-USER-AGENT; \ + fi ALL_CFLAGS += $(BASIC_CFLAGS) ALL_LDFLAGS += $(BASIC_LDFLAGS) @@ -2021,8 +2024,10 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ '-DGIT_MAN_PATH=$(mandir_SQ)' \ '-DGIT_INFO_PATH=$(infodir_SQ)' +version.sp version.s version.o: GIT-USER-AGENT version.sp version.s version.o: EXTRA_CPPFLAGS = \ - '-DGIT_VERSION=$(GIT_VERSION)' + '-DGIT_VERSION=$(GIT_VERSION)' \ + '-DGIT_USER_AGENT=$(GIT_USER_AGENT_CQ_SQ)' $(BUILT_INS): git$X $(QUIET_BUILT_IN)$(RM) $@ \ @@ -2744,6 +2749,7 @@ ifndef NO_TCLTK $(MAKE) -C git-gui clean endif $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS + $(RM) GIT-USER-AGENT .PHONY: all install profile-clean clean strip .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell -- 1.7.10.4 -- 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] branch: introduce --set-upstream-to
Hi,
Carlos Martín Nieto wrote:
The existing --set-uptream option can cause confusion, as it uses the
usual branch convention of assuming a starting point of HEAD if none
is specified, causing
git branch --set-upstream origin/master
to create a new local branch 'origin/master' that tracks the current
branch. As --set-upstream already exists, we can't simply change its
behaviour. To work around this, introduce --set-upstream-to which
accepts a compulsory argument
Thanks. A part of me really dislikes this --set-upstream-to which
is named more awkwardly than the deprecated mistake it replaces,
though.
Here's a patch on top to play with that names the new option
--set-upstream=. Untested.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
diff --git i/Documentation/git-branch.txt w/Documentation/git-branch.txt
index f572913f..57935a64 100644
--- i/Documentation/git-branch.txt
+++ w/Documentation/git-branch.txt
@@ -49,7 +49,7 @@ branch so that 'git pull' will appropriately merge from
the remote-tracking branch. This behavior may be changed via the global
`branch.autosetupmerge` configuration flag. That setting can be
overridden by using the `--track` and `--no-track` options, and
-changed later using `git branch --set-upstream-to`.
+changed later using `git branch --set-upstream`.
With a `-m` or `-M` option, oldbranch will be renamed to newbranch.
If oldbranch had a corresponding reflog, it is renamed to match
@@ -174,11 +174,13 @@ start-point is either a local or remote-tracking branch.
like `--track` would when creating the branch, except that where
branch points to is not changed.
--u upstream::
---set-upstream-to=upstream::
+--set-upstream=upstream::
Set up branchname's tracking information so upstream is
considered branchname's upstream branch. If no branch is
specified it defaults to the current branch.
++
+If no argument is attached, for historical reasons the meaning is
+different. See above.
--edit-description::
Open an editor and edit the text to explain what the branch is
diff --git i/builtin/branch.c w/builtin/branch.c
index c886fc06..0d705790 100644
--- i/builtin/branch.c
+++ w/builtin/branch.c
@@ -669,6 +669,31 @@ static int opt_parse_merge_filter(const struct option
*opt, const char *arg, int
return 0;
}
+struct set_upstream_params {
+ enum branch_track *track;
+ const char **new_upstream;
+};
+static int parse_opt_set_upstream(const struct option *opt, const char *arg,
int unset)
+{
+ struct set_upstream_params *o = opt-value;
+
+ if (unset) {/* --no-set-upstream */
+ *o-track = BRANCH_TRACK_NEVER;
+ *o-new_upstream = NULL;
+ return 0;
+ }
+
+ *o-track = BRANCH_TRACK_OVERRIDE;
+ if (!arg) /* --set-upstream branchname start-point */
+ *o-new_upstream = NULL;
+ else/* --set-upstream=upstream branchname */
+ *o-new_upstream = arg;
+ return 0;
+}
+#define OPT_SET_UPSTREAM(s, l, v) \
+ { OPTION_CALLBACK, (s), (l), (v), upstream, change upstream info, \
+ PARSE_OPT_OPTARG, parse_opt_set_upstream }
+
static const char edit_description[] = BRANCH_DESCRIPTION;
static int edit_branch_description(const char *branch_name)
@@ -716,6 +741,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
const char *new_upstream = NULL;
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
+ struct set_upstream_params set_upstream_args = { track, new_upstream
};
struct commit_list *with_commit = NULL;
struct option options[] = {
@@ -725,9 +751,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT__QUIET(quiet, suppress informational messages),
OPT_SET_INT('t', track, track, set up tracking mode (see
git-pull(1)),
BRANCH_TRACK_EXPLICIT),
- OPT_SET_INT( 0, set-upstream, track, change upstream info,
- BRANCH_TRACK_OVERRIDE),
- OPT_STRING('u', set-upstream-to, new_upstream, upstream,
change the upstream info),
+ OPT_SET_UPSTREAM(0, set-upstream, set_upstream_args),
OPT__COLOR(branch_use_color, use colored output),
OPT_SET_INT('r', remotes, kinds, act on remote-tracking
branches,
REF_REMOTE_BRANCH),
--
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/3] branch: suggest how to undo a --set-upstream when given one branch
Hi,
Quick nitpicks.
Carlos Martín Nieto wrote:
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -864,10 +864,32 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
info and making sure new_upstream is correct */
create_branch(head, branch-name, new_upstream, 0, 0, 0, quiet,
BRANCH_TRACK_OVERRIDE);
} else if (argc 0 argc = 2) {
+ struct branch *branch = branch_get(argv[0]);
+ const char *old_upstream = NULL;
+ int branch_existed = 0;
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make
sense with a branch name));
+
+ /* Save what argv[0] was pointing to so we can give
+the --set-upstream-to hint */
+ if (branch_has_merge_config(branch))
+ old_upstream = shorten_unambiguous_ref(branch-merge[0]-dst,
0);
Whitespace is odd here. Maybe this case could be factored out as a
new function to make room on the right margin and make cmd_branch()
easier to read straight through.
+
+ branch_existed = ref_exists(branch-refname);
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
force_create, reflog, 0, quiet, track);
+
+ if (argc == 1) {
+ printf(If you wanted to make '%s' track '%s', do
this:\n, head, argv[0]);
+ if (branch_existed)
+ printf( $ git branch --set-upstream '%s'
'%s'\n, argv[0], old_upstream);
+ else
+ printf( $ git branch -d '%s'\n, argv[0]);
+
+ printf( $ git branch --set-upstream-to '%s'\n,
argv[0]);
Message should go on stderr and be guarded with an advice option (see
advice.c).
Like this:
const char *arg;
...
if (argc != 1 || !advice_old_fashioned_set_upstream)
return 0; /* ok. */
arg = argv[0];
advise(If you wanted to make '%s' track '%s', do this:,
head, arg);
if (branch_existed)
advise( $ git branch --set-upstream-to='%s' '%s',
old_upstream, arg);
else
advise( $ git branch -d '%s', arg);
advise( $ git branch --set-upstream-to='%s', arg);
If an argument contains single-quotes, the quoting will be wrong, but
that's probably not worth worrying about.
Hope that helps,
Jonathan
--
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
