Re: git-submodule nested subrepo bug (Segmentation fault)
I'm guessing Kirill meant to send this to the list and not just to me. It looks to me like the segfault is in MSys's mkdir.exe and not a Git process. - Forwarded message from Kirill Berezin ene...@develop-project.ru - From: Kirill Berezin ene...@develop-project.ru To: John Keeping j...@keeping.me.uk Date: Wed, 22 May 2013 09:54:47 +0400 Message-ID: caaobgf-pe_6pf3af5uba8rfkwxa43gxeg9romjf62usz7k4...@mail.gmail.com Subject: Re: git-submodule nested subrepo bug (Segmentation fault) Ok, version is: 1.8.1.msysgit.1 Segmentation fault at the git clone --recursive http://github.com/Exsul/al_server 0 [main] mkdir 6596 open_stackdumpfile: Dumping stack trace to mkdir.exe.stackdump C:\Users\\libexec\git-core\git-submodule: line 181: 6596 Segmentation fault (core dumped) mkdir -p $ditdir_base fatal: Could not switch to 's:/USER//al_server/.git/modules/': No such file or directory Clone of 'https://.../Exsul/chat.git' into submodule path 'chat' failed PS so much mails per day... 2013/5/20 Kirill Berezin ene...@develop-project.ru: This is standart github shell(Windows 7). Right now i cant access my home platform, will tell you tommorow. 2013/5/20 John Keeping j...@keeping.me.uk: On Mon, May 20, 2013 at 09:32:21AM +0400, Kirill Berezin wrote: When you trying to add submodule, that already has submodule, it craches. For example you could try: git clone --recursive http://github.com/Exsul/al_server Which version of Git were you using? I was not able to reproduce this with 1.8.3-rc3. - End forwarded message - -- 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: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 12:29:59PM +0200, Andreas Krey wrote: On Thu, 23 May 2013 05:48:38 +, John Szakmeister wrote: ... This is a feature of `git pull` that I really despise. I really wish `git pull` treated the remote as the first parent in its merge operation. I'd actually only like it that way when pulling from the tracking branch, not for any pull. I'll add my voice to the annoyed by this pile ;-) I've been annoyed by this at $DAYJOB recently. A lot of people seem to blindly git pull without much thought about how the history is ending up and what they actually want to do. I wonder if it would make sense for git pull (with no arguments) to pass --ff-only to git-merge, allowing this to be overridden with --rebase and --merge (which doesn't currently exist). With some suitable advice output we could hopefully educate users about how to shape their history. -- 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: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 09:01:15AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: I've been annoyed by this at $DAYJOB recently. A lot of people seem to blindly git pull without much thought about how the history is ending up and what they actually want to do. I think these two are essentially the same thing, and having an option to flip the heads of a merge only solves a half of the problem. A merge that shows everybody else's work merged into your history means you are the integrator, the keeper of the main history. And the first-parent view of the history is useful only when the keeper of the main history takes good care of the main history. When you are using a central shared repository workflow, if you had and used an option to flip the heads of a merge to record what you have done so far as a side branch of what everybody else did to do the merge, or if you rebased your work on top of what everybody else did, the first-parent view would make a bit more sense than what you currently get. At least, everybody else's work will not appear as a side branch that does 47 unrelated things, and your work will appear as a side branch. That is a big plus. But the other half of the problem still remains, i.e. what they actually want to do. People tend to do too many pull when their work is not ready, only to catch up, and that is the real problem. ... One obvious way to solve it is to use a topic branch workflow (the first picture above; 'x's are built not on local 'master'), and you do a git pull from the shared repository while you are on your 'master', which is free of your 'x's until that 6-commit series is complete and ready. Then you locally merge that topic branch and push it back for everybody to see, which will give you the first picture in this message. Incidentally, this does not need the flip the heads option. Yes, I don't think this is as much of a problem when using a topic branch workflow, because it's clear what the history should look like and users are expected to get it right. Where I see this is when people are aiming for a linear history but don't get that because with git pull to catch up they end up with these backwards merges. In these cases, I think what users really want is git pull --rebase. I have to wonder how often git pull with no arguments actually does what users really want (even if they don't know it!) when it doesn't result in a fast-forward (and pull.rebase isn't configured). Hence my suggestion to error when git pull doesn't result in a fast-forward and no branch name is specified. We could give some advice like: Your local changes are not included in the local branch and you haven't told Git how to preserve them. If you want to rebase your changes onto the modified upstream branch, run: git pull --rebase Solving half a problem is better than solving no problem, and especially because not all changes need to be multi-commit series but can be done directly, perfectly and fully on the local 'master' (i.e. 2+3+1=6 split would not happen for such changes). For these reasons, I personally am not strongly opposed to a flip the heads option, if implemented cleanly. But people need to realize that it is not solving the other half, a more fundamental problem some people have in their workflow. Yes, but some users don't realise that their workflow is broken, and perhaps we can nudge them in the right direction. -- 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: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 02:01:39PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: I have to wonder how often git pull with no arguments actually does what users really want (even if they don't know it!) when it doesn't result in a fast-forward (and pull.rebase isn't configured). If you are in a totally centralized shared repository mindset without using topic branch workflow, --first-parent would not help you. In your history the second parent is more likely to be the mainline. So for them git pull that either fast-forward when it can, or makes a merge that records the then-current state of the central shared repository, is perfectly sensible. They will view gitk and see all the changes, git shortlog and git log --no-merges will give them what they expect. Yes, but for people used to a cleaner history it's confusing to see the mainline branch and one small change the wrong way round. When I see people doing this, it's normally something like: ... do some work for several hours... git commit -a git push # fails because it's not a fast forward git pull git push In this scenario, just adding --rebase to git pull actually results in a much more sensible history. Hence my suggestion to error when git pull doesn't result in a fast-forward and no branch name is specified. We could give some advice like: Your local changes are not included in the local branch and you haven't told Git how to preserve them. If you want to rebase your changes onto the modified upstream branch, run: git pull --rebase I can parse the first paragraph above, but cannot make much sense out of it. Unless you are talking about local changes that are not committed yet, that is. But in that case I fail to see what it has to do with the current discussion, or suggestion to use rebase. This isn't about swap parents, it's about helping people realise that just git pull isn't necessarily the best thing for them to do, and that they may want --rebase. So I was asking if it would be sensible (possibly in Git 2.0) to make git-pull pass --ff-only to git-merge by default. -- 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 stash deletes/drops changes of
On Fri, May 24, 2013 at 01:57:12AM +0200, Petr Baudis wrote: Just to clear up on what the best practice is, I'd imagine the setup to be something like: (a) Makefile contains inclusion of Makefile.include. (b) There is a file like Makefile.include.template containing a template to be copied over and filled by the user. (c) Makefile contains code that makes sure all variables that are supposed to be set are set and obsolete variables are not, since there is no mechanism to cause e.g. a merge conflict on change of Makefile.include.template. Is there a better way to solve this? I think the best practice would be what Git itself does ;-) The Makefile sets default values for all parameters, some of which are inferred based on the system. It then includes config.mak, which allows the user to override any of these values. -- 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: first parent, commit graph layout, and pull merge direction
On Thu, May 23, 2013 at 06:53:36PM -0500, Felipe Contreras wrote: The alternatives are these: a) you annoy the vast majority of the user-base by making 'git pull' a dangerous operation that should be avoided, and replaced with 'git fetch'+'git rebase'. b) you annoy a minority of the user-base by making 'git pull' not do the merge the expected, so they have to do +'git merge' (which is already less of a change than a)), or configure the default (which they most likely are able to do, if they did intent to do a merge). Note that in my email that started this, I tried to be clear that I was talking about git pull *without a branch name*. If this user explicitly says git pull remote branch then I consider that a clear indication that they really do mean to perform a merge; I would not recommend changing the current behaviour in that case. If the user just says git pull then it is more likely that they are just trying to synchronise with the upstream branch, in which case they probably don't actually want a merge. -- 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 stash deletes/drops changes of
On Fri, May 24, 2013 at 11:40:07AM +0200, Petr Baudis wrote: On Fri, May 24, 2013 at 09:22:53AM +0100, John Keeping wrote: On Fri, May 24, 2013 at 01:57:12AM +0200, Petr Baudis wrote: Just to clear up on what the best practice is, I'd imagine the setup to be something like: (a) Makefile contains inclusion of Makefile.include. (b) There is a file like Makefile.include.template containing a template to be copied over and filled by the user. (c) Makefile contains code that makes sure all variables that are supposed to be set are set and obsolete variables are not, since there is no mechanism to cause e.g. a merge conflict on change of Makefile.include.template. Is there a better way to solve this? I think the best practice would be what Git itself does ;-) The Makefile sets default values for all parameters, some of which are inferred based on the system. It then includes config.mak, which allows the user to override any of these values. So that's pretty similar to what I described, modulo the filenames. I'd say it's more friendly if you don't need to tweak any of the defaults in the common case, but less friendly if you always need to tweak something/everything (you really want a template file then and not covering (c) is a problem). I don't see anything wrong with having a template file documenting the parameters, but I think it's important that there are sensible defaults in place when the user's configuration file does not specify a value for a parameter. It wasn't clear to me from your definition that there were defaults to be overridden by the user's configuration file, as opposed to forcing the user to define certain values and causing an error if those are not defined. -- 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 stash deletes/drops changes of
On Fri, May 24, 2013 at 12:14:16PM +0200, Petr Baudis wrote: On Fri, May 24, 2013 at 11:06:12AM +0100, John Keeping wrote: I don't see anything wrong with having a template file documenting the parameters, but I think it's important that there are sensible defaults in place when the user's configuration file does not specify a value for a parameter. It wasn't clear to me from your definition that there were defaults to be overridden by the user's configuration file, as opposed to forcing the user to define certain values and causing an error if those are not defined. That's the case in plenty of situations - when specifying usernames and passwords and server hostnames, paths to cross-compiling environments that pretty much everyone has at a different place, and so on. Yeah, I didn't mean to say that everything can have a sensible default. Going back to where this started, in the omxplayer Makefile, I would map my suggestion to a change like this: * Change most of the := in Makefile.include to = so that the order of variable definition matters less * Move Makefile.include to Makefile.defaults * Change the include Makefile.include at the top of Makefile to: include Makefile.defaults -include Makefile.config * Add Makefile.config to .gitignore So that it continues to Just Work for people using buildroot but you can create Makefile.config to override those defaults. I agree that this isn't possible in all cases, and your template approach is certainly useful for configuration files - particularly because those templates can be included in end-user documentation or the installation as they are likely to be needed in the installed application and not just development. -- 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 stash deletes/drops changes of
On Fri, May 24, 2013 at 01:03:22PM +0200, Petr Baudis wrote: On Fri, May 24, 2013 at 11:40:18AM +0100, John Keeping wrote: So that it continues to Just Work for people using buildroot but you can create Makefile.config to override those defaults. Indeed, that doesn't cover some corner cases of (c), but that's not a big deal in practice I guess. My point still stands - this is extra hassle, done just for the sake of the tool; I think the tool should not get in the way. Moreover, it's not the default solution for your typical original author and therefore you will still often find yourself in a situation where you have to deal with a setup that's broken already. I think we're in violent agreement here. I can see that there are cases where an --ignore-changes option that behaves like --assume-unchanged but without ever overwriting the local file is a useful feature. I was simply trying to point at what I consider best practices for makefiles, which was relevant for the example you gave. Sorry if that was unclear. -- 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 stash deletes/drops changes of
On Fri, May 24, 2013 at 03:34:26PM +, Jim Greenleaf wrote: Phil Hord phil.hord at gmail.com writes: The wording of --ignore-changes suffers the same lack of clarity that --assume-unchanged does. What's better? --sequester is probably too obscure. Maybe --hold. Or --silence. Or --shut-up. How about --freeze? I wonder if this would be better as a file rather than another option to git-update-index. We already have .git/info/exclude so we could add .git/info/freeze or .git/info/local with the same syntax as the normal .gitignore file. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git stash deletes/drops changes of
On Fri, May 24, 2013 at 03:42:37PM +, Jim Greenleaf wrote: John Keeping john at keeping.me.uk writes: I wonder if this would be better as a file rather than another option to git-update-index. We already have .git/info/exclude so we could add .git/info/freeze or .git/info/local with the same syntax as the normal .gitignore file. .git/info/freeze would be a good solution. It would avoid the need to add a new class of files for git-status, while keeping a simple, familiar record of all frozen files in a single location. Now I've thought about it a bit more, I'm not sure this does work. If an entry in the freeze list means ignore local changes in this file, we really want to be talking about local changes relative to some base. Otherwise, what happens if the upstream file is radically altered? A user probably doesn't want to keep their file unchanged when this happens. So we don't just want to store the filename, we want to store the version of the file that the user chose to ignore. One way to do this might be to mark the file as a conflict whenever a change to it comes in and ignore the freeze file when there is a conflict in the index. But then we either need to introduce a new command to manage this state or some way for the user to perform Git operations ignoring the freeze file, otherwise how can the user pull down updates? Perhaps a more user-friendly way to handle this would be to introduce auto-stash around any operation that will modify a frozen file. So we stash the user's (frozen) changes and then apply them after changing the file. If there are conflicts then these are marked in the index and must be resolved, then the unstaged changes in the file are ignored again. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #07; Fri, 24)
On Fri, May 24, 2013 at 02:15:55PM -0700, Junio C Hamano wrote: * jk/submodule-subdirectory-ok (2013-04-24) 3 commits (merged to 'next' on 2013-04-24 at 6306b29) + submodule: fix quoting in relative_path() (merged to 'next' on 2013-04-22 at f211e25) + submodule: drop the top-level requirement + rev-parse: add --prefix option Allow various subcommands of git submodule to be run not from the top of the working tree of the superproject. What's the status of this one? As far as I'm concerned this is done. If you're re-rolling next you could squash the top two together but I don't think that's really necessary. -- 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] for-each-ref: introduce %C(...) for color
On Sat, May 25, 2013 at 05:20:29PM +0530, Ramkumar Ramachandra wrote: Antoine Pelisse wrote: Is it not possible for color to be used uninitialized here ? My compiler didn't complain; what am I missing? Doesn't the declaration char color[COLOR_MAXLEN]; initialize an empty string? Why would it? The variable's begin allocated on the stack and the C standard only zero-initializes variables with static storage duration; Section 6.7.9 of the C11 standard says: If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. I suspect the compiler doesn't complain because there is a path through the function that initializes color before reading it (if we hit the if branch in the loop before the else branch) and the compile assumes that there is something in the function's contract that guarantees that we follow this path. But I don't think that's correct so you do need to initialize color to the empty string. More importantly, aren't there numerous instances of this in the codebase? Care to point at one? I had a quick look and all places I inspected are either static or write to the array before reading it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool --dir-diff: copy back all files matching the working tree
On Mon, May 27, 2013 at 12:00:46AM +0900, Kenichi Saita wrote:
After running the user's diff tool, git difftool --dir-dif --no-symlink
currently copied back a temporary file to working tree only when a file
contains unstaged changes in the working tree.
Change this behavior so that temporary files are copied back to working
tree whenever the right-hand side of the comparison has the same SHA1
as the file in the working tree.
Signed-off-by: Kenichi Saita nito...@gmail.com
This change looks sensible to me, but I found the commit message quite
confusing.
The code being changed here is to do with choosing whether to copy the
working tree file to the temporary directory (or symlink it) and hence
only indirectly related to whether it will be copied back. It might be
clearer to phrase it like this:
difftool --dir-diff: always use identical working tree file
When deciding whether or not we should link a working tree file into
the temporary right-hand directory for a directory diff, we
currently behave differently in the --symlink and --no-symlink
cases. If using symlinks any identical files are linked across but
with --no-symlink only files that contain unstaged changes are
copied.
Change this so that identical files are copied across as well. This
is beneficial because it widens the set of circumstances in which we
copy changes made by the user back into the working tree.
---
git-difftool.perl |9 ++---
t/t7800-difftool.sh | 19 +++
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 8a75205..e57d3d1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -85,13 +85,9 @@ sub exit_cleanup
sub use_wt_file
{
- my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+ my ($repo, $workdir, $file, $sha1) = @_;
my $null_sha1 = '0' x 40;
- if ($sha1 ne $null_sha1 and not $symlinks) {
- return 0;
- }
-
if (! -e $workdir/$file) {
# If the file doesn't exist in the working tree, we cannot
# use it.
@@ -213,8 +209,7 @@ EOF
if ($rmode ne $null_mode) {
my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
- $dst_path, $rsha1,
- $symlinks);
+ $dst_path, $rsha1);
if ($use) {
push @working_tree, $dst_path;
$wtindex .= $rmode $wt_sha1\t$dst_path\0;
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index d46f041..2418528 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -385,6 +385,25 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff
--symlink without unstage
test_cmp actual expect
'
+write_script modify-right-file \EOF
+echo new content $2/file
+EOF
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged change' '
+ test_when_finished git reset --hard
+ echo orig content file
+ git difftool -d $symlinks --extcmd $(pwd)/modify-right-file branch
+ echo new content expect
+ test_cmp expect file
+'
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged
change' '
+ test_when_finished git reset --hard
+ git difftool -d $symlinks --extcmd $(pwd)/modify-right-file branch
+ echo new content expect
+ test_cmp expect file
+'
+
write_script modify-file \EOF
echo new content file
EOF
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git clone does not understand insteadOf URLs
On Sun, May 26, 2013 at 08:09:56PM +0200, Gioele Barabucci wrote: Il 26/05/2013 20:00, Andreas Schwab ha scritto: Simple, I keep all my projects on the same server, so I would like to refer to that server + path using 'remote-repo'. git+ssh://git.example.org//users/gioele/projects insteadOf remote-repo You can use remote-repo: instead. Do you mean I could use git+ssh://git.example.org//users/gioele/projects insteadOf remote-repo:? Yes, but now I have dozens of repositories already set up in various workstations and I do not want to go and change all of them. What really bugs me is the fact that `git clone` and `git remote add` parse the same path in different ways. Git already has many inconsistencies. This one can be easily ironed out. In what way do you think that `git remote add` handles the path? All `git remote add` does is add a new remote.name.url entry to the configuration file with the value as given on the command line. The insteadOf mapping will only be applied when you try to fetch from/push to the remote. -- 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 clone does not understand insteadOf URLs
On Sun, May 26, 2013 at 08:21:45PM +0200, Gioele Barabucci wrote:
Il 26/05/2013 20:14, John Keeping ha scritto:
On Sun, May 26, 2013 at 08:09:56PM +0200, Gioele Barabucci wrote:
Il 26/05/2013 20:00, Andreas Schwab ha scritto:
Simple, I keep all my projects on the same server, so I would like to
refer to that server + path using 'remote-repo'.
git+ssh://git.example.org//users/gioele/projects insteadOf
remote-repo
In what way do you think that `git remote add` handles the path?
All `git remote add` does is add a new remote.name.url entry to the
configuration file with the value as given on the command line. The
insteadOf mapping will only be applied when you try to fetch from/push
to the remote.
Regardless of the implementation of the commands, if I do
mkdir projectA
cd projectA
git init .
git remote add origin remote-repo/projectA.git
git pull origin master
I get a working repository. If I do
git clone remote-repo/projectA.git
all I will get is an error.
So the problem is that git clone does not seem to perform normal
remote processing if you give it something that looks like a path.
More specifically, it looks like the problem is that if you give clone
something that does not contain a colon (':') it considers it to be a
local path and dies if that path does not exist. Adding a colon as
Andreas suggested makes it look like a remote URL so it will be handled
correctly.
--
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] fetch: don't try to update unfetched tracking refs
Since commit f269048 (fetch: opportunistically update tracking refs,
2013-05-11) we update tracking refs opportunistically when fetching
remote branches. However, if a refspec is given on the command line
that does not include a configured (non-pattern) refspec a fatal error
occurs.
Fix this by setting the missing_ok flag when calling get_fetch_map.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e41cc0d..d15a734 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -183,7 +183,7 @@ static struct ref *get_ref_map(struct transport *transport,
old_tail = tail;
for (i = 0; i transport-remote-fetch_refspec_nr; i++)
get_fetch_map(ref_map, transport-remote-fetch[i],
- tail, 0);
+ tail, 1);
for (rm = *old_tail; rm; rm = rm-next)
rm-fetch_head_status = FETCH_HEAD_IGNORE;
} else {
--
1.8.3.rc3.438.gb3e4ae3
--
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] fetch: don't try to update unfetched tracking refs
On Mon, May 27, 2013 at 11:42:52AM -0400, Jeff King wrote: On Mon, May 27, 2013 at 12:40:25PM +0100, John Keeping wrote: Since commit f269048 (fetch: opportunistically update tracking refs, 2013-05-11) we update tracking refs opportunistically when fetching remote branches. However, if a refspec is given on the command line that does not include a configured (non-pattern) refspec a fatal error occurs. I'm not sure I understand what the last sentence means. I tried to add a test like: diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ff43e08..02e30e1 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -422,6 +422,16 @@ test_expect_success 'configured fetch updates tracking' ' ) ' +test_expect_success 'non-configured ref does not confuse tracking update' ' + cd $D + git update-ref refs/odd/location HEAD + ( + cd three + git fetch origin refs/odd/location + git rev-parse --verify FETCH_HEAD + ) +' + test_expect_success 'pushing nonexistent branch by mistake should not segv' ' cd $D but it does not fail with the existing code. Can you give an example that fails? I have this in my .git/config for git.git: [remote origin] url = git://github.com/gitster/git fetch = +refs/heads/*:refs/remotes/origin/* fetch = +refs/notes/amlog:refs/notes/amlog Then doing git fetch origin master fails because: fatal: Couldn't find remote ref refs/notes/amlog The following test fails for me (and passes with my patch) - note that in two, remote.one.fetch is configured as refs/heads/master:refs/heads/one. -- 8 -- diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ff43e08..c540257 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -422,6 +422,19 @@ test_expect_success 'configured fetch updates tracking' ' ) ' +test_expect_success 'configured ref does not confuse tracking' ' + + cd $D + ( + cd one + git branch -f side + ) + ( + cd two + git fetch one side + ) +' + test_expect_success 'pushing nonexistent branch by mistake should not segv' ' cd $D -- 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] fetch: don't try to update unfetched tracking refs
Since commit f269048 (fetch: opportunistically update tracking refs,
2013-05-11) we update tracking refs opportunistically when fetching
remote branches. However, if there is a configured non-pattern refspec
that does not match any of the refspecs given on the command line then a
fatal error occurs.
Fix this by setting the missing_ok flag when calling get_fetch_map.
Test-added-by: Jeff King p...@peff.net
Signed-off-by: John Keeping j...@keeping.me.uk
Acked-by: Jeff King p...@peff.net
---
On Mon, May 27, 2013 at 12:19:34PM -0400, Jeff King wrote:
On Mon, May 27, 2013 at 05:01:29PM +0100, John Keeping wrote:
I'm not sure I understand what the last sentence means. I tried to add a
test like:
[...]
but it does not fail with the existing code. Can you give an example
that fails?
I have this in my .git/config for git.git:
[remote origin]
url = git://github.com/gitster/git
fetch = +refs/heads/*:refs/remotes/origin/*
fetch = +refs/notes/amlog:refs/notes/amlog
Ah, I see. It is not the refspec on the command-line does not match a
configured refspec, but rather there exists a configured non-pattern
refspec that does not match what was on the command-line (even if what
was on the command-line did match another refspec).
Exactly. I've changed the commit message to (hopefully) make this
clearer.
So your fix makes perfect sense. Do you mind squashing in this test
below? I think it is a little less subtle than what you posted, as it
sets up the situation explicitly in the test. It also checks that the
refs we _did_ match still get updated (master in this case).
Done.
builtin/fetch.c | 2 +-
t/t5510-fetch.sh | 16
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e41cc0d..d15a734 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -183,7 +183,7 @@ static struct ref *get_ref_map(struct transport *transport,
old_tail = tail;
for (i = 0; i transport-remote-fetch_refspec_nr; i++)
get_fetch_map(ref_map, transport-remote-fetch[i],
- tail, 0);
+ tail, 1);
for (rm = *old_tail; rm; rm = rm-next)
rm-fetch_head_status = FETCH_HEAD_IGNORE;
} else {
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index ff43e08..fde6891 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -422,6 +422,22 @@ test_expect_success 'configured fetch updates tracking' '
)
'
+test_expect_success 'non-matching refspecs do not confuse tracking update' '
+ cd $D
+ git update-ref refs/odd/location HEAD
+ (
+ cd three
+ git update-ref refs/remotes/origin/master base-origin-master
+ git config --add remote.origin.fetch \
+ refs/odd/location:refs/remotes/origin/odd
+ o=$(git rev-parse --verify refs/remotes/origin/master)
+ git fetch origin master
+ n=$(git rev-parse --verify refs/remotes/origin/master)
+ test $o != $n
+ test_must_fail git rev-parse --verify refs/remotes/origin/odd
+ )
+'
+
test_expect_success 'pushing nonexistent branch by mistake should not segv' '
cd $D
--
1.8.3.rc3.438.gb3e4ae3
--
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-send-email doesn't deal with quoted names
On Tue, May 28, 2013 at 01:40:20AM +0200, Jason A. Donenfeld wrote:
My commit author name is Jason A. Donenfeld. Because this has a dot,
SMTP handling likes to put it in quotes.
git-send-email has this line:
if (defined $author and $author ne $sender) {
With my name, this always winds up false, because it's comparing
'Jason A. Donenfeld ja...@zx2c4.com' with 'Jason A. Donenfeld
ja...@zx2c4.com'.
So, the logic needs to be fixed somehow.
There was a patch for this recently, although it appears to be still
under discussion:
http://article.gmane.org/gmane.comp.version-control.git/225247
--
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] difftool --dir-diff: always use identical working tree file
On Tue, May 28, 2013 at 11:06:13AM -0700, Junio C Hamano wrote:
Kenichi Saita nito...@gmail.com writes:
When deciding whether or not we should link a working tree file into
the temporary right-hand directory for a directory diff, we
currently behave differently in the --symlink and --no-symlink
cases. If using symlinks any identical files are linked across but
with --no-symlink only files that contain unstaged changes are
copied back into the working tree.
I may have missed an earlier discussion, but I do not follow the
last sentence. The former part (i.e. symlinks) talks about what is
done to populate the temporary tree (i.e. everything is linked), but
the latter part (i.e. not symlinks) only talks about what is copied
back, i.e. it is not a contrast between the behaviour of symlink vs
no-symlink case wrt how the temporary tree is populated.
Confused...
Yeah, the commit message is still quite focused on the end effect of
copying files back. But that's not what's being changed here.
In my suggested commit message I tried to make it clear that we're
changing when we decide to copy a file across to the temporary tree.
This has the beneficial (side-)effect of changing the set of files we
consider for copying back into the working tree after the diff tool has
been run.
Change this so that identical files are copied back as well. This
is beneficial because it widens the set of circumstances in which we
copy changes made by the user back into the working tree.
Ah, OK, you meant that the set of files we keep in @working_tree
array for later copying back are different between the two.
Signed-off-by: Kenichi Saita nito...@gmail.com
---
git-difftool.perl |9 ++---
t/t7800-difftool.sh | 19 +++
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index 8a75205..e57d3d1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -85,13 +85,9 @@ sub exit_cleanup
sub use_wt_file
{
- my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+ my ($repo, $workdir, $file, $sha1) = @_;
my $null_sha1 = '0' x 40;
- if ($sha1 ne $null_sha1 and not $symlinks) {
- return 0;
- }
-
if (! -e $workdir/$file) {
# If the file doesn't exist in the working tree, we cannot
# use it.
@@ -213,8 +209,7 @@ EOF
if ($rmode ne $null_mode) {
my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
- $dst_path, $rsha1,
- $symlinks);
+ $dst_path, $rsha1);
if ($use) {
push @working_tree, $dst_path;
$wtindex .= $rmode $wt_sha1\t$dst_path\0;
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index d46f041..2418528 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -385,6 +385,25 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff
--symlink without unstage
test_cmp actual expect
'
+write_script modify-right-file \EOF
+echo new content $2/file
+EOF
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree with unstaged
change' '
+ test_when_finished git reset --hard
+ echo orig content file
+ git difftool -d $symlinks --extcmd $(pwd)/modify-right-file branch
+ echo new content expect
+ test_cmp expect file
+'
+
+run_dir_diff_test 'difftool --dir-diff syncs worktree without unstaged
change' '
+ test_when_finished git reset --hard
+ git difftool -d $symlinks --extcmd $(pwd)/modify-right-file branch
+ echo new content expect
+ test_cmp expect file
+'
+
write_script modify-file \EOF
echo new content file
EOF
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool --dir-diff: always use identical working tree file
On Tue, May 28, 2013 at 11:57:08AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Yeah, the commit message is still quite focused on the end effect of copying files back. But that's not what's being changed here. In my suggested commit message I tried to make it clear that we're changing when we decide to copy a file across to the temporary tree. This has the beneficial (side-)effect of changing the set of files we consider for copying back into the working tree after the diff tool has been run. I actually think the effect of copying files back _is_ the primary motivation of this change, and stressing that end effect is a much better description. After all, if the working tree files do not have any difference from the RHS of the comparison, copying from the working tree and stuffing the $rsha1 to the RHS temporary index and running checkout -f should produce identical temporary directory for the user to start viewing. The _only_ difference in behaviour before and after this patch that matters to the end user is if that path is in @working_tree, which is returned to @worktree of dir_diff sub to be later copied back, no? I would view this change as a mere means, an implementation detail, to achieve that end of stuffing more paths in the @worktree array. I agree with this, but like you I found it confusing that the patch touched code seemingly unrelated to copying files back. I went toward describing the patch more literally and giving the motivation in the final paragraph. Your message below is better, but I think it needs to say that the set of files considered for copying back is the set that is copied across to begin with. Perhaps difftool --dir-diff: allow changing any clean working tree file The temporary directory prepared by difftool --dir-diff to show the result of a change can be modified by the user via the tree diff program, and we try hard not to lose changes to them after tree diff program returns to us. However, the set of files to be copied back is computed differently between --symlinks and --no-symlinks modes. The former checks all paths that start out as identical to the working tree file, while the latter checks paths that already had a local modification in the working tree, allowing changes made in the tree diff program to paths that did not have any local change to be lost. or something. This invites a few questions, though. - By allowing these files in the temporary directory to be modified, aren't we making the user's life harder by forcing them to handle working tree file was already modified, made different changes in the temporary directory, now these changes need to be consolidated case? - When comparing two revisions, e.g. --dir-diff HEAD^^ HEAD^, that checks out (via $rsha1 to checkout -f codepath) a blob that does not match what is in the working tree of HEAD to the temporary directory, we still allow modifications to the copy in the temporary directory, but what can the user do with these changes that are _not_ based on HEAD, short of checking out HEAD^ and apply the difference first? I still cannot shake this nagging feeling that giving a writable temporary directory might have been a mistake in the first place. Perhaps it may be a better design to make the ones that the user shouldn't touch (or will lead to the above confusion) read-only, while the ones that match the working tree read-write? My ideal scenario would be that we only allow users to edit files when they are comparing against the working tree, but that would require git-difftool to fully understand all git-diff options since it just passes through any it doesn't recognise. I don't think there's an easy way to do that, which leaves us with this confusing situation. -- 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: Poor performance of git describe in big repos
On Thu, May 30, 2013 at 11:38:32AM +0100, Alex Bennée wrote: One factor might be the size of my repo (.git is around 2.4G). Could this just be due to computational cost of searching through large packs to walk the commit chain? Is there any way to make this easier for git to do? What does git count-objects -v say for your repository? You may find that performance improves if you repack with git gc --aggressive. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-gui: fix file name handling with non-empty prefix
In the hope that the Pat Thoyts who just posted in another thread from a
GMail address is the same one that maintains git-gui, let's see if that
address works...
On Sat, May 11, 2013 at 10:03:25PM -0400, Andrew Wong wrote:
Sorry for the late reply. I was able to reproduce the problem that you
were describing a while ago. And your patch indeed fixes it. It's a much
more elegant way of dealing with the absolute vs relative path problem
that I was trying to fix.
Thanks!
As for Pat, I'm not sure wha'ts going on with his email address. It was
working back in October, and his username still seems to be active over
at SourceForge... let's see if this email reaches him.
Here's a link for his reference just in case he missed your original email:
http://thread.gmane.org/gmane.comp.version-control.git/222646
On 04/27/13 10:18, John Keeping wrote:
I got a bounce with 550 no such user for Pat's email address when
sending this. Does anyone have more up-to-date contact details? Or is
it just SourceForge being broken?
On Sat, Apr 27, 2013 at 02:24:16PM +0100, John Keeping wrote:
Commit e3d06ca (git-gui: Detect full path when parsing arguments -
2012-10-02) fixed the handling of absolute paths passed to the browser
and blame subcommands by checking whether the file exists without the
prefix before prepending the prefix and checking again. Since we have
chdir'd to the top level of the working tree before doing this, this
does not work if a file with the same name exists in a subdirectory and
at the top level (for example Makefile in git.git's t/ directory).
Instead of doing this, revert that patch and fix absolute path issue by
using file join to prepend the prefix to the supplied path. This will
correctly handle absolute paths by skipping the prefix in that case.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-gui.sh | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index e11..a94ad7f 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3003,19 +3003,11 @@ blame {
set jump_spec {}
set is_path 0
foreach a $argv {
- if {[file exists $a]} {
- if {$path ne {}} usage
- set path [normalize_relpath $a]
- break
- } elseif {[file exists $_prefix$a]} {
- if {$path ne {}} usage
- set path [normalize_relpath $_prefix$a]
- break
- }
+ set p [file join $_prefix $a]
- if {$is_path} {
+ if {$is_path || [file exists $p]} {
if {$path ne {}} usage
- set path [normalize_relpath $_prefix$a]
+ set path [normalize_relpath $p]
break
} elseif {$a eq {--}} {
if {$path ne {}} {
--
1.8.3.rc0.149.g98a72f2.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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: Should git help respect the 'pager' setting?
On Thu, May 30, 2013 at 10:38:59PM +0530, Ramkumar Ramachandra wrote: Matthieu Moy wrote: I find it a bit weird that Git sets the configuration for external commands, but it may make sense. No strong opinion here. I don't mean a setenv() kind of thing: how would we unset it after that? Perhaps something like execvpe(), passing in the environment as an argument? Overriding PAGER might make sense, but I'd be quite annoyed if Git decided to override MANPAGER without providing some way to override it. If a user sets MANPAGER then it's because they want a specific pager when reading man pages - invoking man through git help shouldn't cause it to behave differently in this case. -- 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: Poor performance of git describe in big repos
On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote:
Alex Bennée kernel-hac...@bennee.com writes:
On 30 May 2013 16:33, Thomas Rast tr...@inf.ethz.ch wrote:
Alex Bennée kernel-hac...@bennee.com writes:
41.58% git libcrypto.so.1.0.0 [.] sha1_block_data_order_ssse3
33.62% git libz.so.1.2.3.4 [.] inflate_fast
10.39% git libz.so.1.2.3.4 [.] adler32
2.03% git [kernel.kallsyms] [k] clear_page_c
Do you have any large blobs in the repo that are referenced directly by
a tag?
Most probably. I've certainly done a bunch of releases (which are tagged)
were
the last thing that was updated was an FPGA image.
[...]
git-describe should probably be fixed to avoid loading blobs, though I'm
not sure off hand if we have any infrastructure to infer the type of a
loose object without inflating it. (This could probably be added by
inflating only the first block.) We do have this for packed objects, so
at least for packed repos there's a speedup to be had.
Will it be loading the blob for every commit it traverses or just ones that
hit
a tag? Why does it need to load the blob at all? Surely the commit
tree state doesn't
need to be walked down?
No, my theory is that you tagged *the blobs*. Git supports this.
You can see if that is the case by doing something like this:
eval $(git for-each-ref --shell --format '
test $(git cat-file -t %(objectname)^{}) = commit ||
echo %(refname);')
That will print out the name of any ref that doesn't point at a commit.
--
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: Poor performance of git describe in big repos
On Fri, May 31, 2013 at 09:14:49AM +0100, Alex Bennée wrote:
On 30 May 2013 20:30, John Keeping j...@keeping.me.uk wrote:
On Thu, May 30, 2013 at 06:21:55PM +0200, Thomas Rast wrote:
Alex Bennée kernel-hac...@bennee.com writes:
On 30 May 2013 16:33, Thomas Rast tr...@inf.ethz.ch wrote:
Alex Bennée kernel-hac...@bennee.com writes:
snip
Will it be loading the blob for every commit it traverses or just ones
that hit
a tag? Why does it need to load the blob at all? Surely the commit
tree state doesn't
need to be walked down?
No, my theory is that you tagged *the blobs*. Git supports this.
Wait is this the difference between annotated and non-annotated tags?
I thought a non-annotated just acted like references to a particular
tree state?
No, this is something slightly different. In Git there are four types
of object: tag, commit, tree and blob. When you have a heavyweight tag,
the tag reference points at a tag object (which in turn points at
another object). With a lightweight tag, the tag reference typically
points at a commit object.
However, there is no restriction that says that a tag object must point
to a commit or that a lightweight tag must point at a commit - it is
equally possible to point directly at a tree or a blob (although a lot
less common).
Thomas is suggesting that you might have a tag that does not point at a
commit but instead points to a blob object.
You can see if that is the case by doing something like this:
eval $(git for-each-ref --shell --format '
test $(git cat-file -t %(objectname)^{}) = commit ||
echo %(refname);')
That will print out the name of any ref that doesn't point at a
commit.
Hmm that didn't seem to work.
You mean there was no output? In that case it's likely that all your
references do indeed point at commits.
But looking at the output by hand I
certainly have a mix of tags that are commits vs tags:
09:08 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep commit | wc -l
1345
09:12 ajb@sloy/x86_64 [work.git] git for-each-ref | grep refs/tags
| grep -v commit | wc -l
66
This means that you have 1345 lightweight tags and 66 heavyweight tags,
assuming that all of the lines that don't say commit do say tag.
By the way, I don't remember if you said which version of Git you're
using. If it's an older version then it's possible that something has
changed.
--
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: Poor performance of git describe in big repos
On Fri, May 31, 2013 at 09:49:57AM +0100, Alex Bennée wrote: On 31 May 2013 09:32, John Keeping j...@keeping.me.uk wrote: Thomas is suggesting that you might have a tag that does not point at a commit but instead points to a blob object. It's looking like I just have some very heavy commits. One data point I probably should have mentioned at the beginning is this was a converted CVS repo and I'm wondering if some of the artifacts that introduced has contributed to this. You can try another for-each-ref invocation to see if that's the case: eval $(git for-each-ref --format 'printf %s %s\n \ $(git cat-file -s %(objectname)) %(refname);') | sort -n That will print the size of each object followed by the ref that points to it, sorted by size. I'm running the GIT stable PPA: 09:38 ajb@sloy/x86_64 [work.git] git --version git version 1.8.3 Although I have also tested with the latest git.git maint. I'm happy to try master if it's likely to have changed. master's still very close to 1.8.3 at the moment, so I don't think that will make a difference. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Thu, May 30, 2013 at 09:23:40PM +0200, Jens Lehmann wrote: Am 30.05.2013 01:58, schrieb Junio C Hamano: * jk/submodule-subdirectory-ok (2013-04-24) 3 commits (merged to 'next' on 2013-04-24 at 6306b29) + submodule: fix quoting in relative_path() (merged to 'next' on 2013-04-22 at f211e25) + submodule: drop the top-level requirement + rev-parse: add --prefix option Allow various subcommands of git submodule to be run not from the top of the working tree of the superproject. The summary and status commands are looking good in this version (they are now showing the submodule directory paths relative to the current directory). Apart from that my other remarks from gmane $221575 still seem to apply. And this series has only tests for status, summary and add (and that just with an absolute URL), I'd rather like to see a test for each submodule command (and a relative add to) to document the desired behavior. To summarize what I think are the outstanding issues from your email: * Should '$sm_path' be relative in submodule foreach? * submodule add with a relative path * submodule init initializes all submodules * Tests The current version does make '$sm_path' relative in submodule foreach, although it's hard to spot because we have to leave doing so until right before the eval. I'm not sure what you mean about submodule add - the new version treats the path argument as relative (providing it is not an absolute path). The repository argument is not changed by running from a subdirectory but I think that's correct since it is documented as being relative to the superproject's origin repository. submodule init is behaving in the same way as deinit - if you say submodule init . then it will only initialize submodules below the current directory. The difference is that deinit dies if it is not given any arguments whereas init will initialize everything from the top level down. I'm not sure whether to change this; given the direction git add -u is heading in for 2.0 I think the current behaviour is the most consistent with the rest of Git. But I'm not sure if it's better to have another iteration of this series or to address the open issues a follow-up series. Having status, summary and add - at least with absolute URLs - lose the toplevel requirement is already a huge improvement IMO. Opinions? I think the only thing outstanding is tests. I'm happy to add those as a follow-up or in a re-roll. -- 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: ls-files -i directories
On Fri, May 31, 2013 at 04:22:37PM -0400, Roland Schulz wrote: Hi, the gitignore rules work so that if a directory is ignored, all files in that directory are ignored. While that behavior isn't clearly documented in gitignore, this behavior is consistent across all git tools (status, ls-files, ...). An exception is that listing the ignored files using ls-files -i doesn't behave the same way. example: $ mkdir d $ touch d/f $ echo /d/ .gitignore $ git ls-files -o --exclude-standard .gitignore #d/f is correctly not listed $ git ls-files -i --exclude-standard #no output d/f isn't listed even though it is treated as an ignored file by all other git tools. That seems inconsistent to me. Is that behavior intentionally or is this a bug? It is listed with git ls-files -i -o --exclude-standard. The documentation says: Show only ignored files in the output. When showing files in the index, print only those matched by an exclude pattern. When showing other files, show only those matched by an exclude pattern. If you do this then it is shown: $ git add -f d/f $ git ls-files -i --exclude-standard d/f I think this is working as documented. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Mon, Jun 03, 2013 at 11:47:23PM +0200, Jens Lehmann wrote: Am 31.05.2013 21:40, schrieb John Keeping: The current version does make '$sm_path' relative in submodule foreach, although it's hard to spot because we have to leave doing so until right before the eval. Yes. If I read the code correctly the submodule is cd'ed in before the foreach command is executed, so $sm_path should only be used for displaying info about where the command is executed anyway. Looks like your code is doing the right thing adjusting $sm_path to be relative to the directory the user is in. But a test showing that would really be nice ;-) Agreed. I've also noticed that the legacy path variable hasn't been adjusted and the printing of the module paths does not make them relative. I'll fix them in the next version. I'm not sure what you mean about submodule add - the new version treats the path argument as relative (providing it is not an absolute path). The repository argument is not changed by running from a subdirectory but I think that's correct since it is documented as being relative to the superproject's origin repository. Sorry, I should have been more specific here. I saw that you did some changes to make submodule add do the right thing with relative paths, but the following change to t7406 does not work like I believe it should but instead makes the test fail: ---8- diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index a4ffea0..9766b9e 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa test_expect_success 'submodule add places git-dir in superprojects git-dir' ' (cd super mkdir deeper -git submodule add ../submodule deeper/submodule +(cd deeper + git submodule add ../../submodule submodule +) (cd deeper/submodule git log ../../expected ) ---8- Ah, ok. I think this case is problematic because the repository argument is either relative to remote.origin.url or to the top of the working tree if there is no origin remote. I wonder if we should just die when a relative path is given for the repository and we're not at the top of the working tree. submodule init is behaving in the same way as deinit - if you say submodule init . then it will only initialize submodules below the current directory. The difference is that deinit dies if it is not given any arguments whereas init will initialize everything from the top level down. I'm not sure whether to change this; given the direction git add -u is heading in for 2.0 I think the current behaviour is the most consistent with the rest of Git. I meant that both commands still print the submodule names from the top-level directory, not the one the user is in. Will fix. -- 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: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Tue, Jun 04, 2013 at 09:17:17PM +1000, Heiko Voigt wrote: On Tue, Jun 04, 2013 at 09:10:45AM +0100, John Keeping wrote: On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote: On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote: Sorry, I should have been more specific here. I saw that you did some changes to make submodule add do the right thing with relative paths, but the following change to t7406 does not work like I believe it should but instead makes the test fail: ---8- diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index a4ffea0..9766b9e 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to the same pa test_expect_success 'submodule add places git-dir in superprojects git-dir' ' (cd super mkdir deeper -git submodule add ../submodule deeper/submodule +(cd deeper + git submodule add ../../submodule submodule +) (cd deeper/submodule git log ../../expected ) ---8- Ah, ok. I think this case is problematic because the repository argument is either relative to remote.origin.url or to the top of the working tree if there is no origin remote. I wonder if we should just die when a relative path is given for the repository and we're not at the top of the working tree. Why not behave as if we are at the top of the working tree for relative paths? If there is an origin remote thats fine. If there is no origin remote you could warn that the path used is taken relative from the root of the superproject during add. What do you think? That's what the patch currently queued on pu does, which Jens wants to change, isn't it? True I did not realize this when reading it the first time. But I think we should still not die when in a subdirectory. After all this series is trying to archive that the submodule command works in subdirectories seamlessly right? So you probably want to translate a relative path without origin remote given from a subdirectory to the superproject level and use that. Then you do not have to die. The problem is that sometimes you do want to adjust the path and sometimes you don't. Reading git-submodule(1), it says: This may be either an absolute URL, or (if it begins with ./ or ../), the location relative to the superproject’s origin repository. [snip] If the superproject doesn’t have an origin configured the superproject is its own authoritative upstream and the current working directory is used instead. So I think it's quite reasonable to have a server layout that looks like this: project |- libs | |- libA | `- libB |- core.git and with only core.git on your local system do: cd core/libs git submodule add ../libs/libB expecting that to point to libB. But if we adjust the path then the user has to do: git submodule add ../../libs/libB However, it is also perfectly reasonable to have no remote configured and the library next to the repository itself. In which case we do want to specify the additional ../ so that shell completion works in the natural way. The only way I can see to resolve the ambiguity is to die when we hit this particular case. This should be acceptable because people shouldn't be adding new submodules anywhere near as often as they perform other submodule operations and it doesn't affect absolute URLs. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Tue, Jun 04, 2013 at 11:39:25PM +0200, Jens Lehmann wrote:
Am 04.06.2013 14:48, schrieb John Keeping:
The problem is that sometimes you do want to adjust the path and
sometimes you don't. Reading git-submodule(1), it says:
This may be either an absolute URL, or (if it begins with ./ or
../), the location relative to the superproject’s origin
repository.
[snip]
If the superproject doesn’t have an origin configured the
superproject is its own authoritative upstream and the current
working directory is used instead.
So I think it's quite reasonable to have a server layout that looks like
this:
project
|- libs
| |- libA
| `- libB
|- core.git
and with only core.git on your local system do:
cd core/libs
git submodule add ../libs/libB
expecting that to point to libB. But if we adjust the path then the
user has to do:
git submodule add ../../libs/libB
However, it is also perfectly reasonable to have no remote configured
and the library next to the repository itself. In which case we do want
to specify the additional ../ so that shell completion works in the
natural way.
Exactly.
The only way I can see to resolve the ambiguity is to die when we hit
this particular case.
Hmm, I'm not so sure about that. Don't the first three lines in
resolve_relative_url() show how to distinguish between these two
cases?
resolve_relative_url ()
{
remote=$(get_default_remote)
remoteurl=$(git config remote.$remote.url) ||
remoteurl=$(pwd) # the repository is its own authoritative
upstream
...
If it's this simple, yes. But I think there's also a third possibility
that combines both of these: what if the local directory structure is
the same as that on the origin remote? Then origin exists but we
still want to adjust for the subdirectory.
The risk is that I can't see a behaviour that doesn't seem to choose
whether to convert the given path or not arbitrarily. Even knowing the
rules, I expect that I could end up being surprised by this if I create
a new repository and haven't set up origin yet.
--
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: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)
On Tue, Jun 04, 2013 at 06:57:34PM -0400, Phil Hord wrote: On Tue, Jun 4, 2013 at 8:48 AM, John Keeping j...@keeping.me.uk wrote: The problem is that sometimes you do want to adjust the path and sometimes you don't. Reading git-submodule(1), it says: This may be either an absolute URL, or (if it begins with ./ or ../), the location relative to the superproject’s origin repository. [snip] If the superproject doesn’t have an origin configured the superproject is its own authoritative upstream and the current working directory is used instead. So I think it's quite reasonable to have a server layout that looks like this: project |- libs | |- libA | `- libB |- core.git and with only core.git on your local system do: cd core/libs git submodule add ../libs/libB expecting that to point to libB. But if we adjust the path then the user has to do: git submodule add ../../libs/libB However, it is also perfectly reasonable to have no remote configured and the library next to the repository itself. In which case we do want to specify the additional ../ so that shell completion works in the natural way. In submodule add, the leading '../' prefix on the repository url has always meant that the url is relative to the url of the current repo. The given repo-url is precisely what ends up in .gitmodules' submodule.$name.url. It works this way whether there is a remote configured or not. It does seem like we need to be cautious around this change. The only way I can see to resolve the ambiguity is to die when we hit this particular case. This should be acceptable because people shouldn't be adding new submodules anywhere near as often as they perform other submodule operations and it doesn't affect absolute URLs. I don't think I like that. But I don't know if I like anything I dreamed up, either. I've decided that I will make it die (unless anyone comes up with an obviously correct solution before I get around to sending the reroll) because it's a lot easier to loosen that in the future than to change it if we get the behaviour wrong now. I don't want to hold every other subcommand hostage to this one case. -- 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] build: get rid of the notion of a git library
On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
The plan is simple; make libgit.a a proper library, starting by
clarifying what goes into libgit.a, and what doesn't. If there's any
hopes of ever having a public library, it's clear what code doesn't
belong in libgit.a; code that is meant for builtins, that code belongs
in builtins/lib.a, or similar.
Give this a try:
--- a/sequencer.c
+++ b/sequencer.c
libgit.a(sequencer.o): In function `copy_notes':
/home/felipec/dev/git/sequencer.c:110: undefined reference to
`init_copy_notes_for_rewrite'
/home/felipec/dev/git/sequencer.c:114: undefined reference to
`finish_copy_notes_for_rewrite'
This is a good example: yes, I'm convinced that the code does need to
be reorganized. Please resend your {sequencer.c -
builtin/sequencer.c} patch with this example as the rationale, and
let's work towards improving libgit.a.
Why should sequencer.c move into builtin/ to solve this? Why not pull
init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into
notes.c?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] build: get rid of the notion of a git library
On Sun, Jun 09, 2013 at 10:40:32AM -0500, Felipe Contreras wrote:
On Sun, Jun 9, 2013 at 10:12 AM, John Keeping j...@keeping.me.uk wrote:
On Sun, Jun 09, 2013 at 08:26:32PM +0530, Ramkumar Ramachandra wrote:
Felipe Contreras wrote:
The plan is simple; make libgit.a a proper library, starting by
clarifying what goes into libgit.a, and what doesn't. If there's any
hopes of ever having a public library, it's clear what code doesn't
belong in libgit.a; code that is meant for builtins, that code belongs
in builtins/lib.a, or similar.
Give this a try:
--- a/sequencer.c
+++ b/sequencer.c
libgit.a(sequencer.o): In function `copy_notes':
/home/felipec/dev/git/sequencer.c:110: undefined reference to
`init_copy_notes_for_rewrite'
/home/felipec/dev/git/sequencer.c:114: undefined reference to
`finish_copy_notes_for_rewrite'
This is a good example: yes, I'm convinced that the code does need to
be reorganized. Please resend your {sequencer.c -
builtin/sequencer.c} patch with this example as the rationale, and
let's work towards improving libgit.a.
Why should sequencer.c move into builtin/ to solve this? Why not pull
init_copy_notes_for_rewrite and finish_copy_notes_for_rewrite up into
notes.c?
Because finish_copy_notes_for_rewrite is only useful for builtin
commands, so it belongs in builtin/. If there's any meaning to the
./*.o vs. builtin/*.o divide, it's for that. Otherwise we should just
squash all objects into libgit.a and be done with it.
How is it only useful for builtin commands? By that logic everything
belongs in builtin/ because it's all only used by builtin commands (I
realise that is what you're arguing towards).
But we make a distinction between things that are specific to one
command (especially argument parsing and user interaction) and more
generally useful features. Copying notes around in the notes tree is
generally useful so why shouldn't it be in notes.c with the other note
manipulation functions? The current API may not be completely suitable
but that doesn't mean that it cannot be extracted into notes.c. In
fact, other than the commit message saying Notes added by 'git notes
copy' I don't see what's wrong with the current functions being
extracted 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] build: get rid of the notion of a git library
On Sun, Jun 09, 2013 at 11:22:06AM -0500, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 11:02 AM, John Keeping j...@keeping.me.uk wrote: But we make a distinction between things that are specific to one command (especially argument parsing and user interaction) and more generally useful features. No, we don't. Everything under ./*.o goes to libgit.a, and everything under ./builtin/*.o goes to 'git'. So builtin/commit.o can access code from builtin/notes.o, but sequencer.o can't. I would argue that it was a mistake not to extract these functions from builtin/notes.c to notes.c when builtin/commit.c started using them. Calling across from one builtin/*.c file to another is just as wrong as calling into a builtin/*.c file from a top-level file but the build system happens not to enforce the former. -- 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] build: get rid of the notion of a git library
On Sun, Jun 09, 2013 at 12:13:41PM -0500, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 12:03 PM, Ramkumar Ramachandra artag...@gmail.com wrote: John Keeping wrote: Calling across from one builtin/*.c file to another is just as wrong as calling into a builtin/*.c file from a top-level file but the build system happens not to enforce the former. So libgit.a is a collection of everything that is shared between builtins? Does that correspond to reality? I think that's *precisely* what libgit.a is. It doesn't currently correspond exactly to reality, but that's mostly for historic reasons (see below). $ ls *.h | sed 's/.h$/.c/' | xargs file An example violation: builtin/log.c uses functions defined in builtin/shortlog.c. What is the point of all this separation, if no external scripts are ever going to use libgit.a? Why do we structure code in a certain way at all? The reason libgit.a was introduced (according to commit 0a02ce7) is: This introduces the concept of git library objects that the real programs use, and makes it easier to add such things to a libgit.a. And all the functions should be static, which doesn't seem to be the case: 03c0 T add_files_to_cache 0530 T interactive_add 0410 T run_add_interactive 1920 T textconv_object 05b0 T fmt_merge_msg 0090 T fmt_merge_msg_config 0c00 T init_db 0b40 T set_git_dir_init 0360 T overlay_tree_on_cache 0500 T report_path_error 11a0 T copy_note_for_rewrite 1210 T finish_copy_notes_for_rewrite 1060 T init_copy_notes_for_rewrite T prune_packed_objects 0510 T shortlog_add_commit 06b0 T shortlog_init 0780 T shortlog_output T stripspace A quick check with git log -S... shows that most of these have barely been touched since the builtin/ directory was created. So the reason they're not static is most likely because no one has tidied them up since the division between builtins was introduced. It is a fact of life that as we live and work with a system we realise that there may be a better way of doing something. This doesn't mean that someone needs to immediately convert everything to the new way, it is often sufficient to do new things in the new way and slowly move existing things across as and when they are touched for other reasons. -- 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 v4 10/45] sequencer: trivial fix
On Sun, Jun 09, 2013 at 07:33:42PM +0200, SZEDER Gábor wrote: On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote: On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com A shortlog-friendlier subject could be: sequencer: free objects before leaving. I already defended my rationale for this succinct commit message: http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610 Your arguments were unconvincing. The mere fact that I raised this issue unbeknownst to the earlier posting clearly shows that there's demand for descriptive subjects. Not to mention that with your subject no body is needed, making the overall message more succinct. When reading a log, as soon as I see trivial I become suspicious that someone is trying to cover something up, much like left as an exercise for the reader. If the subject says fix memory leak then it's obvious what the patch is meant to do, and when there is no subtlety to be explained (as there isn't in this patch) there is no need for a body. -- 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 v4 10/45] sequencer: trivial fix
On Sun, Jun 09, 2013 at 12:53:38PM -0500, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 12:37 PM, John Keeping j...@keeping.me.uk wrote: On Sun, Jun 09, 2013 at 07:33:42PM +0200, SZEDER Gábor wrote: On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor sze...@ira.uka.de wrote: On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote: We should free objects before leaving. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com A shortlog-friendlier subject could be: sequencer: free objects before leaving. I already defended my rationale for this succinct commit message: http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610 Your arguments were unconvincing. The mere fact that I raised this issue unbeknownst to the earlier posting clearly shows that there's demand for descriptive subjects. Not to mention that with your subject no body is needed, making the overall message more succinct. It's not succinct at all, because there's no short and quick description of what the patch actually is; a trivial fix. Is it not equally succinct to say fix memory leak? When reading a log, as soon as I see trivial I become suspicious that someone is trying to cover something up, much like left as an exercise for the reader. If the subject says fix memory leak then it's obvious what the patch is meant to do, and when there is no subtlety to be explained (as there isn't in this patch) there is no need for a body. You are not a rational person then. The commit message has absolutely no bearing on the quality of the code. If you are less suspicious of a commit message that says fix memory leak, you are being completely biased. Whether the commit message says fix memory leak, or trivial fix, or foobar, the code might still be doing something wrong, and you can't decide that until you look at the code. I have a certain level of trust that commit summaries in git.git will be accurate. If I want to know what has changed, then fix memory leak implies no functional change; if I see trivial fix then how do I know what that is? It could be a whitespace change, a fix to a memory leak, a typo correction, a change to a separator in a message shown to the user, or even a small change to corner case behaviour. If you don't care about the code, but still want to know what the patch is doing, then you can look at the whole commit message, and We should free objects before leaving. explains that perfectly. The short message is what appears in What's Cooking, why should I need to break out of my mail client to find out what it means? -- 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 v4 0/7] git send-email suppress-cc=self fixes
On Mon, Jun 10, 2013 at 09:53:24AM +0300, Michael S. Tsirkin wrote:
I vaguely remember there was some way to say
head of the remote I am tracking - but I could not find it.
Where are all the tricks like foo^{} documented?
gitrevisions(7) is what you're looking for here.
In this case I think you want '@{upstream}' or its short form '@{u}'.
I tried fgrep '{}' Documentation/*txt and it only returned
git-show-ref.txt which isn't really informative ...
'{' and '}' need to be escaped in AsciiDoc so you have to grep for
'\\{\\}'.
--
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: Different diff strategies in add --interactive
On Mon, Jun 10, 2013 at 05:11:41PM -0400, Jeff King wrote:
On Mon, Jun 10, 2013 at 12:28:55PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
I think the first thing to do is read the diff.algorithm setting in
git-add--interactive and pass its value to the underlying diff-index and
diff-files commands, but should we also have a command line parameter to
git-add to specify the diff algorithm in interactive mode? And if so,
can we simply add --diff-algorithm to git-add, or is that too
confusing?
Making git add--interactive read from diff.algorithm is probably a
good idea, because the command itself definitely is a Porcelain. We
would probably need a way to defeat the configured default for
completeness, either:
git add -p --diff-algorithm=default
git -c diff.algorithm=default add -p
but I suspect that a new option to git add that only takes effect
together with -p is probably an overkill, only in order to support
the former and not having to say the latter, but I can be persuaded
either way.
Worse than that, you would need to add such an option to checkout -p,
reset -p, stash -p, etc. I think the latter form you suggest is
probably acceptable in this case.
That's what I'm planning to do at the moment, if anyone wants to extend
it further in the future then that can be built on top.
Overall, I think respecting diff.algorithm in add--interactive is a very
sane thing to do. I would even be tempted to say we should allow a few
other select diff options (e.g., fewer or more context lines). If you
allowed diff options like this:
git add --patch=--patience -U5
that is very flexible, but I would not want to think about what the code
does when you pass --patch=--raw or equal nonsense.
An alternative would be to permit them to be set from within the
interactive UI. I'd find it quite useful to experiment with various
diff options when I encounter a hunk that isn't as easy to pick as I'd
like. I expect it would be very hard to do that on a per-hunk basis,
although per-file doesn't seem like it would be too hard.
I don't intend to investigate that though - respecting diff.algorithm is
good enough for my usage.
But I cannot off the top of my head think of other options besides -U
that would be helpful. I have never particularly wanted it for add -p,
either, though I sometimes generate patches to the list with a greater
number of context lines when I think it makes the changes to a short
function more readable.
--function-context might also be useful, but that's in the same category
as -U.
The patch I'm using is below. I'm not sure how we can test this though;
it seems fragile to invent a patch that appears different with different
diff algorithms. Any suggestions?
-- 8 --
git-add--interactive.perl | 5 +
1 file changed, 5 insertions(+)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d2c4ce6..0b0fac2 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -44,6 +44,8 @@ my ($diff_new_color) =
my $normal_color = $repo-get_color(, reset);
+my $diff_algorithm = ($repo-config('diff.algorithm') or 'default');
+
my $use_readkey = 0;
my $use_termcap = 0;
my %term_escapes;
@@ -731,6 +733,9 @@ sub run_git_apply {
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split( , $patch_mode_flavour{DIFF});
+ if ($diff_algorithm ne default) {
+ push @diff_cmd, --diff-algorithm=${diff_algorithm};
+ }
if (defined $patch_mode_revision) {
push @diff_cmd, $patch_mode_revision;
}
--
1.8.3.779.g691e267
--
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/CommunityGuidelines
On Tue, Jun 11, 2013 at 10:00:56AM -0700, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: * When reviewing other peoples' code, be tactful and constructive. Set high expectations, but do what you can to help the submitter achieve them. Don't demand changes based only on your personal preferences. Don't let the perfect be the enemy of the good. I think this is 30% aimed at me (as I think I do about that much of the reviews around here). I fully agree with most of them, but the last sentence is a bit too fuzzy to be a practically useful guideline. Somebody's bare minimum is somebody else's perfection. An unqualified perfect is the enemy of good is often incorrectly used to justify It works for me. and There already are other codepaths that do it in the same wrong way., both of which make things _worse_ for the long term project health. One thing that I think is missing from these proposals so far is some clear indication that a review should not be confrontational. Consider the following two review comments (taken from a recent example that happened to stick in my mind, but I don't want to single out any one individual here): Ugh, why this roundabout-passive-past tone? Use imperative tone like this: ... vs. We normally use the imperative in commit messages, perhaps like this? ... Both say the same thing but the first immediately puts the submitter on the defensive. If I see something like that on one of my patches I have to consciously resist the urge to reply immediately and instead review what I'm about to send once I've calmed down. I realise that we shouldn't take offence to review comments, but we are all human and it is sometimes hard not to take things personally. In the examples above, the first makes it feel like the submitter is fighting to get a patch included, but the second feels like we're collaborating to get to the best result for the project. As my mother would say, politeness costs nothing ;-) -- 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/CommunityGuidelines
On Tue, Jun 11, 2013 at 08:52:05PM +0200, Michael Haggerty wrote: That's a very good point (and a good illustration, too). How do you like the new second and third sentences below? * When reviewing other peoples' code, be tactful and constructive. Remember that submitting patches for public critique can be very intimidating and when mistakes are found it can be embarrassing. Do what you can to make it a positive and pleasant experience for the submitter. Set high expectations, but do what you can to help the submitter achieve them. Don't demand changes based only on your personal preferences. Don't let the perfect be the enemy of the good. I'm not sure. I like the intent, but I'm not sure that it's clear enough that we're talking about the tone of comments rather than the type of feedback to provide. How about something like this? * Having your code reviewed should feel like a collaboration aiming for the best result for the project, not like a fight to get your patch accepted. Try to bear this in mind when reviewing other peoples' code and consider how you would feel reading the same comments if the review was the other way round. We are only human and the tone of a review can influence how the following discussion progresses. * If you do feel that a review is aggressive, don't reply immediately. Contributors are spread around the world in different timezones and it is often better to wait a few hours for others to comment before rushing to defend your 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] Documentation/CommunityGuidelines
On Wed, Jun 12, 2013 at 12:16:28AM +0530, Ramkumar Ramachandra wrote: John Keeping wrote: Ugh, why this roundabout-passive-past tone? Use imperative tone like this: ... vs. We normally use the imperative in commit messages, perhaps like this? ... As my mother would say, politeness costs nothing ;-) The review is being honest about her feelings in the first one, and being artificially diplomatic in the second one. I don't think it is artificially diplomatic, it's an attempt to convey a helpful tone in an email. As has been said elsewhere, it is easy to read an email in the wrong tone (there is an oft-cited statistic about the percentage of communication that is non-verbal, and which cannot be inferred from written text). For this reason I think it is important for reviewers to make an effort to minimise the risk that what they write can be interpreted as being aggressive. Both of them are constructive and friendly, in that they provide an example for the submitter to follow. Both provide the same advice, yes. But I disagree that they are both friendly. The top example reads (to me at least) as an attack on the submitter for not knowing better. It may sometimes be necessary to resort to strong wording if someone appears to be wilfully ignoring sensible advice but we should not expect every submitter to know the expectations of the project; the first message to someone should gently guide them in the right direction. Either way, I'm not interested in problems that have no solutions. The only solution I see here is to suffocate every contributor until they are tactful enough for the majority's liking, and remove the ones that don't conform. If you do have an alternate solution, please share it with us. I don't have a solution, only a hope that regular contributors will learn from others how they can phrase review comments less aggressively. I expect different people will read the same statement differently; people are from different cultures and what is considered acceptable in one culture can be considered rude in another. We should aim to cultivate our own culture where we try to minimise the risk that what we write will be misinterpreted by someone with a different cultural background. -- 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] add--interactive: respect diff.algorithm
When staging hunks interactively it is sometimes useful to use an
alternative diff algorithm which splits the changes into hunks in a more
logical manner. This is not possible because the plumbing commands
called by add--interactive ignore the diff.algorithm configuration
option (as they should).
Since add--interactive is a porcelain command it should respect this
configuration variable. To do this, make it read diff.algorithm and
pass its value to the underlying diff-index and diff-files invocations.
At this point, do not add options to git add, git reset or git
checkout (all of which can call git-add--interactive). If a user want
to override the value on the command line they can use:
git -c diff.algorithm=$ALGO ...
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Mon, Jun 10, 2013 at 05:56:56PM -0400, Jeff King wrote:
On Mon, Jun 10, 2013 at 10:46:38PM +0100, John Keeping wrote:
Overall, I think respecting diff.algorithm in add--interactive is a very
sane thing to do. I would even be tempted to say we should allow a few
other select diff options (e.g., fewer or more context lines). If you
allowed diff options like this:
git add --patch=--patience -U5
that is very flexible, but I would not want to think about what the code
does when you pass --patch=--raw or equal nonsense.
An alternative would be to permit them to be set from within the
interactive UI. I'd find it quite useful to experiment with various
diff options when I encounter a hunk that isn't as easy to pick as I'd
like. I expect it would be very hard to do that on a per-hunk basis,
although per-file doesn't seem like it would be too hard.
That's an interesting idea, for a subset of options (e.g., increase
context for this hunk). I suspect implementing it would be painful,
though, as you would have to re-run diff, and you have no guarantee of
getting the same set of hunks (e.g., the hunk might end up coalesced
with another).
I think you'd need to re-run the diff over the whole file and then skip
hunks until you reach one that overlaps with the original hunk. But I
suspect it would end up being quite a lot more complicated than that.
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d2c4ce6..0b0fac2 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -44,6 +44,8 @@ my ($diff_new_color) =
my $normal_color = $repo-get_color(, reset);
+my $diff_algorithm = ($repo-config('diff.algorithm') or 'default');
+
my $use_readkey = 0;
my $use_termcap = 0;
my %term_escapes;
@@ -731,6 +733,9 @@ sub run_git_apply {
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split( , $patch_mode_flavour{DIFF});
+ if ($diff_algorithm ne default) {
+ push @diff_cmd, --diff-algorithm=${diff_algorithm};
+ }
if (defined $patch_mode_revision) {
push @diff_cmd, $patch_mode_revision;
Yeah, that looks like the sane way to do it to me. As a perl style
thing, I think the usual way of spelling 'default' is 'undef'. I.e.:
my $diff_algorithm = $repo-config('diff.algorithm');
...
if (defined $diff_algorithm) {
push @diff_cmd, --diff-algorithm=$diff_algorithm;
}
OK. The default is actually the value that is equivalent to 'myers'
for diff.algorithm and I was originally going to add --diff-algorithm
to the command line unconditionally.
But I think it's better to add it only when it has been specified so
I've changed it as you suggest.
git-add--interactive.perl | 5 +
1 file changed, 5 insertions(+)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d2c4ce6..5310959 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -44,6 +44,8 @@ my ($diff_new_color) =
my $normal_color = $repo-get_color(, reset);
+my $diff_algorithm = $repo-config('diff.algorithm');
+
my $use_readkey = 0;
my $use_termcap = 0;
my %term_escapes;
@@ -731,6 +733,9 @@ sub run_git_apply {
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split( , $patch_mode_flavour{DIFF});
+ if (defined $diff_algorithm) {
+ push @diff_cmd, --diff-algorithm=${diff_algorithm};
+ }
if (defined $patch_mode_revision) {
push @diff_cmd, $patch_mode_revision;
}
--
1.8.3.779.g691e267
--
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 stash while pending merge should not be allowed
On Fri, Jun 07, 2013 at 11:47:07AM -0700, Junio C Hamano wrote: Scott McPeak smcp...@coverity.com writes: I suggest that this problem could easily have been avoided if git stash refused to run with a pending merge (present MERGE_HEAD file), since this is crucial repository state that it does not save. This seems similar to what git cherry-pick does. Sounds senslbe. What do we want to see happen in other states, in which Git gives control back to the user asking for help before moving forward (e.g. am, rebase, cherry-pick, revert)? I don't think there's any need to prevent stash running in these cases and I sometimes find it useful that I can stash during a rebase. Having said that, I wonder what happens with cherry-pick -x if you do stash changes while it is stopped. I don't think that is as serious as the merge case because it's easy to detect in the commit message. -- 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] format-patch: remove existing output-directory
On Fri, Jun 14, 2013 at 06:13:33PM +0530, Ramkumar Ramachandra wrote: The following command $ git format-patch -o outgoing master does not ensure that the output-directory outgoing doesn't already exist. As a result, it's possible for patches from two different series to get mixed up if the user is not careful. Fix the problem by unconditionally removing the output-directory before writing to it. I don't think this is the correct behaviour. I can think of cases where I would want to output multiple things into the same directory. It may be better to issue a warning when this happens, or die and provide a flag to let the user bypass that. -- 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] format-patch: remove existing output-directory
On Fri, Jun 14, 2013 at 06:45:19PM +0530, Ramkumar Ramachandra wrote: John Keeping wrote: I don't think this is the correct behaviour. I can think of cases where I would want to output multiple things into the same directory. format.cleanOutputDirectory = true|false? Maybe, but I was thinking of something more like: Output directory is not empty, use --allow-non-empty-dir if you really want to proceed. Using that configuration variable lets someone shoot themselves in the foot quite badly if they forget that they have set it and set the output directory to somewhere containing important data. -- 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 ignore logic does not work as advertised
On Sat, Jun 15, 2013 at 06:18:46PM +0200, Thomas Koch wrote: I'm using vcsh[1] to have my dotfiles in GIT. With that I use a .gitignore file referenced by core.excludesfile that looks like this: # ignore everything by default * # but do not ignore emacs stuff !.emacs.d/ # but than again please ignore backup files inside the .emacs.d folder .emacs.d/backups Now I'd expect git status to show everything in .emacs.d but not to show .emacs.d/backups. However the .emacs.d/backups folder is still shown in git status. I'd say that this is not in line with the man page and might be considered a bug. Which version of Git are you using? You may be hitting a regression that was introduced in Git 1.8.3 and is fixed in Git 1.8.3.1. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/6] t7401: make indentation consistent
Only leading whitespace is changed in this patch. Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7401-submodule-summary.sh | 80 ++-- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 30b429e..c328726 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -76,8 +76,8 @@ head3=$( ) test_expect_success 'modified submodule(backward)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head2...$head3 (2): Add foo3 Add foo2 @@ -89,8 +89,8 @@ EOF head4=$(add_file sm1 foo4 foo5) head4_full=$(GIT_DIR=sm1/.git git rev-parse --verify HEAD) test_expect_success 'modified submodule(backward and forward)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head2...$head4 (4): Add foo5 Add foo4 @@ -102,15 +102,15 @@ EOF test_expect_success '--summary-limit' -git submodule summary -n 3 actual -cat expected -EOF + git submodule summary -n 3 actual + cat expected -EOF * sm1 $head2...$head4 (4): Add foo5 Add foo4 Add foo3 EOF -test_cmp expected actual + test_cmp expected actual commit_file sm1 @@ -122,8 +122,8 @@ rm -f sm1 mv sm1-bak sm1 test_expect_success 'typechanged submodule(submodule-blob), --cached' -git submodule summary --cached actual -cat expected -EOF + git submodule summary --cached actual + cat expected -EOF * sm1 $head4(submodule)-$head5(blob) (3): Add foo5 @@ -132,59 +132,59 @@ EOF test_expect_success 'typechanged submodule(submodule-blob), --files' -git submodule summary --files actual -cat expected -EOF + git submodule summary --files actual + cat expected -EOF * sm1 $head5(blob)-$head4(submodule) (3): Add foo5 EOF -test_i18ncmp actual expected + test_i18ncmp actual expected rm -rf sm1 git checkout-index sm1 test_expect_success 'typechanged submodule(submodule-blob)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head4(submodule)-$head5(blob): EOF -test_i18ncmp actual expected + test_i18ncmp actual expected rm -f sm1 test_create_repo sm1 head6=$(add_file sm1 foo6 foo7) test_expect_success 'nonexistent commit' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head4...$head6: Warn: sm1 doesn't contain commit $head4_full EOF -test_i18ncmp actual expected + test_i18ncmp actual expected commit_file test_expect_success 'typechanged submodule(blob-submodule)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head5(blob)-$head6(submodule) (2): Add foo7 EOF -test_i18ncmp expected actual + test_i18ncmp expected actual commit_file sm1 rm -rf sm1 test_expect_success 'deleted submodule' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head6...000: EOF -test_cmp expected actual + test_cmp expected actual test_create_repo sm2 @@ -192,43 +192,43 @@ head7=$(add_file sm2 foo8 foo9) git add sm2 test_expect_success 'multiple submodules' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head6...000: * sm2 000...$head7 (2): Add foo9 EOF -test_cmp expected actual + test_cmp expected actual test_expect_success 'path filter' -git submodule summary sm2 actual -cat expected -EOF + git submodule summary sm2 actual + cat expected -EOF * sm2 000...$head7 (2): Add foo9 EOF -test_cmp expected actual + test_cmp expected actual commit_file sm2 test_expect_success 'given commit' -git submodule summary HEAD^ actual -cat expected -EOF + git submodule summary HEAD^ actual + cat expected -EOF * sm1 $head6...000: * sm2 000...$head7 (2): Add foo9 EOF -test_cmp expected actual + test_cmp expected actual test_expect_success '--for-status' -git submodule summary --for-status HEAD^ actual -test_i18ncmp actual - EOF + git submodule summary --for-status HEAD^ actual + test_i18ncmp actual - EOF # Submodule changes to be committed: # # * sm1 $head6...000: @@ -240,14 +240,14 @@ EOF test_expect_success 'fail when using --files together with --cached' -test_must_fail git submodule summary --files --cached + test_must_fail git submodule summary --files --cached
[PATCH v4 0/6] submodule: drop the top-level requirement
Changes since v3: * There are four new patches, three of which are style fixes for existing tests and one fixes an existing error message to return a more accurate path when recursing. * You now cannot run git submodule add relative URL from a subdirectory. Because the interpretation of the URL changes depending on whether or not remote.origin.url is configured, I have decided to just ban this for now. If someone comes up with a sensible way to handle this then we can lift this restriction later. * The path variable exported in submodule foreach now uses the relative path and matches the sm_path variable. * I audited the code again and fixed a few more cases that weren't printing relative paths (notably submodule init and submodule foreach). * More tests. John Keeping (6): t7401: make indentation consistent t7403: modernize style t7403: add missing chaining submodule: show full path in error message rev-parse: add --prefix option submodule: drop the top-level requirement Documentation/git-rev-parse.txt | 16 ++ builtin/rev-parse.c | 24 ++- git-submodule.sh| 135 ++ t/t1513-rev-parse-prefix.sh | 96 ++ t/t7400-submodule-basic.sh | 80 + t/t7401-submodule-summary.sh| 116 +++- t/t7403-submodule-sync.sh | 388 ++-- t/t7406-submodule-update.sh | 15 ++ t/t7407-submodule-foreach.sh| 16 ++ 9 files changed, 673 insertions(+), 213 deletions(-) create mode 100755 t/t1513-rev-parse-prefix.sh -- 1.8.3.779.g691e267 -- 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 v4 2/6] t7403: modernize style
Change the indentation to use tabs consistently and start content on the line after the paren opening a subshell. Also don't put a space in file and remove : from : file to be consistent with the majority of tests elsewhere. Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7403-submodule-sync.sh | 316 +++--- 1 file changed, 183 insertions(+), 133 deletions(-) diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 94e26c4..38f6cc4 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -11,216 +11,266 @@ These tests exercise the git submodule sync subcommand. . ./test-lib.sh test_expect_success setup ' - echo file file + echo file file git add file test_tick git commit -m upstream git clone . super git clone super submodule - (cd submodule -git submodule add ../submodule sub-submodule -test_tick -git commit -m sub-submodule + ( + cd submodule + git submodule add ../submodule sub-submodule + test_tick + git commit -m sub-submodule ) - (cd super -git submodule add ../submodule submodule -test_tick -git commit -m submodule + ( + cd super + git submodule add ../submodule submodule + test_tick + git commit -m submodule ) git clone super super-clone - (cd super-clone git submodule update --init --recursive) + ( + cd super-clone + git submodule update --init --recursive + ) git clone super empty-clone - (cd empty-clone git submodule init) + ( + cd empty-clone + git submodule init + ) git clone super top-only-clone git clone super relative-clone - (cd relative-clone git submodule update --init --recursive) + ( + cd relative-clone + git submodule update --init --recursive + ) git clone super recursive-clone - (cd recursive-clone git submodule update --init --recursive) + ( + cd recursive-clone + git submodule update --init --recursive + ) ' test_expect_success 'change submodule' ' - (cd submodule -echo second line file -test_tick -git commit -a -m change submodule + ( + cd submodule + echo second line file + test_tick + git commit -a -m change submodule ) ' test_expect_success 'change submodule url' ' - (cd super -cd submodule -git checkout master -git pull + ( + cd super + cd submodule + git checkout master + git pull ) mv submodule moved-submodule - (cd moved-submodule -git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule -test_tick -git commit -a -m moved-sub-submodule + ( + cd moved-submodule + git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule + test_tick + git commit -a -m moved-sub-submodule ) - (cd super -git config -f .gitmodules submodule.submodule.url ../moved-submodule -test_tick -git commit -a -m moved-submodule + ( + cd super + git config -f .gitmodules submodule.submodule.url ../moved-submodule + test_tick + git commit -a -m moved-submodule ) ' test_expect_success 'git submodule sync should update submodule URLs' ' - (cd super-clone -git pull --no-recurse-submodules -git submodule sync + ( + cd super-clone + git pull --no-recurse-submodules + git submodule sync ) - test -d $(cd super-clone/submodule -git config remote.origin.url + test -d $( + cd super-clone/submodule + git config remote.origin.url ) - test ! -d $(cd super-clone/submodule/sub-submodule -git config remote.origin.url + test ! -d $( + cd super-clone/submodule/sub-submodule + git config remote.origin.url ) - (cd super-clone/submodule -git checkout master -git pull + ( + cd super-clone/submodule + git checkout master + git pull ) - (cd super-clone -test -d $(git config submodule.submodule.url) + ( + cd super-clone + test -d $(git config submodule.submodule.url) ) ' test_expect_success 'git submodule sync --recursive
[PATCH v4 4/6] submodule: show full path in error message
When --recursive was added to submodule foreach in commit 15fc56a (git submodule foreach: Add --recursive to recurse into nested submodules, 2009-08-19), the error message when the script returns a non-zero status was not updated to contain $prefix to show the full path. Fix this. Signed-off-by: John Keeping j...@keeping.me.uk --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 79bfaac..bdb438b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -485,7 +485,7 @@ cmd_foreach() cmd_foreach --recursive $@ fi ) 3 3- || - die $(eval_gettext Stopping at '\$sm_path'; script returned non-zero status.) + die $(eval_gettext Stopping at '\$prefix\$sm_path'; script returned non-zero status.) fi done } -- 1.8.3.779.g691e267 -- 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 v4 6/6] submodule: drop the top-level requirement
Use the new rev-parse --prefix option to process all paths given to the
submodule command, dropping the requirement that it be run from the
top-level of the repository.
Since the interpretation of a relative submodule URL depends on whether
or not remote.origin.url is configured, explicitly block relative URLs
in git submodule add when not at the top level of the working tree.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-submodule.sh | 135 ---
t/t7400-submodule-basic.sh | 80 +
t/t7401-submodule-summary.sh | 36
t/t7403-submodule-sync.sh| 72 +++
t/t7406-submodule-update.sh | 15 +
t/t7407-submodule-foreach.sh | 16 +
6 files changed, 319 insertions(+), 35 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index bdb438b..7756d81 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -14,10 +14,13 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name
name] [--reference re
or: $dashless [--quiet] foreach [--recursive] command
or: $dashless [--quiet] sync [--recursive] [--] [path...]
OPTIONS_SPEC=
+SUBDIRECTORY_OK=Yes
. git-sh-setup
. git-sh-i18n
. git-parse-remote
require_work_tree
+wt_prefix=$(git rev-parse --show-prefix)
+cd_to_toplevel
command=
branch=
@@ -106,12 +109,48 @@ resolve_relative_url ()
echo ${is_relative:+${up_path}}${remoteurl#./}
}
+# Resolve a path to be relative to another path. This is intended for
+# converting submodule paths when git-submodule is run in a subdirectory
+# and only handles paths where the directory separator is '/'.
+#
+# The output is the first argument as a path relative to the second argument,
+# which defaults to $wt_prefix if it is omitted.
+relative_path ()
+{
+ local target curdir result
+ target=$1
+ curdir=${2-$wt_prefix}
+ curdir=${curdir%/}
+ result=
+
+ while test -n $curdir
+ do
+ case $target in
+ $curdir/*)
+ target=${target#$curdir/}
+ break
+ ;;
+ esac
+
+ result=${result}../
+ if test $curdir = ${curdir%/*}
+ then
+ curdir=
+ else
+ curdir=${curdir%/*}
+ fi
+ done
+
+ echo $result$target
+}
+
#
# Get submodule info for registered submodules
# $@ = path to limit submodule list
#
module_list()
{
+ eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)
(
git ls-files --error-unmatch --stage -- $@ ||
echo unmatched pathspec exists
@@ -282,6 +321,7 @@ isnumber()
cmd_add()
{
# parse $args after submodule ... add.
+ reference_path=
while test $# -ne 0
do
case $1 in
@@ -298,11 +338,11 @@ cmd_add()
;;
--reference)
case $2 in '') usage ;; esac
- reference=--reference=$2
+ reference_path=$2
shift
;;
--reference=*)
- reference=$1
+ reference_path=${1#--reference=}
;;
--name)
case $2 in '') usage ;; esac
@@ -323,6 +363,14 @@ cmd_add()
shift
done
+ if test -n $reference_path
+ then
+ is_absolute_path $reference_path ||
+ reference_path=$wt_prefix$reference_path
+
+ reference=--reference=$reference_path
+ fi
+
repo=$1
sm_path=$2
@@ -335,9 +383,14 @@ cmd_add()
usage
fi
+ is_absolute_path $sm_path || sm_path=$wt_prefix$sm_path
+
# assure repo is absolute or relative to parent
case $repo in
./*|../*)
+ test -z $wt_prefix ||
+ die $(gettext Relative path can only be used from the
toplevel of the working tree)
+
# dereference source url relative to parent's url
realrepo=$(resolve_relative_url $repo) || exit
;;
@@ -471,21 +524,23 @@ cmd_foreach()
die_if_unmatched $mode
if test -e $sm_path/.git
then
- say $(eval_gettext Entering '\$prefix\$sm_path')
+ displaypath=$(relative_path $sm_path)
+ say $(eval_gettext Entering '\$prefix\$displaypath')
name=$(module_name $sm_path)
(
prefix=$prefix$sm_path/
clear_local_git_env
- # we make $path available to scripts ...
- path=$sm_path
cd $sm_path
[PATCH v4 3/6] t7403: add missing chaining
Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7403-submodule-sync.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 38f6cc4..bf90098 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -174,7 +174,7 @@ test_expect_success 'git submodule sync handles origin URL of the form foo/bar cd submodule #actual foo/submodule test $(git config remote.origin.url) = ../foo/submodule - ) + ) ( cd submodule/sub-submodule test $(git config remote.origin.url) != ../../foo/submodule @@ -191,7 +191,7 @@ test_expect_success 'git submodule sync --recursive propagates changes in orig cd submodule #actual foo/submodule test $(git config remote.origin.url) = ../foo/submodule - ) + ) ( cd submodule/sub-submodule test $(git config remote.origin.url) = ../../foo/submodule -- 1.8.3.779.g691e267 -- 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 v4 5/6] rev-parse: add --prefix option
This makes 'git rev-parse' behave as if it were invoked from the
specified subdirectory of a repository, with the difference that any
file paths which it prints are prefixed with the full path from the top
of the working tree.
This is useful for shell scripts where we may want to cd to the top of
the working tree but need to handle relative paths given by the user on
the command line.
Signed-off-by: John Keeping j...@keeping.me.uk
---
Documentation/git-rev-parse.txt | 16 +++
builtin/rev-parse.c | 24 ---
t/t1513-rev-parse-prefix.sh | 96 +
3 files changed, 131 insertions(+), 5 deletions(-)
create mode 100755 t/t1513-rev-parse-prefix.sh
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 947d62f..993903c 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -59,6 +59,22 @@ OPTIONS
If there is no parameter given by the user, use `arg`
instead.
+--prefix arg::
+ Behave as if 'git rev-parse' was invoked from the `arg`
+ subdirectory of the working tree. Any relative filenames are
+ resolved as if they are prefixed by `arg` and will be printed
+ in that form.
++
+This can be used to convert arguments to a command run in a subdirectory
+so that they can still be used after moving to the top-level of the
+repository. For example:
++
+
+prefix=$(git rev-parse --show-prefix)
+cd $(git rev-parse --show-toplevel)
+eval set -- $(git rev-parse --sq --prefix $prefix $@)
+
+
--verify::
Verify that exactly one parameter is provided, and that it
can be turned into a raw 20-byte SHA-1 that can be used to
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index f267a1d..de894c7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char
*datestr)
show(buffer);
}
-static int show_file(const char *arg)
+static int show_file(const char *arg, int output_prefix)
{
show_default();
if ((filter (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
- show(arg);
+ if (output_prefix) {
+ const char *prefix = startup_info-prefix;
+ show(prefix_filename(prefix,
+prefix ? strlen(prefix) : 0,
+arg));
+ } else
+ show(arg);
return 1;
}
return 0;
@@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n
int cmd_rev_parse(int argc, const char **argv, const char *prefix)
{
int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
+ int output_prefix = 0;
unsigned char sha1[20];
const char *name = NULL;
@@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
const char *arg = argv[i];
if (as_is) {
- if (show_file(arg) as_is 2)
+ if (show_file(arg, output_prefix) as_is 2)
verify_filename(prefix, arg, 0);
continue;
}
@@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
as_is = 2;
/* Pass on the -- if we show anything but
files.. */
if (filter (DO_FLAGS | DO_REVS))
- show_file(arg);
+ show_file(arg, 0);
continue;
}
if (!strcmp(arg, --default)) {
@@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
i++;
continue;
}
+ if (!strcmp(arg, --prefix)) {
+ prefix = argv[i+1];
+ startup_info-prefix = prefix;
+ output_prefix = 1;
+ i++;
+ continue;
+ }
if (!strcmp(arg, --revs-only)) {
filter = ~DO_NOREV;
continue;
@@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
if (verify)
die_no_single_rev(quiet);
as_is = 1;
- if (!show_file(arg))
+ if (!show_file(arg, output_prefix))
continue;
verify_filename(prefix, arg, 1);
}
diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh
new file mode 100755
index 000..87ec3ae
[PATCH 1/2] Documentation/Makefile: fix spaces around assignments
A simple style fix; no functional change. Signed-off-by: John Keeping j...@keeping.me.uk --- Nothing in maint..pu is touching this at the moment, so hopefully this is a good time to fix the whitespace here. Documentation/Makefile | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 62dbd9a..af3d8a4 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -31,11 +31,11 @@ MAN7_TXT += gittutorial.txt MAN7_TXT += gitworkflows.txt MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT) -MAN_XML=$(patsubst %.txt,%.xml,$(MAN_TXT)) -MAN_HTML=$(patsubst %.txt,%.html,$(MAN_TXT)) +MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT)) +MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT)) OBSOLETE_HTML = git-remote-helpers.html -DOC_HTML=$(MAN_HTML) $(OBSOLETE_HTML) +DOC_HTML = $(MAN_HTML) $(OBSOLETE_HTML) ARTICLES = howto-index ARTICLES += everyday @@ -74,35 +74,35 @@ SP_ARTICLES += technical/api-index DOC_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES)) -DOC_MAN1=$(patsubst %.txt,%.1,$(MAN1_TXT)) -DOC_MAN5=$(patsubst %.txt,%.5,$(MAN5_TXT)) -DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT)) +DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT)) +DOC_MAN5 = $(patsubst %.txt,%.5,$(MAN5_TXT)) +DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT)) -prefix?=$(HOME) -bindir?=$(prefix)/bin -htmldir?=$(prefix)/share/doc/git-doc -pdfdir?=$(prefix)/share/doc/git-doc -mandir?=$(prefix)/share/man -man1dir=$(mandir)/man1 -man5dir=$(mandir)/man5 -man7dir=$(mandir)/man7 -# DESTDIR= +prefix ?= $(HOME) +bindir ?= $(prefix)/bin +htmldir ?= $(prefix)/share/doc/git-doc +pdfdir ?= $(prefix)/share/doc/git-doc +mandir ?= $(prefix)/share/man +man1dir = $(mandir)/man1 +man5dir = $(mandir)/man5 +man7dir = $(mandir)/man7 +# DESTDIR = ASCIIDOC = asciidoc ASCIIDOC_EXTRA = MANPAGE_XSL = manpage-normal.xsl XMLTO = xmlto XMLTO_EXTRA = -INSTALL?=install +INSTALL ?= install RM ?= rm -f MAN_REPO = ../../git-manpages HTML_REPO = ../../git-htmldocs -infodir?=$(prefix)/share/info -MAKEINFO=makeinfo -INSTALL_INFO=install-info -DOCBOOK2X_TEXI=docbook2x-texi -DBLATEX=dblatex +infodir ?= $(prefix)/share/info +MAKEINFO = makeinfo +INSTALL_INFO = install-info +DOCBOOK2X_TEXI = docbook2x-texi +DBLATEX = dblatex ifndef PERL_PATH PERL_PATH = /usr/bin/perl endif -- 1.8.3.779.g691e267 -- 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] Documentation/Makefile: move infodir to be with other '*dir's
Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index af3d8a4..0cfdc36 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT)) prefix ?= $(HOME) bindir ?= $(prefix)/bin htmldir ?= $(prefix)/share/doc/git-doc +infodir ?= $(prefix)/share/info pdfdir ?= $(prefix)/share/doc/git-doc mandir ?= $(prefix)/share/man man1dir = $(mandir)/man1 @@ -98,7 +99,6 @@ RM ?= rm -f MAN_REPO = ../../git-manpages HTML_REPO = ../../git-htmldocs -infodir ?= $(prefix)/share/info MAKEINFO = makeinfo INSTALL_INFO = install-info DOCBOOK2X_TEXI = docbook2x-texi -- 1.8.3.779.g691e267 -- 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] mergetool--lib: refactor {diff,merge}_cmd logic
Instead of needing a wrapper to call the diff/merge command, simply
provide the diff_cmd and merge_cmd functions for user-specified tools in
the same way as we do for built-in tools.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-mergetool--lib.sh | 82 ++-
1 file changed, 35 insertions(+), 47 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index e338be5..6a72106 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -114,6 +114,33 @@ valid_tool () {
test -n $cmd
}
+setup_user_tool () {
+ merge_tool_cmd=$(get_merge_tool_cmd $tool)
+ test -n $merge_tool_cmd || return 1
+
+ diff_cmd () {
+ ( eval $merge_tool_cmd )
+ status=$?
+ return $status
+ }
+
+ merge_cmd () {
+ trust_exit_code=$(git config --bool \
+ mergetool.$1.trustExitCode || echo false)
+ if test $trust_exit_code = false
+ then
+ touch $BACKUP
+ ( eval $merge_tool_cmd )
+ status=$?
+ check_unchanged
+ else
+ ( eval $merge_tool_cmd )
+ status=$?
+ fi
+ return $status
+ }
+}
+
setup_tool () {
tool=$1
@@ -142,15 +169,15 @@ setup_tool () {
if ! test -f $MERGE_TOOLS_DIR/$tool
then
- # Use a special return code for this case since we want to
- # source defaults even when an explicit tool path is
- # configured since the user can use that to override the
- # default path in the scriptlet.
- return 2
+ setup_user_tool
+ return $?
fi
# Load the redefined functions
. $MERGE_TOOLS_DIR/$tool
+ # Now let the user override the default command for the tool. If
+ # they have not done so then this will return 1 which we ignore.
+ setup_user_tool
if merge_mode ! can_merge
then
@@ -187,20 +214,7 @@ run_merge_tool () {
status=0
# Bring tool-specific functions into scope
- setup_tool $1
- exitcode=$?
- case $exitcode in
- 0)
- :
- ;;
- 2)
- # The configured tool is not a built-in tool.
- test -n $merge_tool_path || return 1
- ;;
- *)
- return $exitcode
- ;;
- esac
+ setup_tool $1 || return 1
if merge_mode
then
@@ -213,38 +227,12 @@ run_merge_tool () {
# Run a either a configured or built-in diff tool
run_diff_cmd () {
- merge_tool_cmd=$(get_merge_tool_cmd $1)
- if test -n $merge_tool_cmd
- then
- ( eval $merge_tool_cmd )
- status=$?
- return $status
- else
- diff_cmd $1
- fi
+ diff_cmd $1
}
# Run a either a configured or built-in merge tool
run_merge_cmd () {
- merge_tool_cmd=$(get_merge_tool_cmd $1)
- if test -n $merge_tool_cmd
- then
- trust_exit_code=$(git config --bool \
- mergetool.$1.trustExitCode || echo false)
- if test $trust_exit_code = false
- then
- touch $BACKUP
- ( eval $merge_tool_cmd )
- status=$?
- check_unchanged
- else
- ( eval $merge_tool_cmd )
- status=$?
- fi
- return $status
- else
- merge_cmd $1
- fi
+ merge_cmd $1
}
list_merge_tool_candidates () {
--
1.8.3.779.g691e267
--
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: [Request] Git reset should be able to ignore file permissions
On Tue, Jun 18, 2013 at 03:25:22PM +0200, Alexander Nestorov wrote: Recently I had to write some automation scripts and I found that git reset --hard actually restores each file's permissions. That is causing both the created and the last-modified dates of the file to get changed to the time of the git reset. This behavior is easy to demonstrate: echo test myfile chmod 777 myfile git add myfile git commit -m Test git push chmod 775 myfile git reset --hard origin/master After the git reset --hard command, the entire file was checkout-ed. Instead, git should be able to check if the content of the file changed and only if it did, check it out. Does git reset --keep behave in the same way? I would expect it to leave permissions as they were. -- 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: fast-import bug?
On Fri, Jun 21, 2013 at 02:21:47AM -0700, Dave Abrahams wrote:
The docs for fast-import seem to imply that I can use ls to get the
SHA1 of a commit for which I have a mark:
Reading from a named tree
The dataref can be a mark reference (:idnum) or the full
40-byte
SHA-1 of a Git tag, commit, or tree object, preexisting or waiting
to
be written. The path is relative to the top level of the tree
named by
dataref.
'ls' SP dataref SP path LF
See filemodify above for a detailed description of path.
Output uses the same format as git ls-tree tree -- path:
mode SP ('blob' | 'tree' | 'commit') SP dataref HT path LF
The dataref represents the blob, tree, or commit object at path and
^^
can be used in later cat-blob, filemodify, or ls commands.
but I can't get it to work. It's not entirely clear it's supposed to
work. What path would I pass? Passing an empty path simply causes git
to report missing .
Which version of Git are you using? I just tried this and get the error
fatal: Empty path component found in input, which seems to be from
commit 178e1de (fast-import: don't allow 'ls' of path with empty
components, 2012-03-09), which is included in Git 1.7.9.5.
It seems to be slightly more complicated than that though, because after
allowing empty trees I get the missing message for the root tree.
This seems to be because its mode is 0 and not S_IFDIR.
With the patch below, things are working as I expect but I don't
understand why the mode of the root is not set correctly at this point.
Perhaps someone more familiar with fast-import will have some insight...
-- 8 --
diff --git a/fast-import.c b/fast-import.c
index 23f625f..bcce651 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1626,6 +1626,15 @@ del_entry:
return 1;
}
+static void copy_tree_entry(struct tree_entry *dst, struct tree_entry *src)
+{
+ memcpy(dst, src, sizeof(*dst));
+ if (src-tree is_null_sha1(src-versions[1].sha1))
+ dst-tree = dup_tree_content(src-tree);
+ else
+ dst-tree = NULL;
+}
+
static int tree_content_get(
struct tree_entry *root,
const char *p,
@@ -1651,11 +1660,7 @@ static int tree_content_get(
e = t-entries[i];
if (e-name-str_len == n !strncmp_icase(p,
e-name-str_dat, n)) {
if (!slash1) {
- memcpy(leaf, e, sizeof(*leaf));
- if (e-tree
is_null_sha1(e-versions[1].sha1))
- leaf-tree = dup_tree_content(e-tree);
- else
- leaf-tree = NULL;
+ copy_tree_entry(leaf, e);
return 1;
}
if (!S_ISDIR(e-versions[1].mode))
@@ -3065,7 +3070,11 @@ static void parse_ls(struct branch *b)
die(Garbage after path in: %s, command_buf.buf);
p = uq.buf;
}
- tree_content_get(root, p, leaf);
+ if (!*p) {
+ copy_tree_entry(leaf, root);
+ leaf.versions[1].mode = S_IFDIR;
+ } else
+ tree_content_get(root, p, leaf);
/*
* A directory in preparation would have a sha1 of zero
* until it is saved. Save, for simplicity.
--
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] completion: learn about --man-path
Signed-off-by: John Keeping j...@keeping.me.uk --- contrib/completion/git-completion.bash | 2 ++ t/t9902-completion.sh | 1 + 2 files changed, 3 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8fbf941..c3290af 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2513,11 +2513,13 @@ __git_main () --exec-path --exec-path= --html-path + --man-path --info-path --work-tree= --namespace= --no-replace-objects --help + -c ;; *) __git_compute_porcelain_commands diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 81a1657..14d605a 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -231,6 +231,7 @@ test_expect_success 'double dash git itself' ' --exec-path Z --exec-path= --html-path Z + --man-path Z --info-path Z --work-tree= --namespace= -- 1.8.3.1.676.gaae6535 -- 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] completion: handle unstuck form of base git options
git-completion.bash's parsing of the command name relies on everything
preceding it starting with '-' unless it is the -c option. This
allows users to use the stuck form of --work-tree=path and
--namespace=path but not the unstuck forms --work-tree path and
--namespace path. Fix this.
Similarly, the completion only handles the stuck form --git-dir=path
and not --git-dir path, so fix this as well.
Signed-off-by: John Keeping j...@keeping.me.uk
---
contrib/completion/git-completion.bash | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c3bafe..8fbf941 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2492,9 +2492,10 @@ __git_main ()
i=${words[c]}
case $i in
--git-dir=*) __git_dir=${i#--git-dir=} ;;
+ --git-dir) ((c++)) ; __git_dir=${words[c]} ;;
--bare) __git_dir=. ;;
--help) command=help; break ;;
- -c) c=$((++c)) ;;
+ -c|--work-tree|--namespace) ((c++)) ;;
-*) ;;
*) command=$i; break ;;
esac
--
1.8.3.1.676.gaae6535
--
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] help: introduce man.viewer = eman
On Sat, Jun 22, 2013 at 05:13:29PM +0530, Ramkumar Ramachandra wrote:
Corresponding to woman.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Documentation/git-help.txt | 3 +++
builtin/help.c | 11 ---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index b21e9d7..0cb4c46 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -104,6 +104,9 @@ The 'man.viewer' config variable will be checked if the
'man' format
is chosen. The following values are currently supported:
* man: use the 'man' program as usual,
+* eman: use 'emacsclient' to launch the man mode in emacs
+(this only works starting with emacsclient versions 22), on systems
+with man,
* woman: use 'emacsclient' to launch the woman mode in emacs
(this only works starting with emacsclient versions 22),
* konqueror: use 'kfmclient' to open the man page in a new konqueror
diff --git a/builtin/help.c b/builtin/help.c
index 062957f..7cb44e0 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -120,7 +120,7 @@ static int check_emacsclient_version(void)
return 0;
}
-static void exec_woman_emacs(const char *path, const char *page)
+static void exec_woman_emacs(const char *path, const char *page, int eman)
{
if (!check_emacsclient_version()) {
/* This works only with emacsclient version = 22. */
@@ -128,7 +128,10 @@ static void exec_woman_emacs(const char *path, const
char *page)
if (!path)
path = emacsclient;
- strbuf_addf(man_page, (woman \%s\), page);
+ if (eman)
+ strbuf_addf(man_page, (man \%s\), page);
+ else
+ strbuf_addf(man_page, (woman \%s\), page);
Would it be nicer to pass a string in here instead of a flag? Then this
becomes:
strbuf_addf(man_page, (%s \%s\), command, page);
You should probably also rename this function to exec_emacsclient or
something as well now that it doesn't just launch woman.
execlp(path, emacsclient, -e, man_page.buf, (char *)NULL);
warning(_(failed to exec '%s': %s), path, strerror(errno));
}
@@ -341,8 +344,10 @@ static void exec_viewer(const char *name, const char
*page)
if (!strcasecmp(name, man))
exec_man_man(info, page);
+ else if (!strcasecmp(name, eman))
+ exec_woman_emacs(info, page, 1);
else if (!strcasecmp(name, woman))
- exec_woman_emacs(info, page);
+ exec_woman_emacs(info, page, 0);
else if (!strcasecmp(name, konqueror))
exec_man_konqueror(info, page);
else if (info)
--
1.8.3.1.487.g3e7a5b4.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fast-import bug?
On Sat, Jun 22, 2013 at 07:16:48PM -0700, Dave Abrahams wrote:
on Sat Jun 22 2013, John Keeping john-AT-keeping.me.uk wrote:
On Fri, Jun 21, 2013 at 02:21:47AM -0700, Dave Abrahams wrote:
The docs for fast-import seem to imply that I can use ls to get the
SHA1 of a commit for which I have a mark:
Reading from a named tree
The dataref can be a mark reference (:idnum) or the full
40-byte
SHA-1 of a Git tag, commit, or tree object, preexisting or
waiting to
be written. The path is relative to the top level of the tree
named by
dataref.
'ls' SP dataref SP path LF
See filemodify above for a detailed description of path.
Output uses the same format as git ls-tree tree -- path:
mode SP ('blob' | 'tree' | 'commit') SP dataref HT path LF
The dataref represents the blob, tree, or commit object at path
and
^^
can be used in later cat-blob, filemodify, or ls commands.
but I can't get it to work. It's not entirely clear it's supposed to
work. What path would I pass? Passing an empty path simply causes git
to report missing .
Which version of Git are you using?
,[ git --version ]
| git version 1.8.3.1
`
I just tried this and get the error
fatal: Empty path component found in input,
I get that too.
which seems to be from commit 178e1de (fast-import: don't allow 'ls'
of path with empty components, 2012-03-09), which is included in Git
1.7.9.5.
Yes, that's at least part of the issue. I notice git-fast-import
rejects the root path for other commands, e.g. when used as the
source of a filecopy we get the same issue. I also note that the docs
don't make it clear that quoting the path is mandatory if it might turn
out to be empty.
Interesting. There are two places that can produce this error message,
tree_content_get and tree_content_set, but I wonder if this means that
tree_content_get should not be doing this check. The two places that
call it are:
1) parse_ls as discussed here
2) file_change_cr which deals with file copy and rename.
My patch in the previous message only changes the behaviour for the
parse_ls case, but it seems that you have a valid use case for removing
this check in the file_change_cr case as well.
I also note that the docs
don't make it clear that quoting the path is mandatory if it might turn
out to be empty.
That's not quite the case. It looks to me like quoting the path is
mandatory if no dataref is given, and indeed the documentation says:
Reading from the active commit
This form can only be used in the middle of a commit. The path
names a directory entry within fast-import’s active commit. The
path must be quoted in this case.
'ls' SP path LF
It seems to be slightly more complicated than that though, because after
allowing empty trees I get the missing message for the root tree.
Yeah, I've tried to patch Git to solve this but ran into that problem
and gave up.
This seems to be because its mode is 0 and not S_IFDIR.
Aha.
With the patch below, things are working as I expect
Awesome; works for me, too!
but I don't understand why the mode of the root is not set correctly
at this point. Perhaps someone more familiar with fast-import will
have some insight...
Yeah... there's no bug tracker for Git, right? So if nobody pays
attention to this thread, the problem will persist?
Yes, but I don't see that happening particularly often. In the worst
case issues are normally documented by a failing test case.
In this case, I think I do now understand why the mode is 0: in parse_ls
a new tree object is created and the SHA1 of the original is copied in
but the mode is left blank; clearly this should be set to S_IFDIR when
the SHA1 is non-null.
I think the patch I now have is correct (and addresses the copy from
root scenario), but I need to spend some time understanding t9300 so
that I can add suitable test cases.
-- 8 --
diff --git a/fast-import.c b/fast-import.c
index 23f625f..e2c9d50 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1629,7 +1629,8 @@ del_entry:
static int tree_content_get(
struct tree_entry *root,
const char *p,
- struct tree_entry *leaf)
+ struct tree_entry *leaf,
+ int allow_root)
{
struct tree_content *t;
const char *slash1;
@@ -1641,31 +1642,39 @@ static int tree_content_get(
n = slash1 - p;
else
n = strlen(p);
- if (!n)
+ if (!n !allow_root)
die(Empty path component found in input);
if (!root-tree)
load_tree(root);
+
+ if (!n) {
+ e = root;
+ goto found_entry;
+ }
+
t
Re: fast-import bug?
On Sun, Jun 23, 2013 at 07:19:25AM -0700, Dave Abrahams wrote: on Sun Jun 23 2013, John Keeping john-AT-keeping.me.uk wrote: In this case, I think I do now understand why the mode is 0: in parse_ls a new tree object is created and the SHA1 of the original is copied in but the mode is left blank; clearly this should be set to S_IFDIR when the SHA1 is non-null. I think the patch I now have is correct (and addresses the copy from root scenario), but I need to spend some time understanding t9300 so that I can add suitable test cases. t9300? t/t9300-fast-import.sh in Git's source tree - it's where the tests for fast-import live. Thanks; I'll try this one too. Thanks. I now have a patch series incorporating this which also adds a few tests for handling of empty paths. I'm sending it out in the next few minutes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] fast-import: set valid mode on root tree in ls
This prevents a failure later when we lift the restriction on ls with the empty path. Signed-off-by: John Keeping j...@keeping.me.uk --- fast-import.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fast-import.c b/fast-import.c index 23f625f..bdbadea 100644 --- a/fast-import.c +++ b/fast-import.c @@ -3051,6 +3051,8 @@ static void parse_ls(struct branch *b) struct object_entry *e = parse_treeish_dataref(p); root = new_tree_entry(); hashcpy(root-versions[1].sha1, e-idx.sha1); + if (!is_null_sha1(root-versions[1].sha1)) + root-versions[1].mode = S_IFDIR; load_tree(root); if (*p++ != ' ') die(Missing space after tree-ish: %s, command_buf.buf); -- 1.8.3.1.676.gaae6535 -- 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] fast-import: handle empty paths better
This series addressed Dave Abraham's recent bug report [1] about using fast-import's ls command with an empty path. I also found a couple of other places that do not handle the empty path when it can reasonably be interpreted as meaning the root tree object, which are also fixed here. [1] http://article.gmane.org/gmane.comp.version-control.git/228586 John Keeping (4): t9300: document fast-import empty path issues fast-import: set valid mode on root tree in ls fast-import: allow ls or filecopy of the root tree fast-import: allow moving the root tree fast-import.c | 58 t/t9300-fast-import.sh | 65 ++ 2 files changed, 103 insertions(+), 20 deletions(-) -- 1.8.3.1.676.gaae6535 -- 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] t9300: document fast-import empty path issues
When given an empty path, fast-import sometimes reports missing
instead of using the root tree object. On top of this, for ls and
file copy (but not move) it dies with Empty path component found in
input.
Document this behaviour with failing test cases.
Reported-by: Dave Abrahams d...@boostpro.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
t/t9300-fast-import.sh | 65 ++
1 file changed, 65 insertions(+)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index ac6f3b6..f4b9355 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1031,6 +1031,32 @@ test_expect_success \
git diff-tree -M -r M3^ M3 actual
compare_diff_raw expect actual'
+cat input INPUT_END
+commit refs/heads/M4
+committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
+data COMMIT
+rename root
+COMMIT
+
+from refs/heads/M2^0
+R sub
+
+INPUT_END
+
+cat expect EOF
+:100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1
7123f7f44e39be127c5eb701e5968176ee9d78b1 R100 file2/oldf sub/file2/oldf
+:100755 100755 85df50785d62d3b05ab03d9cbf7e4a0b49449730
85df50785d62d3b05ab03d9cbf7e4a0b49449730 R100 file4 sub/file4
+:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc
f1fb5da718392694d0076d677d6d0e364c79b0bc R100 i/am/new/to/you
sub/i/am/new/to/you
+:100755 100755 e74b7d465e52746be2b4bae983670711e6e66657
e74b7d465e52746be2b4bae983670711e6e66657 R100 newdir/exec.sh
sub/newdir/exec.sh
+:100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791
fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 R100 newdir/interesting
sub/newdir/interesting
+EOF
+test_expect_failure \
+ 'M: rename root to subdirectory' \
+ 'git fast-import input
+git diff-tree -M -r M4^ M4 actual
+cat actual
+compare_diff_raw expect actual'
+
###
### series N
###
@@ -1227,6 +1253,29 @@ test_expect_success \
git diff-tree -C --find-copies-harder -r N4 N6 actual
compare_diff_raw expect actual'
+test_expect_failure \
+ 'N: copy root by path' \
+ 'cat expect -\EOF
+ :100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc
f1fb5da718392694d0076d677d6d0e364c79b0bc C100 file2/newf
oldroot/file2/newf
+ :100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1
7123f7f44e39be127c5eb701e5968176ee9d78b1 C100 file2/oldf
oldroot/file2/oldf
+ :100755 100755 85df50785d62d3b05ab03d9cbf7e4a0b49449730
85df50785d62d3b05ab03d9cbf7e4a0b49449730 C100 file4 oldroot/file4
+ :100755 100755 e74b7d465e52746be2b4bae983670711e6e66657
e74b7d465e52746be2b4bae983670711e6e66657 C100 newdir/exec.sh
oldroot/newdir/exec.sh
+ :100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791
fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 C100 newdir/interesting
oldroot/newdir/interesting
+ EOF
+cat input -INPUT_END
+ commit refs/heads/N-copy-root-path
+ committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
+ data COMMIT
+ copy root directory by (empty) path
+ COMMIT
+
+ from refs/heads/branch^0
+ C oldroot
+ INPUT_END
+git fast-import input
+git diff-tree -C --find-copies-harder -r branch N-copy-root-path
actual
+compare_diff_raw expect actual'
+
test_expect_success \
'N: delete directory by copying' \
'cat expect -\EOF
@@ -2934,4 +2983,20 @@ test_expect_success 'S: ls with garbage after sha1 must
fail' '
test_i18ngrep space after tree-ish err
'
+###
+### series T (ls)
+###
+# Setup is carried over from series S.
+
+test_expect_failure 'T: ls root tree' '
+ sed -e s/Z\$// expect -EOF
+ 04 tree $(git rev-parse S^{tree}) Z
+ EOF
+ sha1=$(git rev-parse --verify S)
+ git fast-import --import-marks=marks -EOF actual
+ ls $sha1
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.8.3.1.676.gaae6535
--
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] fast-import: allow ls or filecopy of the root tree
Commit 178e1de (fast-import: don't allow 'ls' of path with empty
components, 2012-03-09) restricted paths which:
. contain an empty directory component (e.g. foo//bar is invalid),
. end with a directory separator (e.g. foo/ is invalid),
. start with a directory separator (e.g. /foo is invalid).
However, the implementation also caught the empty path, which should
represent the root tree. Relax this restriction so that the empty path
is explicitly allowed and refers to the root tree.
Reported-by: Dave Abrahams d...@boostpro.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
fast-import.c | 35 ++-
t/t9300-fast-import.sh | 4 ++--
2 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index bdbadea..e2c9d50 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1629,7 +1629,8 @@ del_entry:
static int tree_content_get(
struct tree_entry *root,
const char *p,
- struct tree_entry *leaf)
+ struct tree_entry *leaf,
+ int allow_root)
{
struct tree_content *t;
const char *slash1;
@@ -1641,31 +1642,39 @@ static int tree_content_get(
n = slash1 - p;
else
n = strlen(p);
- if (!n)
+ if (!n !allow_root)
die(Empty path component found in input);
if (!root-tree)
load_tree(root);
+
+ if (!n) {
+ e = root;
+ goto found_entry;
+ }
+
t = root-tree;
for (i = 0; i t-entry_count; i++) {
e = t-entries[i];
if (e-name-str_len == n !strncmp_icase(p,
e-name-str_dat, n)) {
- if (!slash1) {
- memcpy(leaf, e, sizeof(*leaf));
- if (e-tree
is_null_sha1(e-versions[1].sha1))
- leaf-tree = dup_tree_content(e-tree);
- else
- leaf-tree = NULL;
- return 1;
- }
+ if (!slash1)
+ goto found_entry;
if (!S_ISDIR(e-versions[1].mode))
return 0;
if (!e-tree)
load_tree(e);
- return tree_content_get(e, slash1 + 1, leaf);
+ return tree_content_get(e, slash1 + 1, leaf, 0);
}
}
return 0;
+
+found_entry:
+ memcpy(leaf, e, sizeof(*leaf));
+ if (e-tree is_null_sha1(e-versions[1].sha1))
+ leaf-tree = dup_tree_content(e-tree);
+ else
+ leaf-tree = NULL;
+ return 1;
}
static int update_branch(struct branch *b)
@@ -2415,7 +2424,7 @@ static void file_change_cr(struct branch *b, int rename)
if (rename)
tree_content_remove(b-branch_tree, s, leaf);
else
- tree_content_get(b-branch_tree, s, leaf);
+ tree_content_get(b-branch_tree, s, leaf, 1);
if (!leaf.versions[1].mode)
die(Path %s not in branch, s);
if (!*d) { /* C path/to/subdir */
@@ -3067,7 +3076,7 @@ static void parse_ls(struct branch *b)
die(Garbage after path in: %s, command_buf.buf);
p = uq.buf;
}
- tree_content_get(root, p, leaf);
+ tree_content_get(root, p, leaf, 1);
/*
* A directory in preparation would have a sha1 of zero
* until it is saved. Save, for simplicity.
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index f4b9355..04385a7 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1253,7 +1253,7 @@ test_expect_success \
git diff-tree -C --find-copies-harder -r N4 N6 actual
compare_diff_raw expect actual'
-test_expect_failure \
+test_expect_success \
'N: copy root by path' \
'cat expect -\EOF
:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc
f1fb5da718392694d0076d677d6d0e364c79b0bc C100 file2/newf
oldroot/file2/newf
@@ -2988,7 +2988,7 @@ test_expect_success 'S: ls with garbage after sha1 must
fail' '
###
# Setup is carried over from series S.
-test_expect_failure 'T: ls root tree' '
+test_expect_success 'T: ls root tree' '
sed -e s/Z\$// expect -EOF
04 tree $(git rev-parse S^{tree}) Z
EOF
--
1.8.3.1.676.gaae6535
--
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] fast-import: allow moving the root tree
Because fast-import.c::tree_content_remove does not check for the empty
path, it is not possible to move the root tree to a subdirectory.
Instead the error Path not in branch is produced (note the double
space where the empty path has been inserted).
Fix this by explicitly checking for the empty path and handling it.
Signed-off-by: John Keeping j...@keeping.me.uk
---
fast-import.c | 21 ++---
t/t9300-fast-import.sh | 2 +-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index e2c9d50..21db3fc 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1568,7 +1568,8 @@ static int tree_content_set(
static int tree_content_remove(
struct tree_entry *root,
const char *p,
- struct tree_entry *backup_leaf)
+ struct tree_entry *backup_leaf,
+ int allow_root)
{
struct tree_content *t;
const char *slash1;
@@ -1583,6 +1584,12 @@ static int tree_content_remove(
if (!root-tree)
load_tree(root);
+
+ if (!*p allow_root) {
+ e = root;
+ goto del_entry;
+ }
+
t = root-tree;
for (i = 0; i t-entry_count; i++) {
e = t-entries[i];
@@ -1599,7 +1606,7 @@ static int tree_content_remove(
goto del_entry;
if (!e-tree)
load_tree(e);
- if (tree_content_remove(e, slash1 + 1, backup_leaf)) {
+ if (tree_content_remove(e, slash1 + 1, backup_leaf, 0))
{
for (n = 0; n e-tree-entry_count; n++) {
if
(e-tree-entries[n]-versions[1].mode) {
hashclr(root-versions[1].sha1);
@@ -2188,7 +2195,7 @@ static uintmax_t do_change_note_fanout(
}
/* Rename fullpath to realpath */
- if (!tree_content_remove(orig_root, fullpath, leaf))
+ if (!tree_content_remove(orig_root, fullpath, leaf, 0))
die(Failed to remove path %s, fullpath);
tree_content_set(orig_root, realpath,
leaf.versions[1].sha1,
@@ -2323,7 +2330,7 @@ static void file_change_m(struct branch *b)
/* Git does not track empty, non-toplevel directories. */
if (S_ISDIR(mode) !memcmp(sha1, EMPTY_TREE_SHA1_BIN, 20) *p) {
- tree_content_remove(b-branch_tree, p, NULL);
+ tree_content_remove(b-branch_tree, p, NULL, 0);
return;
}
@@ -2384,7 +2391,7 @@ static void file_change_d(struct branch *b)
die(Garbage after path in: %s, command_buf.buf);
p = uq.buf;
}
- tree_content_remove(b-branch_tree, p, NULL);
+ tree_content_remove(b-branch_tree, p, NULL, 1);
}
static void file_change_cr(struct branch *b, int rename)
@@ -2422,7 +2429,7 @@ static void file_change_cr(struct branch *b, int rename)
memset(leaf, 0, sizeof(leaf));
if (rename)
- tree_content_remove(b-branch_tree, s, leaf);
+ tree_content_remove(b-branch_tree, s, leaf, 1);
else
tree_content_get(b-branch_tree, s, leaf, 1);
if (!leaf.versions[1].mode)
@@ -2530,7 +2537,7 @@ static void note_change_n(struct branch *b, unsigned char
*old_fanout)
}
construct_path_with_fanout(sha1_to_hex(commit_sha1), *old_fanout, path);
- if (tree_content_remove(b-branch_tree, path, NULL))
+ if (tree_content_remove(b-branch_tree, path, NULL, 0))
b-num_notes--;
if (is_null_sha1(sha1))
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 04385a7..31a770d 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1050,7 +1050,7 @@ cat expect EOF
:100755 100755 e74b7d465e52746be2b4bae983670711e6e66657
e74b7d465e52746be2b4bae983670711e6e66657 R100 newdir/exec.sh
sub/newdir/exec.sh
:100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791
fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 R100 newdir/interesting
sub/newdir/interesting
EOF
-test_expect_failure \
+test_expect_success \
'M: rename root to subdirectory' \
'git fast-import input
git diff-tree -M -r M4^ M4 actual
--
1.8.3.1.676.gaae6535
--
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] add--interactive: respect diff.algorithm
On Sun, Jun 23, 2013 at 12:19:05PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
+my $diff_algorithm = ($repo-config('diff.algorithm') or 'default');
+
my $use_readkey = 0;
my $use_termcap = 0;
my %term_escapes;
@@ -731,6 +733,9 @@ sub run_git_apply {
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split( , $patch_mode_flavour{DIFF});
+if ($diff_algorithm ne default) {
+push @diff_cmd, --diff-algorithm=${diff_algorithm};
+}
This is not exactly sanitary for stash -p, whose DIFF element is
defined like so:
'stash' = {
DIFF = 'diff-index -p HEAD',
and you will end up appending an option after a non-option argument,
It may happen to be accepted by the command line parser which is
overly lax, but we would want to tighten it in the longer term.
As a band-aid, we could do something like the attached patch, but
for the longer term, we might need to rethink the way the tweaking
of the command line is done by $patch_mode_revision.
I originally used splice here then for some reason decided that just
appending the option was okay (I think I thought we were already
appending an option with $patch_mode_revision).
The patch below involves deeper Perl magic than I fully grok, but
wouldn't it be simpler to simply use the fact that the string is
command --options... and use:
splice @diff_cmd 1, 0, --diff-algorithm=${diff_algorithm};
-- 8 --
Subject: add -i: add extra options at the right place in diff command line
Appending --diff-algorithm=histogram at the end of canned command
line for various modes of diff is correct for most of them but not
for stash that has a non-option already wired in, like so:
'stash' = {
DIFF = 'diff-index -p HEAD',
Appending an extra option after non-option may happen to work due to
overly lax command line parser, but that is not something we should
rely on. Instead, splice in the extra argument immediately after a
'-p' option, which is an option to ask for textual diff output that
has to be in all variants.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
git-add--interactive.perl | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 5310959..b50551a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -730,11 +730,23 @@ sub run_git_apply {
return close $fh;
}
+# The command array must have a single -p to ask for output in the
+# patch form. Splice additional options immediately after it; we
+# should not be randomly appending them, as some of the canned command.
+# has non-option argument like HEAD already on it.
+
+sub splice_diff_options {
+ my $diff_cmd = shift;
+ @$diff_cmd = map {
+ ($_ eq '-p') ? ($_, @_) : $_;
+ } @$diff_cmd;
+}
+
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split( , $patch_mode_flavour{DIFF});
if (defined $diff_algorithm) {
- push @diff_cmd, --diff-algorithm=${diff_algorithm};
+ splice_diff_options(\@diff_cmd,
--diff-algorithm=${diff_algorithm});
}
if (defined $patch_mode_revision) {
push @diff_cmd, $patch_mode_revision;
--
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] am: replace uses of --resolved with --continue
On Wed, Jun 26, 2013 at 11:06:41PM +0300, Kevin Bracey wrote:
git am was previously modified to provide --continue for consistency
with rebase, merge etc, and the documentation changed to showing
--continue as the primary form.
Complete the work by replacing remaining uses of --resolved by
--continue, most notably in suggested command reminders.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
Documentation/git-am.txt | 4 ++--
Documentation/user-manual.txt | 2 +-
git-am.sh | 8
t/t7512-status-help.sh| 4 ++--
wt-status.c | 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 5bbe7b6..54d8461 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -132,7 +132,7 @@ default. You can use `--no-utf8` to override this.
--resolvemsg=msg::
When a patch failure occurs, msg will be printed
to the screen before exiting. This overrides the
- standard message informing you to use `--resolved`
+ standard message informing you to use `--continue`
or `--skip` to handle the failure. This is solely
for internal use between 'git rebase' and 'git am'.
@@ -176,7 +176,7 @@ aborts in the middle. You can recover from this in one
of two ways:
. hand resolve the conflict in the working directory, and update
the index file to bring it into a state that the patch should
- have produced. Then run the command with the '--resolved' option.
+ have produced. Then run the command with the '--continue' option.
It isn't new in this patch, but there is an inconsistency in the quoting
of the options here. In the previous hunk we use backticks but here it
uses SQs.
The documentation isn't at all consistent on this, but
backticks seem to be the preferred style (there are some false positives
in both counts but this gives a good indication):
$ git grep '-- -- Documentation/ | wc -l
186
$ git grep '`--' -- Documentation/ | wc -l
487
The command refuses to process new mailboxes until the current
operation is finished, so if you decide to start over from scratch,
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index e831cc2..8218cf9 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1835,7 +1835,7 @@ Once the index is updated with the results of the
conflict
resolution, instead of creating a new commit, just run
-
-$ git am --resolved
+$ git am --continue
-
and Git will create the commit for you and continue applying the
diff --git a/git-am.sh b/git-am.sh
index 9f44509..7ea40fe 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -6,7 +6,7 @@ SUBDIRECTORY_OK=Yes
OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC=\
git am [options] [(mbox|Maildir)...]
-git am [options] (--resolved | --skip | --abort)
+git am [options] (--continue | --skip | --abort)
--
i,interactive run interactively
b,binary* (historical option -- no-op)
@@ -102,7 +102,7 @@ stop_here_user_resolve () {
printf '%s\n' $resolvemsg
stop_here $1
fi
-eval_gettextln When you have resolved this problem, run \\$cmdline
--resolved\.
+eval_gettextln When you have resolved this problem, run \\$cmdline
--continue\.
If you prefer to skip this patch, run \\$cmdline --skip\ instead.
To restore the original branch and stop patching, run \\$cmdline --abort\.
@@ -523,7 +523,7 @@ Use \git am --abort\ to remove it.)
esac
fi
- # Make sure we are not given --skip, --resolved, nor --abort
+ # Make sure we are not given --skip, --continue, nor --abort
test $skip$resolved$abort = ||
die $(gettext Resolve operation not in progress, we are not
resuming.)
@@ -670,7 +670,7 @@ do
# - patch is the patch body.
#
# When we are resuming, these files are either already prepared
- # by the user, or the user can tell us to do so by --resolved flag.
+ # by the user, or the user can tell us to do so by --continue flag.
case $resume in
'')
if test -f $dotest/rebasing
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 4f09bec..bd8aab0 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -510,7 +510,7 @@ test_expect_success 'status in an am session: file
already exists' '
cat expected -\EOF
# On branch am_already_exists
# You are in the middle of an am session.
- # (fix conflicts and then run git am --resolved)
+ # (fix conflicts and then run git am --continue)
# (use git am --skip to skip this patch)
# (use git am --abort to restore the original branch)
#
@@ -532,7 +532,7 @@ test_expect_success 'status in an am session: file
Re: [PATCH] Do not call built-in aliases from scripts
On Thu, Jun 27, 2013 at 11:05:09AM -0700, Junio C Hamano wrote: John Szakmeister j...@szakmeister.net writes: On Thu, Jun 27, 2013 at 1:27 PM, Junio C Hamano gits...@pobox.com wrote: [snip] diff --git a/git-am.sh b/git-am.sh index 9f44509..ad67194 100755 --- a/git-am.sh +++ b/git-am.sh @@ -16,8 +16,8 @@ s,signoff add a Signed-off-by line to the commit message u,utf8 recode into utf8 (default) k,keep pass -k flag to git-mailinfo keep-non-patch pass -b flag to git-mailinfo -keep-cr pass --keep-cr flag to git-mailsplit for mbox format -no-keep-cr do not pass --keep-cr flag to git-mailsplit independent of am.keepcr +keep-cr pass --keep-cr flag to git mailsplit for mbox format +no-keep-cr do not pass --keep-cr flag to git mailsplit independent of am.keepcr c,scissors strip everything before a scissors line whitespace= pass it through git-apply ignore-space-change pass it through git-apply As you were saying yourself, we tell users to prefer the git foo form, so we should also do so in the git am option help, IMHO. What does the above change to the options-help have anything to do with that theme? It does not seem to say anything about git foo vs git-foo? I initially missed it too, but `git-mailsplit` changed to `git mailsplit` in the help. Ahh, OK. That is rendered differently though, I don't think just having the plain text git command is as useful. It should either use the hyphenated form or enclose the text in quotes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] completion: handle unstuck form of base git options
Hi Junio, I don't think you've picked this up. Are you expecting a
re-roll or did it just get lost in the noise?
On Sat, Jun 22, 2013 at 12:25:17PM +0100, John Keeping wrote:
git-completion.bash's parsing of the command name relies on everything
preceding it starting with '-' unless it is the -c option. This
allows users to use the stuck form of --work-tree=path and
--namespace=path but not the unstuck forms --work-tree path and
--namespace path. Fix this.
Similarly, the completion only handles the stuck form --git-dir=path
and not --git-dir path, so fix this as well.
Signed-off-by: John Keeping j...@keeping.me.uk
---
contrib/completion/git-completion.bash | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c3bafe..8fbf941 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2492,9 +2492,10 @@ __git_main ()
i=${words[c]}
case $i in
--git-dir=*) __git_dir=${i#--git-dir=} ;;
+ --git-dir) ((c++)) ; __git_dir=${words[c]} ;;
--bare) __git_dir=. ;;
--help) command=help; break ;;
- -c) c=$((++c)) ;;
+ -c|--work-tree|--namespace) ((c++)) ;;
-*) ;;
*) command=$i; break ;;
esac
--
1.8.3.1.676.gaae6535
--
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] pull: require choice between rebase/merge on non-fast-forward pull
On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote: Because letting a trivial merge automatically handled by Git is so easy with git pull, a person who is new to Git may not realize that the project s/he is interacting with may prefer rebase workflow. Add a safety valve to fail git pull that is not a fast-forward until/unless the user expressed her preference between the two. Those who want the existing behaviour could just do git config --global pull.rebase false once, and they'd not even notice. http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225326 for a full discussion. The fallout from this change to test suite is not very pretty, though. Signed-off-by: Junio C Hamano gits...@pobox.com --- [snip] diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index c56a77d..af02c6d 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -79,7 +79,7 @@ test_expect_success \ test_expect_success \ 'merge-setup part 3' \ -'git pull . branch1' +'git pull --merge . branch1' I think the --merge should be implied here because the suer has specified an explicit remote and branch. Similarly, if --ff, --no-ff or --ff-only are given then we can infer --merge in the absence of any other configuration. However, when I looked at doing this I decided that it would be difficult to get that ideal behaviour without rewriting git-pull as a builtin. -- 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] pull: require choice between rebase/merge on non-fast-forward pull
On Fri, Jun 28, 2013 at 10:22:57AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: test_expect_success \ 'merge-setup part 3' \ -'git pull . branch1' +'git pull --merge . branch1' I think the --merge should be implied here because the suer has specified an explicit remote and branch. The whole point of the topic is It used to be that when you said 'git pull' and did not tell us your preferred way to integrate your work and work by the others', we default to merging, but we no longer do so---you have to choose. Here, git pull . branch1 is merely saying I want to integrate the work on my current branch with that of branch1 without saying how that integration wants to happen. The change that I think is important is that the bring my branch up-to-date operation should force the user to choose what to do if the branch does not fast-forward to its upstream. If that was spelled git update then having git pull perform a merge would be fine, but we spell this operation as git pull so the change needs to happen there. I don't think git pull remote branch falls into the same category as plain git pull so I'm not convinced that defaulting to merge there is unreasonable. The original message about this [1] did talk about only git pull with no arguments. Even though, as an old timer, I find it mildly irritating that we now have to be explicit in these tests, this change is in line with the spirit of the topic. If we didn't have to change this example and the pull silently succeeded without complaining, we achieved nothing. I disagree that we would have achieved nothing. New users will not be using explicit arguments to git-pull when just trying to bring a branch up-to-date. [1] http://article.gmane.org/gmane.comp.version-control.git/225240 -- 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: --follow is ignored when used with --reverse
On Fri, May 24, 2013 at 01:23:24AM +0200, Alois Mahdal wrote: Hello! This [has been reported][1] to this list about half a year ago but with no response so I'm not even sure if it's been acknowledged as bug. [1]: http://marc.info/?l=gitm=135215709307126q=raw When I use `git log --follow file` all is OK, but once I add `--reverse` to it, it no longer follows the file beyond renames. This makes it hard to query for when the file was really added, which I was trying to achieve with $ git -1 --reverse --follow several_times_renamed_file In my testing it actually seems to be worse than that. In git.git: $ git log --oneline builtin/clone.c | wc -l 99 $ git log --oneline --reverse builtin/clone.c | wc -l 99 $ git log --oneline --follow builtin/clone.c | wc -l 125 $ git log --oneline --follow --reverse builtin/clone.c | wc -l 3 So the combination of --reverse and --follow appears to have lost the majority of the commits! -- 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: --follow is ignored when used with --reverse
On Tue, Jul 02, 2013 at 11:51:42AM +0200, Thomas Rast wrote: Lukas Fleischer g...@cryptocrack.de writes: On Tue, Jul 02, 2013 at 10:19:36AM +0100, John Keeping wrote: [...] $ git log --oneline --follow builtin/clone.c | wc -l 125 $ git log --oneline --follow --reverse builtin/clone.c | wc -l 3 I just wanted to point out that it works fine when specifying the *original* file name (which kind of makes sense given that everything is done in reverse order): [...] However, that also doesn't seem to work for builtin/clone.c: $ git log --oneline --follow --reverse -- builtin-clone.c | wc -l 65 I'm pretty sure that is simply because --follow iis a horrible hack, known to be broken in many ways. I have it on my longer-term todo list to unify it with -L -M, which already does the Right Thing (more generally, not in the --reverse interaction, which it never occurred to me I should check). Interesting... this tells me that --reverse doesn't work the way I thought it did (although without any real evidence). Given how --reverse interacts with other options (like --max-count), I assumed it would generate the commit list first and then simply reverse it before display but it seems that this isn't what happens with --follow. I guess that makes sense to avoid calculating the diff twice but I suspect we have to pay that price to get correct output. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Review of git multimail
On Wed, Jul 03, 2013 at 12:53:39AM +0530, Ramkumar Ramachandra wrote:
class CommandError(Exception):
def __init__(self, cmd, retcode):
self.cmd = cmd
self.retcode = retcode
Exception.__init__(
self,
'Command %s failed with retcode %s' % (' '.join(cmd),
retcode,)
So cmd is a list.
class ConfigurationException(Exception):
pass
Dead code?
Huh? ConfigurationException is used elsewhere.
def read_git_output(args, input=None, keepends=False, **kw):
Read the output of a Git command.
return read_output(
['git', '-c', 'i18n.logoutputencoding=%s' % (ENCODING,)] + args,
input=input, keepends=keepends, **kw
)
Okay, although I'm wondering what i18n.logoutputencoding has to do with
anything.
Making sure the output is what's expected and not influenced by
environment variables?
def read_output(cmd, input=None, keepends=False, **kw):
if input:
stdin = subprocess.PIPE
else:
stdin = None
p = subprocess.Popen(
cmd, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
**kw
)
(out, err) = p.communicate(input)
retcode = p.wait()
if retcode:
raise CommandError(cmd, retcode)
if not keepends:
out = out.rstrip('\n\r')
return out
Helper function that serves a single caller, read_git_output().
def read_git_lines(args, keepends=False, **kw):
Return the lines output by Git command.
Return as single lines, with newlines stripped off.
return read_git_output(args, keepends=True, **kw).splitlines(keepends)
Okay.
class Config(object):
def __init__(self, section, git_config=None):
Represent a section of the git configuration.
If git_config is specified, it is passed to git config in
the GIT_CONFIG environment variable, meaning that git config
will read the specified path rather than the Git default
config paths.
self.section = section
if git_config:
self.env = os.environ.copy()
self.env['GIT_CONFIG'] = git_config
else:
self.env = None
Okay.
@staticmethod
def _split(s):
Split NUL-terminated values.
words = s.split('\0')
assert words[-1] == ''
return words[:-1]
Ugh. Two callers of this poorly-defined static method: I wonder if
we'd be better off inlining it.
In what way poorly defined? Personally I'd make it _split_null at the
top level but it seems sensible.
def get(self, name, default=''):
try:
values = self._split(read_git_output(
['config', '--get', '--null', '%s.%s' % (self.section,
name)],
env=self.env, keepends=True,
))
Wait, what is the point of using --null and then splitting by hand
using a poorly-defined static method? Why not drop the --null and
splitlines() as usual?
assert len(values) == 1
When does this assert fail?
In can't, which is presumably why it's an assert - it checks that we're
processing the Git output as expected.
return values[0]
except CommandError:
return default
If you're emulating the dictionary get method, default=None. This is
not C, where all codepaths of the function must return the same type.
def get_bool(self, name, default=None):
try:
value = read_git_output(
['config', '--get', '--bool', '%s.%s' % (self.section,
name)],
env=self.env,
)
except CommandError:
return default
return value == 'true'
Correct. On success, return bool. On failure, return None.
def get_all(self, name, default=None):
Read a (possibly multivalued) setting from the configuration.
Return the result as a list of values, or default if the name
is unset.
try:
return self._split(read_git_output(
['config', '--get-all', '--null', '%s.%s' % (self.section,
name)],
env=self.env, keepends=True,
))
except CommandError, e:
CommandError as e?
Not before Python 2.6.
if e.retcode == 1:
What does this cryptic retcode mean?
It mirrors subprocess.CalledProcessError, retcode is the return code of
the process.
return default
else:
raise
raise what?
The current exception - this is pretty idiomatic Python.
You've instantiated the Config class in two places: user and
multimailhook sections. Considering that you're going to read all the
keys in that section, why not --get-regexp, pre-load the configuration
into a dictionary and refer to that instead of
Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull
On Fri, Jun 28, 2013 at 03:41:34PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Here, git pull . branch1 is merely saying I want to integrate the work on my current branch with that of branch1 without saying how that integration wants to happen. The change that I think is important is that the bring my branch up-to-date operation should force the user to choose what to do if the branch does not fast-forward to its upstream. If that was spelled git update then having git pull perform a merge would be fine, but we spell this operation as git pull so the change needs to happen there. I am not sure I quite get what you want to say with git update, and I am not sure if I necessarily want to know---I do not think we would want to add yet another command that DWIMs for certain _I_, that may not match newbie expectations. I wasn't proposing any new command, I was trying to express the operation that users coming from non-distributed VCSs want to perform (which is called update in svn). The problem is that a DVCS operates in a completely different way and a lot of users do not seem to want to learn the difference but simply try to map the existing commands that they know onto Git commands ([1] is the top result for svn commands to git on Google and maps svn update straight to git pull). [1] http://git.or.cz/course/svn.html I don't think git pull remote branch falls into the same category as plain git pull so I'm not convinced that defaulting to merge there is unreasonable. The original message about this [1] did talk about only git pull with no arguments. If you want to limit the scope to only git pull (without any command line argument), I actually do not have strong preference for or against it either way. Perhaps a follow-up patch to be squashed? I remember looking at this a few weeks ago and being concerned that it's impossible to tell what options you actually have in git-pull because it just invokes 'git fetch $@' and git-pull(1) does advertise a number of fetch options. It may be that test $# = 0 is good enough, but ideally I want to test for non-option arguments. I can't see a way of doing this without putting knowledge of all of the fetch options in git-pull so that we can handle options with arguments correctly. -- 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: Review of git multimail
On Wed, Jul 03, 2013 at 10:02:34AM +0200, Michael Haggerty wrote: On 07/03/2013 12:21 AM, Junio C Hamano wrote: Ramkumar Ramachandra artag...@gmail.com writes: def get(self, name, default=''): try: values = self._split(read_git_output( ['config', '--get', '--null', '%s.%s' % (self.section, name)], env=self.env, keepends=True, )) Wait, what is the point of using --null and then splitting by hand using a poorly-defined static method? Why not drop the --null and splitlines() as usual? You may actually have spotted a bug or misuse of --get here. With this sample configuration: $ cat sample \EOF [a] one = value one = another [b] one = value\nanother EOF A script cannot differentiate between them without using '--null'. $ git config -f sample --get-all a.one $ git config -f sample --get-all b.one But that matters only when you use --get-all, not --get. If this method wants to make sure that the user did not misuse a.one as a multi-valued configuration variable, use of --null --get-all followed by checking how many items the command gives you back would be a way to do so. No, the code in question was a simple sanity check (i.e., mostly a check of my own sanity and understanding of git config behavior) preceding the information-losing next line return values[0]. If it had been meant as a check that the user hadn't misconfigured the system, then I wouldn't have used assert but rather raised a ConfigurationException with an explanatory message. I would be happy to add the checking that you described, but I didn't have the impression that it is the usual convention. Does code that wants a single value from the config usually verify that there is one-and-only-one value, or does it typically just do the equivalent of git config --get and use the returned (effectively the last) value? Doesn't git config --get return an error if there are multiple values? The answer is apparently no - I wrote the text below from git-config(1) and then checked the behaviour. This seems to be a regression in git-config (bisect running now). I think the correct answer is what's below, but it doesn't work like this in current Git: If you want a single value then I think it's normal to just read the output of git config and let it handle the error cases, without needing to split the result at all. I think there is a different issue in the except block following the code quoted at the top though - you will return default if a key happens to be multi-valued. The script should check the return code and raise a ConfigurationException if it is 2. -- 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: Review of git multimail
On Wed, Jul 03, 2013 at 09:29:02AM +0100, John Keeping wrote: Doesn't git config --get return an error if there are multiple values? The answer is apparently no - I wrote the text below from git-config(1) and then checked the behaviour. This seems to be a regression in git-config (bisect running now). Ah, that was an intentional change in commit 00b347d (git-config: do not complain about duplicate entries, 2012-10-23). So the issue is that the documentation was not updated when the behaviour was changed. -- 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] t4205: don't rely on en_US.UTF-8 locale existing
My system doesn't have the en_US.UTF-8 locale (or plain en_US), which
causes t4205 to fail by counting bytes instead of UTF-8 codepoints.
Instead of using sed for this, use Perl which behaves predictably
whatever locale is in use.
Signed-off-by: John Keeping j...@keeping.me.uk
---
This patch is on top of 'as/log-output-encoding-in-user-format'.
t/t4205-log-pretty-formats.sh | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 3cfb744..5864f5b 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -20,9 +20,7 @@ commit_msg () {
# cut string, replace cut part with two dots
# $2 - chars count from the beginning of the string
# $3 - trailing chars
- # LC_ALL is set to make `sed` interpret . as a UTF-8 char not
a byte
- # as it does with C locale
- msg=$(echo $msg | LC_ALL=en_US.UTF-8 sed -e
s/^\(.\{$2\}\)$3/\1../)
+ msg=$(echo $msg | $PERL_PATH -CIO -pe s/^(.{$2})$3/\1../)
fi
echo $msg
}
@@ -205,7 +203,7 @@ test_expect_success 'left alignment formatting with ltrunc'
..sage two
..sage one
add bar Z
-$(commit_msg 0 .\{11\})
+$(commit_msg 0 .{11})
EOF
test_cmp expected actual
@@ -218,7 +216,7 @@ test_expect_success 'left alignment formatting with mtrunc'
mess.. two
mess.. one
add bar Z
-$(commit_msg 4 .\{11\})
+$(commit_msg 4 .{11})
EOF
test_cmp expected actual
--
1.8.3.1.747.g77f7d3a
--
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] t4205: don't rely on en_US.UTF-8 locale existing
On Wed, Jul 03, 2013 at 02:41:06PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
My system doesn't have the en_US.UTF-8 locale (or plain en_US), which
causes t4205 to fail by counting bytes instead of UTF-8 codepoints.
Instead of using sed for this, use Perl which behaves predictably
whatever locale is in use.
Signed-off-by: John Keeping j...@keeping.me.uk
---
This patch is on top of 'as/log-output-encoding-in-user-format'.
Thanks. I think Alexey is going to send incremental updates to the
topic so I won't interfere by applying this patch on top of the
version I have in my tree.
But I do agree that using Perl may be a workable solution.
An alternative might be not to use this cryptic 3-arg form of
commit_msg at all. They are used only for these three:
$(commit_msg 8 ..*$)
$(commit_msg 0 .\{11\})
$(commit_msg 4 .\{11\})
I somehow find them simply not readable, in order to figure out what
is going on.
Just using three variables to hold what are expected would be far
more portable and readable.
# anfänglich whatever it means.
sample_utf8_part=$(printf anf\303\244ng)
commit_msg () {
msg=initial. ${sample_utf8_part}lich;
if test -n $1
then
echo $msg | iconv -f utf-8 -t $1
else
echo $msg
fi
}
And then instead of writing in the expected test output.
$(commit_msg 8 ..*$)
$(commit_msg 0 .\{11\})
$(commit_msg 4 .\{11\})
we can just say
initial...
..an${sample_utf8_part}lich
init..lich
It is no worse than those cryptic 0, 4, 8 and 11 magic numbers we
see in the test, no?
That's probably better since we don't need to rely on some other tool
getting it right.
Alexey, will you incorporate this change in your incremental updates?
--
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 gui replaces amend message when prepare-commit-msg hook is used
On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote: Hi, If a prepare-commit-msg hook is used, git gui executes it for New Commit. If the New Commit is selected, and then immediately Amend (before the hook returns), when the hook returns the message is replaced with the one produced by the hook. I think this is a problem with the hook you are running. The hook is given arguments specifying the message file and optionally the source of whatever is already in the file (see githooks(5) for details). It sounds like your hook is blindly overwriting the file, rather than preserving its contents in the cases where you wish to do so. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git gui replaces amend message when prepare-commit-msg hook is used
On Thu, Jul 04, 2013 at 01:59:10PM +0300, Orgad Shaneh wrote: On Thu, Jul 4, 2013 at 1:34 PM, John Keeping j...@keeping.me.uk wrote: On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote: Hi, If a prepare-commit-msg hook is used, git gui executes it for New Commit. If the New Commit is selected, and then immediately Amend (before the hook returns), when the hook returns the message is replaced with the one produced by the hook. I think this is a problem with the hook you are running. The hook is given arguments specifying the message file and optionally the source of whatever is already in the file (see githooks(5) for details). It sounds like your hook is blindly overwriting the file, rather than preserving its contents in the cases where you wish to do so. Let me try to explain. When git gui is executed, it calls the prepare-commit-msg script with .git/PREPARE_COMMIT_MSG as an argument. When amend is selected, the hook is *not* called at all (what would it prepare? The message is already committed) Use the following hook to reproduce: --- snip --- #!/bin/sh sleep 5 echo $@ /tmp/hook.log echo 'Hello hook' $1 --- snip --- Now run git gui (or press F5 if it is already running), and before 5 seconds pass, click Amend last commit. You'll see the commit's message, but when the 5 seconds pass it is replaced with Hello hook. That's the bug. Yes, and that's a bug in the hook. The hook is called with a second argument commit but it is ignoring this and blindly overwriting the message. githooks(5) says: prepare-commit-msg This hook is invoked by git commit right after preparing the default log message, and before the editor is started. It takes one to three parameters. The first is the name of the file that contains the commit log message. The second is the source of the commit message, and can be: message (if a -m or -F option was given); template (if a -t option was given or the configuration option commit.template is set); merge (if the commit is a merge or a .git/MERGE_MSG file exists); squash (if a .git/SQUASH_MSG file exists); or commit, followed by a commit SHA1 (if a -c, -C or --amend option was given). If the exit status is non-zero, git commit will abort. The purpose of the hook is to edit the message file in place, and it is not suppressed by the --no-verify option. A non-zero exit means a failure of the hook and aborts the commit. It should not be used as replacement for pre-commit hook. Your problem is that your hook script is not checking $2 so it is overwriting the message even when you do not want to do so. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: intend-to-edit flag
On Thu, Jul 04, 2013 at 08:10:07PM +0200, Ævar Arnfjörð Bjarmason wrote: On Thu, Jul 4, 2013 at 7:56 PM, Thomas Koch tho...@koch.ro wrote: we're evaluating Git to be used in our companies Tool. But a hard requirement is the possibility to set an intend-to-edit flag on a file (better path). Notice that I did not use the word lock! :-) One easy implementation might be a special branch XYZ-locks that contains an empty blob for every flagged file. So our tool just needs to check, whether a blob exists for the path that's intended to edit, tries to push a commit that touches the file and only allows editing if the push succeeds. In my experience everyone who thinks this is a hard requirement is wrong. I completely agree with this. Having said that, if you're looking at using Gitolite (which you should if you're hosing your own repositories and not using some other hosting solution), there is a lock command [1]. Note that this cannot stop you committing changes to locked files locally but it does stop you pushing changes to the central repository that touch locked files. [1] http://gitolite.com/gitolite/locking.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
On Fri, Jul 05, 2013 at 03:48:31PM +0530, Ramkumar Ramachandra wrote: Due to a recent change in the Net::SMTP::SSL module, send-email emits the following ugly warning everytime a email is sent via SSL: *** Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client is deprecated! Please set SSL_verify_mode to SSL_VERIFY_PEER together with SSL_ca_file|SSL_ca_path for verification. If you really don't want to verify the certificate and keep the connection open to Man-In-The-Middle attacks please set SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application. *** Fix this by explicitly specifying SSL_verify_mode = SSL_VERIFY_NONE in Net::SMTP::SSL-start_SSL(). I don't think this is really fix, it's more plastering over the problem. As the message from OpenSSL says, specifying this means that we're explicitly saying that we don't want to check the server certificate which loses half of the security of SSL. I'd rather leave this as it is (complete with the big scary error message) and eventually fix it properly by letting the user specify the ca_file or ca_path. Perhaps we can even set a sensible default, although I expect this needs to be platform-specific. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath
On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
brian m. carlson sand...@crustytoothpaste.net writes:
You've covered the STARTTLS case, but not the SSL one right above it.
Someone using smtps on port 465 will still see the warning. You can
pass SSL_verify_mode to Net::SMTP::SSL-new just like you pass it to
start_SSL.
OK, will a fix-up look like this on top of 1/2 and 2/2?
According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
is specified then builtin defaults will be used, so I wonder if we
should pass SSL_VERIFY_PEER regardless (possibly with a switch for
SSL_VERIFY_NONE if people really need that).
[1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm
git-send-email.perl | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 52028ba..3b80340 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1093,6 +1093,25 @@ sub smtp_auth_maybe {
return $auth;
}
+# Helper to come up with SSL/TLS certification validation params
+# and warn when doing no verification
+sub ssl_verify_params {
+ use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
+
+ if (!defined $smtp_ssl_cert_path) {
+ $smtp_ssl_cert_path = /etc/ssl/certs;
+ }
+
+ if (-d $smtp_ssl_cert_path) {
+ return (SSL_verify_mode = SSL_VERIFY_PEER,
+ SSL_ca_path = $smtp_ssl_cert_path);
+ } else {
+ print STDERR warning: Using SSL_VERIFY_NONE. .
+ See sendemail.smtpsslcertpath.\n;
+ return (SSL_verify_mode = SSL_VERIFY_NONE);
+ }
+}
+
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath
On Fri, Jul 05, 2013 at 11:30:11AM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
brian m. carlson sand...@crustytoothpaste.net writes:
You've covered the STARTTLS case, but not the SSL one right above it.
Someone using smtps on port 465 will still see the warning. You can
pass SSL_verify_mode to Net::SMTP::SSL-new just like you pass it to
start_SSL.
OK, will a fix-up look like this on top of 1/2 and 2/2?
According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
is specified then builtin defaults will be used, so I wonder if we
should pass SSL_VERIFY_PEER regardless (possibly with a switch for
SSL_VERIFY_NONE if people really need that).
[1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm
Interesting. That frees us from saying we assume /etc/ssl/cacerts
is the default location, and let the users override it.
To help those I do not want verification because I know my server
does not present valid certificate, I know my server is internal and
trustable, and I do not bother to fix it people, we can let them
specify an empty string (or any non-directory) as the CACertPath,
and structure the code like so?
if (defined $smtp_ssl_cert_path -d $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_PEER,
SSL_ca_path = $smtp_ssl_cert_path);
} elsif (defined $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_NONE);
} else {
return (SSL_verify_mode = SSL_VERIFY_PEER);
}
I'd rather have '$smtp_ssl_cert_path ne ' in the first if condition
(instead of the '-d $smtp_ssl_cert_path') but that seems reasonable and
agrees with my reading of the documentation.
Perhaps a complete solution could allow CA files as well:
if (defined $smtp_ssl_cert_path) {
if ($smtp_ssl_cert_path eq ) {
return (SSL_verify_mode = SSL_VERIFY_NONE);
} elsif (-f $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_PEER,
SSL_ca_file = $smtp_ssl_cert_path);
} else {
return (SSL_verify_mode = SSL_VERIFY_PEER,
SSL_ca_path = $smtp_ssl_cert_path);
}
} else {
return (SSL_verify_mode = SSL_VERIFY_PEER);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath
On Fri, Jul 05, 2013 at 11:25:36PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
I'd rather have '$smtp_ssl_cert_path ne ' in the first if condition
(instead of the '-d $smtp_ssl_cert_path') ...
I agree. The signal for no certs should be an explicit nonsense
value like an empty string, not just a string that does not name an
expected filesystem object. Otherwise people can misspell paths and
disable the validation by accident.
Perhaps a complete solution could allow CA files as well.
Yes, that would be a good idea. Care to roll into a fixup! patch
against [2/2]?
Here's a patch that should do that. However, when testing this I
couldn't get the SSL_verify_mode warning to disappear and
git-send-email kept connecting to my untrusted server - this was using
the SSL code path not the TLS upgrade one.
I think this is caused by the SSL_verify_mode argument not getting all
the way down to the configure_SSL function in IO::Socket::SSL but I
can't see what the code in git-send-email is doing wrong. Can any Perl
experts point out what's going wrong?
Also, I tried Brian's IO::Socket::SSL-import(qw(SSL_VERIFY_PEER
SSL_VERIFY_NONE)); but that produced a warning message about the uses
of SSL_VERIFY_PEER and SSL_VERIFY_NONE following that statement, so I
ended up qualifying each use in the code below.
-- 8 --
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 605f263..b56c215 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -198,6 +198,10 @@ must be used for each option.
--smtp-ssl::
Legacy alias for '--smtp-encryption ssl'.
+--smtp-ssl-verify::
+--no-smtp-ssl-verify::
+ Enable SSL certificate verification. Defaults to on.
+
--smtp-ssl-cert-path::
Path to ca-certificates. Defaults to `/etc/ssl/certs`, or
'sendemail.smtpsslcertpath'.
diff --git a/git-send-email.perl b/git-send-email.perl
index 3b80340..cbe85aa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -69,8 +69,10 @@ git send-email [options] file | directory | rev-list
options
--smtp-pass str * Password for SMTP-AUTH; not necessary.
--smtp-encryption str * tls or ssl; anything else disables.
--smtp-ssl * Deprecated. Use '--smtp-encryption ssl'.
+--[no-]smtp-ssl-verify * Enable SSL certificate verification.
+ Default on.
--smtp-ssl-cert-pathstr * Path to ca-certificates. Defaults to
- /etc/ssl/certs.
+ a platform-specific value.
--smtp-domain str * The domain name sent to HELO/EHLO
handshake
--smtp-debug0|1 * Disable, enable Net::SMTP debug.
@@ -197,7 +199,7 @@ my ($thread, $chain_reply_to, $suppress_from,
$signed_off_by_cc);
my ($to_cmd, $cc_cmd);
my ($smtp_server, $smtp_server_port, @smtp_server_options);
my ($smtp_authuser, $smtp_encryption);
-my ($smtp_ssl_cert_path);
+my ($smtp_ssl_verify, $smtp_ssl_cert_path);
my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
my ($validate, $confirm);
my (@suppress_cc);
@@ -214,6 +216,7 @@ my %config_bool_settings = (
suppressfrom = [\$suppress_from, undef],
signedoffbycc = [\$signed_off_by_cc, undef],
signedoffcc = [\$signed_off_by_cc, undef], # Deprecated
+smtpsslverify = [\$smtp_ssl_verify, 1],
validate = [\$validate, 1],
multiedit = [\$multiedit, undef],
annotate = [\$annotate, undef]
@@ -306,6 +309,8 @@ my $rc = GetOptions(h = \$help,
smtp-pass:s = \$smtp_authpass,
smtp-ssl = sub { $smtp_encryption = 'ssl' },
smtp-encryption=s = \$smtp_encryption,
+ smtp-ssl-cert-path=s = \$smtp_ssl_cert_path,
+ smtp-ssl-verify! = \$smtp_ssl_verify,
smtp-debug:i = \$debug_net_smtp,
smtp-domain:s = \$smtp_domain,
identity=s = \$identity,
@@ -1096,19 +1101,18 @@ sub smtp_auth_maybe {
# Helper to come up with SSL/TLS certification validation params
# and warn when doing no verification
sub ssl_verify_params {
- use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
-
- if (!defined $smtp_ssl_cert_path) {
- $smtp_ssl_cert_path = /etc/ssl/certs;
+ if ($smtp_ssl_verify == 0) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_NONE);
}
- if (-d $smtp_ssl_cert_path) {
- return (SSL_verify_mode = SSL_VERIFY_PEER,
- SSL_ca_path = $smtp_ssl_cert_path);
+ if (! defined $smtp_ssl_cert_path) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_PEER);
+ } elsif (-f $smtp_ssl_cert_path) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_PEER,
+ SSL_ca_file
Re: [PATCH v2 2/2] send-email: introduce sendemail.smtpsslcertpath
On Sat, Jul 06, 2013 at 09:12:31PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
@@ -1096,19 +1101,18 @@ sub smtp_auth_maybe {
# Helper to come up with SSL/TLS certification validation params
# and warn when doing no verification
sub ssl_verify_params {
- use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
-
- if (!defined $smtp_ssl_cert_path) {
- $smtp_ssl_cert_path = /etc/ssl/certs;
+ if ($smtp_ssl_verify == 0) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_NONE);
I do not see any use IO::Socket::SSL anywhere after applying this
patch. Is this expected to work?
I don't get any errors about unknown variables when running it. Do we
get IO::Socket::SSL imported through Net::SMTP::SSL, which extends it?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-config: update doc for --get with multiple values
On Wed, Jul 03, 2013 at 11:47:50AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Since commit 00b347d (git-config: do not complain about duplicate entries, 2012-10-23), git config --get does not exit with an error if there are multiple values for the specified key but instead returns the last value. Update the documentation to reflect this. Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/git-config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 19a7be0..fbad05e 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -82,7 +82,7 @@ OPTIONS --get:: Get the value for a given key (optionally filtered by a regex matching the value). Returns error code 1 if the key was not - found and error code 2 if multiple key values were found. + found and the last value if multiple key values were found. --get-all:: Like get, but does not fail if the number of values for the key Thanks. I wondered if we should explain the significance of last a bit more (like this results in the value from the most specific configuration file to be used, the ones in $GIT_DIR/config overriding what is in $HOME/.gitconfig), but I do not have a strong opinion either way. Let's queue this for 'maint' for now. I don't think that change belongs here. How about doing something like this in the FILES section (the first two hunks are just reordering the existing list, only the last hunk changes the content): -- 8 -- diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index fbad05e..99dc497 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -206,12 +206,8 @@ FILES If not set explicitly with '--file', there are four files where 'git config' will search for configuration options: -$GIT_DIR/config:: - Repository specific configuration file. - -~/.gitconfig:: - User-specific configuration file. Also called global - configuration file. +$(prefix)/etc/gitconfig:: + System-wide configuration file. $XDG_CONFIG_HOME/git/config:: Second user-specific configuration file. If $XDG_CONFIG_HOME is not set @@ -221,8 +217,12 @@ $XDG_CONFIG_HOME/git/config:: you sometimes use older versions of Git, as support for this file was added fairly recently. -$(prefix)/etc/gitconfig:: - System-wide configuration file. +~/.gitconfig:: + User-specific configuration file. Also called global + configuration file. + +$GIT_DIR/config:: + Repository specific configuration file. If no further options are given, all reading options will read all of these files that are available. If the global or the system-wide configuration @@ -230,6 +230,10 @@ file are not available they will be ignored. If the repository configuration file is not available or readable, 'git config' will exit with a non-zero error code. However, in neither case will an error message be issued. +The files are read in the order given above, with last value found taking +precedence over values read earlier. When multiple values are taken then all +values of a key from all files will be used. + All writing options will per default write to the repository specific configuration file. Note that this also affects options like '--replace-all' and '--unset'. *'git config' will only ever change one file at a time*. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Avoid suggestions to merge remote changes
As another aspect of the change to make git-pull die when remote changes do not fast-forward, this series changes the advice messages in git-push to avoid implying that the user wants to merge remote changes. I've chosen the word integrate because it does not carry any special meaning in Git (in terms of being a command) and seems to cover the merge and rebase cases nicely. John Keeping (2): push: avoid suggesting merging remote changes pull: change the description to integrate changes Documentation/git-pull.txt | 2 +- builtin/push.c | 12 ++-- git-pull.sh| 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) -- 1.8.3.2.855.gbc9faed -- 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] push: avoid suggesting merging remote changes
With some workflows, it is more suitable to rebase on top of remote changes when a push does not fast-forward. Change the advice messages in git-push to suggest that a user integrate the remote changes instead of merge the remote changes to make this slightly clearer. Also change the suggested 'git pull' to 'git pull ...' to hint to users that they may want to add other parameters. Suggested-by: Philip Oakley philipoak...@iee.org Signed-off-by: John Keeping j...@keeping.me.uk --- builtin/push.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/push.c b/builtin/push.c index 2d84d10..44e53cd 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -211,8 +211,8 @@ static void setup_default_push_refspecs(struct remote *remote) static const char message_advice_pull_before_push[] = N_(Updates were rejected because the tip of your current branch is behind\n - its remote counterpart. Merge the remote changes (e.g. 'git pull')\n - before pushing again.\n + its remote counterpart. Integrate the remote changes (e.g.\n + 'git pull ...') before pushing again.\n See the 'Note about fast-forwards' in 'git push --help' for details.); static const char message_advice_use_upstream[] = @@ -223,15 +223,15 @@ static const char message_advice_use_upstream[] = static const char message_advice_checkout_pull_push[] = N_(Updates were rejected because a pushed branch tip is behind its remote\n - counterpart. Check out this branch and merge the remote changes\n - (e.g. 'git pull') before pushing again.\n + counterpart. Check out this branch and integrate the remote changes\n + (e.g. 'git pull ...') before pushing again.\n See the 'Note about fast-forwards' in 'git push --help' for details.); static const char message_advice_ref_fetch_first[] = N_(Updates were rejected because the remote contains work that you do\n not have locally. This is usually caused by another repository pushing\n - to the same ref. You may want to first merge the remote changes (e.g.,\n - 'git pull') before pushing again.\n + to the same ref. You may want to first integrate the remote changes\n + (e.g., 'git pull ...') before pushing again.\n See the 'Note about fast-forwards' in 'git push --help' for details.); static const char message_advice_ref_already_exists[] = -- 1.8.3.2.855.gbc9faed -- 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] pull: change the description to integrate changes
Since git-pull learned the --rebase option it has not just been about merging changes from a remote repository (where merge is in the sense of git merge). Change the description to use integrate instead of merge in order to reflect this. Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/git-pull.txt | 2 +- git-pull.sh| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 24ab07a..6ef8d59 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -3,7 +3,7 @@ git-pull(1) NAME -git-pull - Fetch from and merge with another repository or a local branch +git-pull - Fetch from and integrate with another repository or a local branch SYNOPSIS diff --git a/git-pull.sh b/git-pull.sh index 6828e2c..ecf0011 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -5,7 +5,7 @@ # Fetch one or more remote refs and merge it/them into the current HEAD. USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff] [-s strategy]... [fetch-options] repo head...' -LONG_USAGE='Fetch one or more remote refs and merge it/them into the current HEAD.' +LONG_USAGE='Fetch one or more remote refs and integrate it/them into the current HEAD.' SUBDIRECTORY_OK=Yes OPTIONS_SPEC= . git-sh-setup -- 1.8.3.2.855.gbc9faed -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-config(1): clarify precedence of multiple values
In order to clarify which value is used when there are multiple values defined for a key, re-order the list of file locations so that it runs from least specific to most specific. Then add a paragraph which simply says that the last value will be used. Signed-off-by: John Keeping j...@keeping.me.uk --- On Sun, Jul 07, 2013 at 10:31:38AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: I wondered if we should explain the significance of last a bit more (like this results in the value from the most specific configuration file to be used, the ones in $GIT_DIR/config overriding what is in $HOME/.gitconfig), but I do not have a strong opinion either way. Let's queue this for 'maint' for now. I don't think that change belongs here. How about doing something like this in the FILES section (the first two hunks are just reordering the existing list, only the last hunk changes the content): Sounds like a good change to me ;-). So here it is as a proper patch :-) Documentation/git-config.txt | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index fbad05e..99dc497 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -206,12 +206,8 @@ FILES If not set explicitly with '--file', there are four files where 'git config' will search for configuration options: -$GIT_DIR/config:: - Repository specific configuration file. - -~/.gitconfig:: - User-specific configuration file. Also called global - configuration file. +$(prefix)/etc/gitconfig:: + System-wide configuration file. $XDG_CONFIG_HOME/git/config:: Second user-specific configuration file. If $XDG_CONFIG_HOME is not set @@ -221,8 +217,12 @@ $XDG_CONFIG_HOME/git/config:: you sometimes use older versions of Git, as support for this file was added fairly recently. -$(prefix)/etc/gitconfig:: - System-wide configuration file. +~/.gitconfig:: + User-specific configuration file. Also called global + configuration file. + +$GIT_DIR/config:: + Repository specific configuration file. If no further options are given, all reading options will read all of these files that are available. If the global or the system-wide configuration @@ -230,6 +230,10 @@ file are not available they will be ignored. If the repository configuration file is not available or readable, 'git config' will exit with a non-zero error code. However, in neither case will an error message be issued. +The files are read in the order given above, with last value found taking +precedence over values read earlier. When multiple values are taken then all +values of a key from all files will be used. + All writing options will per default write to the repository specific configuration file. Note that this also affects options like '--replace-all' and '--unset'. *'git config' will only ever change one file at a time*. -- 1.8.3.2.855.gbc9faed -- 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: finding $(prefix)/etc/gitconfig when prefix = /usr
On Mon, Jul 08, 2013 at 12:00:02AM +0200, Robin Rosenberg wrote: Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com --- Documentation/git-config.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 9ae2508..3198d52 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -107,7 +107,8 @@ See also FILES. --system:: For writing options: write to system-wide $(prefix)/etc/gitconfig - rather than the repository .git/config. + rather than the repository .git/config. However, $(prefix) is /usr + then /etc/gitconfig is used. That's a build time condition, not something that's decided at runtime so I'm not sure that this logic belongs in the user facing documentation. The technically correct change would be to use $(sysconfdir)/gitconfig but I think that will just confuse users more. Since we have a build step for the documentation, I wonder if it's possible to replace these with the correct directory at build time. + For reading options: read only from system-wide $(prefix)/etc/gitconfig rather than from all available files. @@ -214,7 +215,8 @@ $XDG_CONFIG_HOME/git/config:: file was added fairly recently. $(prefix)/etc/gitconfig:: - System-wide configuration file. + System-wide configuration file, unless $(prefix) is /usr. In the + latter case /etc/gitconfig is used. If no further options are given, all reading options will read all of these files that are available. If the global or the system-wide configuration -- 1.8.3.rc0.19.g7e6a0cc -- 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
