Re: [PATCH] completion: Support exclude prefix, ^rev
Hi,
Quoting Trygve Aaberge trygv...@gmail.com:
When using the exclude pattern, ^rev, the completion did not work.
This enables completion after ^ in the same way that completion after ..
is done.
Interesting, thinking back I can't recall I've ever needed multiple
exclude patterns on the command line, and a single exclude is well
served by the rev1..rev2 notion.
Of course that doesn't mean that nobody might need it, and since it's
easy to implement, I'm for it.
Signed-off-by: Trygve Aaberge trygv...@gmail.com
---
contrib/completion/git-completion.bash | 4
1 file changed, 4 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 8cfee95..3036dac 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -496,6 +496,10 @@ __git_complete_revlist_file ()
cur_=${cur_#*..}
__gitcomp_nl $(__git_refs) $pfx $cur_
;;
+ ^*)
+ cur_=${cur_#^}
+ __gitcomp_nl $(__git_refs) ^ $cur_
+ ;;
This is good, but considering the other cases I suggest making this
the very first case. That way we would not complete e.g. refs after
'^rev..TAB', which would lead to a bad revision error when the
command is executed, or tracked paths after '^rev:TAB', which I
think doesn't make sense, though doesn't lead to error.
*)
__gitcomp_nl $(__git_refs)
;;
--
2.2.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
[ANNOUNCE] git-multimail organizational changes
git-multimail is a server hook that sends email notifications for git pushes. I'd like to announce a few changes in the git-multimail project that will hopefully lessen its dependence on me [1]: * I've created a GitHub organization to house the main repository (previously it was under my own GitHub account). Its new main page is https://github.com/git-multimail/git-multimail The old GitHub pages should redirect here, but anybody who fetches from this repo will have to update his/her git remote configuration. * Matthieu Moy has agreed to become a co-maintainer. He uses git-multimail and has submitted patches in the past, so his participation is very welcome. * If anybody else would like to get involved in maintaining and/or improving git-multimail, please let me and Matthieu know. There are a bunch of pending pull requests that need attention, so feel free to jump in with code review, testing, or whatever. Michael [1] I feel terrible about my neglect of the project. I've been too busy with my day job and this project has suffered. I also don't use git-multimail anymore myself, so I don't experience the itches of its users. -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line 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: [PATCHv2] refs.c: enable large transactions
On 04/21/2015 09:06 PM, Stefan Beller wrote:
This is another attempt on enabling large transactions
(large in terms of open file descriptors). We keep track of how many
lock files are opened by the ref_transaction_commit function.
When more than a reasonable amount of files is open, we close
the file descriptors to make sure the transaction can continue.
Another idea I had during implementing this was to move this file
closing into the lock file API, such that only a certain amount of
lock files can be open at any given point in time and we'd be 'garbage
collecting' open fds when necessary in any relevant call to the lock
file API. This would have brought the advantage of having such
functionality available in other users of the lock file API as well.
The downside however is the over complication, you really need to always
check for (lock-fd != -1) all the time, which may slow down other parts
of the code, which did not ask for such a feature.
Aside from a missing error check (see below), this looks good to me.
Signed-off-by: Stefan Beller sbel...@google.com
---
* Removed unneeded braces in the condition to check if we want to close
the lock file.
* made the counter for the remaining fds an unsigned int. That is what
get_max_fd_limit() returns, so there are no concerns for an overflow.
Also it cannot go below 0 any more.
* moved the initialisation of the remaining_fds a bit down and added a
comment
refs.c| 21 +
t/t1400-update-ref.sh | 4 ++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 4f495bd..34cfcdf 100644
--- a/refs.c
+++ b/refs.c
@@ -3041,6 +3041,8 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
+ if (lock-lk-fd == -1)
+ reopen_lock_file(lock-lk);
You should check that reopen_lock_file() was successful.
if (write_in_full(lock-lk-fd, sha1_to_hex(sha1), 40) != 40 ||
write_in_full(lock-lk-fd, term, 1) != 1 ||
close_ref(lock) 0) {
@@ -3718,6 +3720,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct strbuf *err)
{
int ret = 0, i;
+ unsigned int remaining_fds;
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
@@ -3733,6 +3736,20 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
return 0;
}
+ /*
+ * We need to open many files in a large transaction, so come up with
+ * a reasonable maximum. We still keep some spares for stdin/out and
+ * other open files. Experiments determined we need more fds when
+ * running inside our test suite than directly in the shell. It's
+ * unclear where these fds come from. 32 should be a reasonable large
+ * number though.
+ */
+ remaining_fds = get_max_fd_limit();
+ if (remaining_fds 32)
+ remaining_fds -= 32;
+ else
+ remaining_fds = 0;
+
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
if (ref_update_reject_duplicates(updates, n, err)) {
@@ -3762,6 +3779,10 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ if (remaining_fds 0)
+ remaining_fds--;
+ else
+ close_lock_file(update-lock-lk);
I consider this code a stopgap, and simplicity is more important than
optimization. But just for the sake of discussion, if we planned to keep
this code around, it could be improved by not wasting open file
descriptors for references that are only being verified or deleted, like so:
if (!(flags REF_HAVE_NEW) ||
is_null_sha1(update-new_sha1) ||
remaining_fds == 0)
close_lock_file(update-lock-lk);
else
remaining_fds--;
}
/* Perform updates first so live commits remain referenced */
[...]
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Git does not convert CRLF=LF on files with \r not before \n
Indeed, when changing the gitattributes for '* text', the replacement is OK. Thanks for all the explanations. At first, my use case was some source files (imported from another VCS) with CR in different contexts: - lines ending with CRCRLF - all content in LF or CRLF but some CR that should be EOL... - CR in the middle of the line for no reason! For all this, I will fix the files during import. But when digging I found some shell or awk scripts with CR as a valid char in search/replacement string. I know that the EOL should not be CRLF in this case, but I don't know if this situation could happen in DOS batch files or PowerShell scripts with CRLF EOL. 2015-04-21 21:28 GMT+02:00 Torsten Bögershausen tbo...@web.de: On 2015-04-21 15.51, Alexandre Garnier wrote: Here is a test: git init -q crlf-test cd crlf-test echo '* text=auto' .gitattributes git add .gitattributes git commit -q -m Normalize EOL echo -ne 'some content\r\nother \rcontent with CR\r\ncontent\r\nagain content with\r\r\n' inline-cr.txt echo Working directory content: cat -A inline-cr.txt echo git add inline-cr.txt echo Indexed content: git show :inline-cr.txt | cat -A Result -- File content: some content^M$ other ^Mcontent with CR^M$ content^M$ again content with^M^M$ Indexed content: some content^M$ other ^Mcontent with CR^M$ content^M$ again content with^M^M$ Expected result --- File content: some content^M$ other ^Mcontent with CR^M$ content^M$ again content with^M^M$ Indexed content: some content$ other ^Mcontent with CR$ content$ again content with^M$ # or even 'again content with$' for this last line If you remove the \r that are not at the end of the lines, EOL are converted as expected: File content: some content^M$ other content with CR^M$ content^M$ again content with^M$ Indexed content: some content$ other content with CR$ content$ again content with$ First of all, thanks for the info. The current implementation of Git does an auto-detection if a file is text or binary. For a file which is suspected to be text, it is expected to have either LF or CRLF as line endings, but a bare CR make Git wonder: Should this still be treated as a text file ? If yes, should the CR be kept as is, or should it be converted into LF (or CRLF) ? The current implementation may simply be explained by the fact that nobody has so far asked to treat this file as text, so the implementation assumes it to be binary. (Which makes the code a little bit easier, at the time it was written) So the status of today is that you can force Git to let the CR as is, when you specify that the file is text. Is there a real life problem behind it ? And what should happen to the CRs ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
The plink string detection in GIT_SSH for setting putty to true is very broad. If plink is anywhere in the path to the shell file then putty gets set to true and ssh will fail trying to parse -batch as the hostname. Wouldn’t searching for plink.exe be better?-- To unsubscribe from this list: send the line 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's directory is _prepended_ to PATH when called with an absolute path
On Wed, 22 Apr 2015 08:36:00 +, David Rodríguez wrote: ... * User is relying on a custom path to select their Ruby version. For example, let's say the first folder in path is ~/.rubies/2.2.2/bin. * User runs /usr/bin/git commit and a pre-commit hook is triggered. * The pre-commit hook starts with #!/us/bin/env ruby to select the Ruby to be used in the hook, Yes...but shouldn't the hook itself know which ruby version it needs? After all, if I go into that directory with another ruby setup in my PATH, the hook should still work, and presumably that requires that the hook itself selects its version, and not the user's context. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
fast easy loan from wonga
please open attachment file. 3.5% WONGA EXPRESS LOANS PROMOTION-1.doc Description: MS-Word document
Wrong gitignore precedence?
Hello All, I read the document of gitignore (http://git-scm.com/docs/gitignore), and learned that $GIT_DIR/info/exclude has higher precedence than the file specified by core.excludesfile. But I noticed that patterns in core.excludesfile override patterns in $GIT_DIR/info/exclude. I tested as below: 1. Make a new git repository for test, and move into the repository. $ git init testrepo $ cd testrepo 2. Change core.excludesfile configuration. $ touch ../core_excludesfile $ git config core.excludesfile `realpath ../core_excludesfile` 3. Create test~. In each step I check if the file is ignored or not. $ touch test~ 4. See git status. In this case I expect that test~ should not be ignored. $ git status --ignored 5. Change settings in .git/info/exclude. $ echo '*~' .git/info/exclude 6. See git status. In this case I expect that test~ should be ignored. $ git status --ignored 7. Change settings in .git/info/exclude and core.excludesfile. $ echo '*~' ../core_excludesfile $ echo '!*~' .git/info/exclude 8. See git status. In this case I expect that test~ should not be ignored. $ git status --ignored 9. Change settings in .git/info/exclude and core.excludesfile $ echo '!*~' ../core_excludesfile $ echo '*~' .git/info/exclude 10. See git status. In this case I expect that test~ should be ignored. $ git status --ignored Steps 4. and 6. worked as I expected, but 8. and 10. didn't. I read the source code of Git, and found out the point that seems to cause the problem. In dir.c, setup_standard_excludes() adds patterns in .git/info/exclude to the excludes list first, and patterns in core.excludesfile are added next. In last_exclude_matching_from_list(), pattern is searched from the end of the list, and the first matching pattern is used. Therefore the patterns in core.excludesfile are used in precedence to .git/info/exclude. To meet the precedence described in the document of gitignore, I guess setup_standard_excludes() should be fixed so that patterns in core.excludesfile are added to the list before those in .git/info/ecdlude are added. Thanks -- 遠藤 陽平 (Yohei Endo) yoh...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git's directory is _prepended_ to PATH when called with an absolute path
On Tue, 21 Apr 2015 18:37:29 +, David Rodríguez wrote: ... This causes issues with Ruby git hooks, because Ruby version managers rely on custom settings in PATH to select the Ruby executable, Even if git wouldn't modify PATH this is still a broken way to do that. What ruby to execute a hook with is a property of the hook, not of the user context invoking it. Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line 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 V3 0/2] git-p4: improve client path detection when branches are used
Vitor Antunes vitor@gmail.com writes: The updates introduced in the third revision of these two patches consist only on updates to the commit messages to better clarify what they implement. Vitor Antunes (2): t9801: check git-p4's branch detection with client spec enabled git-p4: improve client path detection when branches are used git-p4.py| 13 -- t/t9801-git-p4-branch.sh | 106 ++ 2 files changed, 115 insertions(+), 4 deletions(-) Thanks; will re-queue. Luke, could you comment? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Git does not convert CRLF=LF on files with \r not before \n
Alexandre Garnier zig...@gmail.com writes: Indeed, when changing the gitattributes for '* text', the replacement is OK. OK. Earlier I said: But it would be a bug if the same thing happens when the user explicitly tells us that the file has CRLF line endings, and I suspect we have that bug, which may want to be corrected. but you are saying that my suspicion is incorrect and we do not have such a bug. Thanks for digging further. -- To unsubscribe from this list: send the line 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: PATH modifications for git-hook processes
Jeff King p...@peff.net writes: IOW, you can do things like alias git=/opt/my-git/git and all the git commands will automatically work fine, even if you didn't know at compile time where you would install them, and you didn't set GIT_EXEC_DIR at run-time. It will still first look in GIT_EXEC_DIR, but if that fails, it will take the git commands from /opt/my-git/ instead of from /usr/bin or whatever. If we can get away with just dropping this element from the PATH, I'd much rather do that than try to implement a complicated path-precedence scheme. I am OK with dropping it at a major version boundary with deprecation notice in the release note. Unlike older days, by now, Git has become so essential to users' everyday life, and there is not much reason for people to keep the installation of Git they built outside their $PATH, and alias git=/opt/git/bin/git has lost much of its value, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git's directory is _prepended_ to PATH when called with an absolute path
On 22/04/15 14:02, brian m. carlson wrote: I want whatever ruby the user chooses. This is exactly what I want. The problem is that git overrides the user's choice by prepending /usr/bin to the path and thus making /usr/bin/env choose system's ruby version. Which is almost always not the Ruby the user chose. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
Hi Patrick, On 2015-04-22 16:36, Patrick Sharp wrote: The plink string detection in GIT_SSH for setting putty to true is very broad. Wow. You probably wanted to state that you are using Windows, downloaded Git from [link here], that you are using [version] and that you use PLink [version] (from the Putty package downloaded [link here]) to do your ssh business. Without that information, you leave readers who have no idea about Putty *quite* puzzled. If plink is anywhere in the path to the shell file then putty gets set to true and ssh will fail trying to parse -batch as the hostname. This is cryptic even for me. Wouldn’t searching for plink.exe be better?-- I invite you to try your hand at improving anything you find flawed. For example, if you want to improve the PLink detection in Git for Windows 1.x, this would be the correct place to start: https://github.com/msysgit/msysgit/blob/70f24b4b0f5f86a5e85f7264a4ee2c0fec2d4391/share/WinGit/install.iss#L232-L253 (yes, you would have to download the development environment from https://msysgit.github.com/#download-msysgit and rebuild your own installer using `/share/msysGit/WinGit/release.sh 1.9.5-patrick` after editing the installer script). Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git's directory is _prepended_ to PATH when called with an absolute path
On Wed, Apr 22, 2015 at 05:31:09PM +0200, Andreas Krey wrote: On Wed, 22 Apr 2015 08:36:00 +, David Rodríguez wrote: ... * User is relying on a custom path to select their Ruby version. For example, let's say the first folder in path is ~/.rubies/2.2.2/bin. * User runs /usr/bin/git commit and a pre-commit hook is triggered. * The pre-commit hook starts with #!/us/bin/env ruby to select the Ruby to be used in the hook, Yes...but shouldn't the hook itself know which ruby version it needs? After all, if I go into that directory with another ruby setup in my PATH, the hook should still work, and presumably that requires that the hook itself selects its version, and not the user's context. More generally, #!/usr/bin/env ruby is saying, I want whatever ruby the user chooses. If you want a ruby that has certain gems, or certain features (e.g. Array#to_h), then that's not what you want. #!/usr/bin/env ruby is basically only safe if you're willing to accept any potential version that might show up. You can use a multiline shebang[0] if you need to execute shell code to make the decision at runtime, such as if you need to use $HOME in the path to your ruby, or select from multiple different options. [0] http://rosettacode.org/wiki/Multiline_shebang -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH] builtin/merge.c: add the merge.verifysignatures config option
Bruno Vieira m...@bmpvieira.com writes:
This space before your Signed-off-by: line is a place to justify why
this is a good idea.
Signed-off-by: Bruno Vieira m...@bmpvieira.com
---
This seemed to be missing. Sorry if otherwise or if I'm doing something wrong
(first time contributing).
builtin/merge.c | 3 +++
1 file changed, 3 insertions(+)
Missing are documentation updates and tests. Tests musth at least
cover these cases, I think:
- having configuration set to true without --verify-signatures on
the command line triggers the check.
- having configuration set to false without the command line option
does not trigger the check.
- having configuration set to true with --no-verify-signatures on
the command line does not trigger the check.
- having configuration set to false with --verify-signatures on the
command line triggers the check.
Thanks.
diff --git a/builtin/merge.c b/builtin/merge.c
index 3b0f8f9..5dbc10f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -598,6 +598,9 @@ static int git_merge_config(const char *k, const char *v,
void *cb)
} else if (!strcmp(k, merge.defaulttoupstream)) {
default_to_upstream = git_config_bool(k, v);
return 0;
+ } else if (!strcmp(k, merge.verifysignatures)) {
+ verify_signatures = git_config_bool(k, v);
+ return 0;
} else if (!strcmp(k, commit.gpgsign)) {
sign_commit = git_config_bool(k, v) ? : NULL;
return 0;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How do I resolve conflict after popping stash without adding the file to index?
Jeff King p...@peff.net writes: Right, I am suggesting that latter: that stash should abort if the index has modified entries. The abort for modified working tree files is done by git-merge, which can be selective about which entries will be changed (since it knows which ones need written). I haven't thought hard enough to say whether it should be doing the same for the index (i.e., whether this is a merge problem or a stash problem). This is a stash problem. I've always thought that it insisted on having a clean index and a clean working tree, but apparently it doesn't, as shown in Dmitry's example sequence. Generally speaking, any mergy operation should be done with a clean index (i.e. matches HEAD) so that any difference between the index and the HEAD after it stops in the middle is purely the result of that mergy operation, and the commands should stop when the index is not clean as a safety measure (e.g. git am -3, git merge) [*1*]. An especially tricky command may also insist on a clean working tree if it is not easy to guarantee that it will not clobber changes by the user when it stops in the middle (e.g. git rebase). But this is an escape hatch for lazy implementations ;-) It would be ideal if a command stops without doing anything when the set of paths it needs to touch would overlap the set of paths in which user has changes before the command is run (e.g. git merge works that way). I think that stash apply requires a clean working tree for the latter reason, and does not require a clean index because it just forgot that it must 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: [PATCH 7/9] strbuf_getwholeline: use getdelim if it is available
Hi, On 2015-04-17 12:16, Eric Sunshine wrote: On Thu, Apr 16, 2015 at 5:01 AM, Jeff King p...@peff.net wrote: We spend a lot of time in strbuf_getwholeline in a tight loop reading characters from a stdio handle into a buffer. The libc getdelim() function can do this for us with less overhead. Just for the record: Git for Windows cannot lean on `getdelim()`, as it is not available on Windows. Do not let that stop you; if it turns out to impact performance, we will just have to come up with our own implementation of that function. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/9] strbuf_getwholeline: use getdelim if it is available
On Wed, Apr 22, 2015 at 08:00:55PM +0200, Johannes Schindelin wrote: On 2015-04-17 12:16, Eric Sunshine wrote: On Thu, Apr 16, 2015 at 5:01 AM, Jeff King p...@peff.net wrote: We spend a lot of time in strbuf_getwholeline in a tight loop reading characters from a stdio handle into a buffer. The libc getdelim() function can do this for us with less overhead. Just for the record: Git for Windows cannot lean on `getdelim()`, as it is not available on Windows. Do not let that stop you; if it turns out to impact performance, we will just have to come up with our own implementation of that function. Hopefully the earlier patch in the series to avoid locking will help on Windows. After the end of the series, it isn't used anymore on Linux, but I kept it in exactly for those less-fortunate systems. If you can find a Windows equivalent that does the same thing as getdelim, it should be pretty easy to drop it into an alternate strbuf_getwholeline implementation (or just provide a compat getdelim if it is close enough to have the same interface). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] stop putting argv[0] dirname at front of PATH
On Wed, Apr 22, 2015 at 2:14 PM, Jeff King p...@peff.net wrote: Subject: stop putting argv[0] dirname at front of PATH When the git wrapper is invoked, we prepend the baked-in exec-path to our PATH, so that any sub-processes we exec will all find the git-foo commands that match the wrapper version. [...] Given that the main motivation for git pulling the argv[0] s/pulling/putting/ dirname into the PATH has been broken for years, that the remaining cases are obscure and unlikely (and easily fixed by the user just setting up their $PATH sanely), and that the behavior is hurting real, reasonably common use cases, it's not worth continuing to do so. Signed-off-by: Jeff King p...@peff.net --- diff --git a/exec_cmd.c b/exec_cmd.c index 8ab37b5..e85f0fd 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -96,7 +96,6 @@ void setup_path(void) struct strbuf new_path = STRBUF_INIT; add_path(new_path, git_exec_path()); - add_path(new_path, argv0_path); if (old_path) strbuf_addstr(new_path, old_path); -- 2.4.0.rc2.498.g02440db -- To unsubscribe from this list: send the line 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] stop putting argv[0] dirname at front of PATH
On Wed, Apr 22, 2015 at 10:46:57AM -0700, Junio C Hamano wrote: If we can get away with just dropping this element from the PATH, I'd much rather do that than try to implement a complicated path-precedence scheme. I am OK with dropping it at a major version boundary with deprecation notice in the release note. Unlike older days, by now, Git has become so essential to users' everyday life, and there is not much reason for people to keep the installation of Git they built outside their $PATH, and alias git=/opt/git/bin/git has lost much of its value, I would think. Actually, on looking further into it, I am not sure that the use-case originally presented hasn't simply been broken since 2007. The patch below is trivial, but I've tried to summarize the situation in the commit message. If we are really breaking workflows, I agree on the deprecation. But I think we may not be, in which case it would be fine (IMHO) to skip the deprecation notice. -- 8 -- Subject: stop putting argv[0] dirname at front of PATH When the git wrapper is invoked, we prepend the baked-in exec-path to our PATH, so that any sub-processes we exec will all find the git-foo commands that match the wrapper version. If you invoke git with an absolute path, like: /usr/bin/git foo we also prepend /usr/bin to the PATH. This was added long ago by by 231af83 (Teach the git command to handle some commands internally, 2006-02-26), with the intent that things would just work if you did something like: cd /opt tar xzf premade-git-package.tar.gz alias git=/opt/git/bin/git as we would then find all of the related external commands in /opt/git/bin. I.e., it made git runtime-relocatable, since at the time of 231af83, we installed all of the git commands into $(bindir). But these days, that is not enough. Since f28ac70 (Move all dashed-form commands to libexecdir, 2007-11-28), we do not put commands into $(bindir), and you actually need to convert /usr/bin into /usr/libexec. And not just for finding binaries; we want to find $(sharedir), etc, the same way. The RUNTIME_PREFIX build knob does this the right way, by assuming a sane hierarchy rooted at $prefix when we run $prefix/bin/git, and inferring $prefix/libexec/git-core, etc. So this feature (prepending the argv[0] dirname to the PATH) is broken for providing a runtime prefix, and has been for many years. Does it do anything for other cases? For the git wrapper itself, as well as any commands shipped by git, the answer is no. Those are already in git's exec-path, which is consulted first. For third-party commands which you've dropped into the same directory, it does include them. So if you do cd /opt tar xzf git-built-specifically-for-opt-git.tar.gz cp third-party/git-foo /opt/git/bin/git-foo alias git=/opt/git/bin/git it does mean that we will find the third-party git-foo, even if you do not put /opt/git/bin into your $PATH. But the flipside of this is that we will bump the precedence of _other_ third-party tools that happen to be in the same directory as git. For example, consider this setup: 1. Git is installed by the system in /usr/bin. There are other system utilities in /usr/bin. E.g., a system vi. 2. The user installs tools they prefer in /usr/local/bin. E.g., vim with a vi symlink. They set their PATH to /usr/local/bin:/usr/bin to prefer their custom tools. 3. Running /usr/bin/git puts /usr/bin at the front of their PATH. When git invokes the editor on behalf of the user, they get the system vi, not their normal vim. There are other variants of this, including overriding system ruby and python (which is quite common using tools like rvm and virtualenv, which use relocatable hierarchies and $PATH settings to get a consistent environment). Given that the main motivation for git pulling the argv[0] dirname into the PATH has been broken for years, that the remaining cases are obscure and unlikely (and easily fixed by the user just setting up their $PATH sanely), and that the behavior is hurting real, reasonably common use cases, it's not worth continuing to do so. Signed-off-by: Jeff King p...@peff.net --- If people _are_ interested in relocatable binary packages, I think RUNTIME_PREFIX is the right way forward. But note that you can't just flip on RUNTIME_PREFIX on non-Windows systems, as some invocations will get the full path to the executable, and others see just git. You'd need to convert that into an absolute path (either by searching the $PATH, or doing something system-specific like looking in /proc/$$/exe). exec_cmd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/exec_cmd.c b/exec_cmd.c index 8ab37b5..e85f0fd 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -96,7 +96,6 @@ void setup_path(void) struct strbuf new_path = STRBUF_INIT; add_path(new_path, git_exec_path()); - add_path(new_path, argv0_path); if (old_path) strbuf_addstr(new_path, old_path); --
Re: How do I resolve conflict after popping stash without adding the file to index?
On Wed, Apr 22, 2015 at 10:41:04AM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Right, I am suggesting that latter: that stash should abort if the index
has modified entries. The abort for modified working tree files is done
by git-merge, which can be selective about which entries will be changed
(since it knows which ones need written). I haven't thought hard enough
to say whether it should be doing the same for the index (i.e., whether
this is a merge problem or a stash problem).
This is a stash problem. I've always thought that it insisted on
having a clean index and a clean working tree, but apparently it
doesn't, as shown in Dmitry's example sequence.
It did check the working tree manually, until my e0e2a9c (stash: drop
dirty worktree check on apply, 2011-04-05). But I don't think we ever
checked that the index was clean with respect to HEAD.
Generally speaking, any mergy operation should be done with a clean
index (i.e. matches HEAD) so that any difference between the index
and the HEAD after it stops in the middle is purely the result of
that mergy operation, and the commands should stop when the index is
not clean as a safety measure (e.g. git am -3, git merge) [*1*].
I guess this was the heart of my question. Should a mergy operation
start with a clean index (which the caller can enforce), or does it only
need the index entries that it is going to touch to be clean (which is
known only to the merging code)? The latter is more permissive, as we
know that we will not create conflict entries that overwrite what is
staged in the index. But I don't think we have good tool support for
operating on only those entries afterwards (e.g., git reset --keep
would hose your staged changes along with undoing the parts modified by
the merge). So probably asking for a completely clean index is the only
sane thing.
An especially tricky command may also insist on a clean working tree
if it is not easy to guarantee that it will not clobber changes by
the user when it stops in the middle (e.g. git rebase). But this
is an escape hatch for lazy implementations ;-) It would be ideal if
a command stops without doing anything when the set of paths it
needs to touch would overlap the set of paths in which user has
changes before the command is run (e.g. git merge works that way).
Right, and I think we do that appropriately in stash since e0e2a9c.
I think that stash apply requires a clean working tree for the
latter reason, and does not require a clean index because it just
forgot that it must do so.
Ironically, the message before e0e2a9c actually recommends staging
changes before applying the stash, which would lead to this exact
situation! So I think the most trivial patch is:
diff --git a/git-stash.sh b/git-stash.sh
index d4cf818..f1865c9 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -442,6 +442,7 @@ apply_stash () {
assert_stash_like $@
git update-index -q --refresh || die $(gettext unable to refresh
index)
+ git diff-index --cached HEAD || die dirty index; cannot apply stash
# current index state
c_tree=$(git write-tree) ||
but it makes me wonder if somebody would find it annoying that they
cannot apply a stash into their work-in-progress (i.e., it _might_ cause
annoyance, but most of the time it will be convenient to do so).
We also have require_clean_work_tree() in git-sh-setup.sh. We definitely
don't want the first half of that, but we do want the diff-index check.
So probably we'd want to refactor that into two separate functions, and
only call the require_clean_index part.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] refs.c: enable large transactions
On Wed, Apr 22, 2015 at 7:11 AM, Michael Haggerty mhag...@alum.mit.edu wrote: + if (lock-lk-fd == -1) + reopen_lock_file(lock-lk); You should check that reopen_lock_file() was successful. ok @@ -3762,6 +3779,10 @@ int ref_transaction_commit(struct ref_transaction *transaction, update-refname); goto cleanup; } + if (remaining_fds 0) + remaining_fds--; + else + close_lock_file(update-lock-lk); I consider this code a stopgap, and simplicity is more important than optimization. Can you explain a bit why you think this is a stopgap? Looking at the patch this looks simple to me, as there are no huge pain points involved. (Compared to [1] as we'd change a lot in that series) Also this is pretty good on performance. The small cases do not have an additional unneeded close and reopen, but only the larger cases do. [1] http://git.661346.n2.nabble.com/PATCHv1-0-6-Fix-bug-in-large-transactions-tt7624363.html#a7624368 But just for the sake of discussion, if we planned to keep this code around, it could be improved by not wasting open file descriptors for references that are only being verified or deleted, like so: I'll pick that up for the resend. -- To unsubscribe from this list: send the line 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] stop putting argv[0] dirname at front of PATH
On Wed, Apr 22, 2015 at 02:23:27PM -0400, Eric Sunshine wrote: On Wed, Apr 22, 2015 at 2:14 PM, Jeff King p...@peff.net wrote: Subject: stop putting argv[0] dirname at front of PATH When the git wrapper is invoked, we prepend the baked-in exec-path to our PATH, so that any sub-processes we exec will all find the git-foo commands that match the wrapper version. [...] Given that the main motivation for git pulling the argv[0] s/pulling/putting/ Heh. Too much git for me, I think. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wrong gitignore precedence?
Yohei Endo yoh...@gmail.com writes: I read the document of gitignore (http://git-scm.com/docs/gitignore), and learned that $GIT_DIR/info/exclude has higher precedence than the file specified by core.excludesfile. But I noticed that patterns in core.excludesfile override patterns in $GIT_DIR/info/exclude. I tend to agree that info/exclude which is per-repository personal preference should take precedence over $XDG_HOME/git/ignore which is a personal preference across repositories that are accessed from that machine. It appears that the precedence was screwed-up between these two files from the very beginning when 896bdfa2 (add: Support specifying an excludes file with a configuration variable, 2007-02-27) introduced core.excludesfile variable; seeing that nobody so far complained with the discrepancy between the documentation and the behaviour, it would indicate either (1) nobody reads the docs, or (2) nobody uses both at the same time. Swapping the order in the code this late in the game after 8 years may affect people who have come to rely on the current behaviour and never read the doc, which is somewhat worrying, though. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
Johannes, You’re correct, looking back over it, I was pretty vague. In truth, we are not using Windows OR putty at all. Running git on an Ubuntu system, but we are setting the GIT_SSH environment variable to point to a shell script to use. Upon attempting to run git ls-remote, the system was spitting out getaddrinfo errors for ‘atch’ . Setting GIT_TRACE=2 showed that -batch was being added to the git command. This was seen on several different servers with git versions 1.8.5.2, 1.9.1 and 2.3.5 After a bit we realized that it was the string ‘uplink’ in the GIT_SSH variable that was linked to the extra -batch flag. Finally, after searching the git source, we narrowed it down to the ‘plink’ portion of the string. https://github.com/git/git/blob/7c597ef345aed345576de616c51f27e6f4b342b3/connect.c#L747-L756 On Apr 22, 2015, at 12:46 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Patrick, On 2015-04-22 16:36, Patrick Sharp wrote: The plink string detection in GIT_SSH for setting putty to true is very broad. Wow. You probably wanted to state that you are using Windows, downloaded Git from [link here], that you are using [version] and that you use PLink [version] (from the Putty package downloaded [link here]) to do your ssh business. Without that information, you leave readers who have no idea about Putty *quite* puzzled. If plink is anywhere in the path to the shell file then putty gets set to true and ssh will fail trying to parse -batch as the hostname. This is cryptic even for me. Wouldn’t searching for plink.exe be better?-- I invite you to try your hand at improving anything you find flawed. For example, if you want to improve the PLink detection in Git for Windows 1.x, this would be the correct place to start: https://github.com/msysgit/msysgit/blob/70f24b4b0f5f86a5e85f7264a4ee2c0fec2d4391/share/WinGit/install.iss#L232-L253 (yes, you would have to download the development environment from https://msysgit.github.com/#download-msysgit and rebuild your own installer using `/share/msysGit/WinGit/release.sh 1.9.5-patrick` after editing the installer script). Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Another approach to large transactions
On Tue, Apr 21, 2015 at 4:21 PM, Jeff King p...@peff.net wrote: On Mon, Apr 20, 2015 at 05:31:11PM -0700, Stefan Beller wrote: When running the test locally, i.e. not in the test suite, but typing the commands myself into the shell, Git is fine with having just 5 file descriptors left. The additional 4 required fds come from beign run inside the test suite. When strace-ing git, I cannot see any possible other fds which would require having some left over space required. So I'd propose we'd just take a reasonable number not too small for various test setups like 32 and then go with the proposed patches. FWIW, we already use a magic value of 25 extra in open_packed_git_1. I don't know if that means the number has been proven in practice, or if it is simply that nobody actually exercises the pack_max_fds code. I suspect it is the latter, especially since d131b7a (sha1_file.c: Don't retain open fds on small packs, 2011-03-02). 25 is equally sound as I could not find any hard calculation on that number in the history or code. I will change it to 25 in the next version of the patch. Thanks! Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RFC/Add documentation for version protocol 2
Stefan Beller sbel...@google.com writes: This adds the design document for protocol version 2. It's better to rewrite the design document instead of trying to squash it into the existing pack-protocol.txt and then differentiating between version 1 and 2 all the time. Just a handful of random thoughts, without expressing agreement or objection at this point. diff --git a/Documentation/technical/pack-protocol-2.txt I wonder, if we are really revamping, if we want this to be limited to pack protocol (see more below). +General structure += + +There are four phases involved in the protocol, which are described below: + +1) capability negotiation +2) goal annoncement +3) reference advertisement +4) pack transfer + + +1) Capability negotiation +- + +In this phase both client and server send their capabilities to the other side +using the following protocol: + +--- +list-of-capabilities = *capability flush-pkt +capability = PKT-LINE(1*(LC_ALPHA / DIGIT / - / _)) + + +The capabilities itself are described in protocol-capabilities.txt +Sending the capabilities to the other side MAY happen concurrently or +one after another. There is no order who sends first. Doesn't that set us up for an easy deadlock (i.e. both sides fill their outgoing pipe because they are not listening)? +2) Goal annoncement +--- + +The goal of this phase is for the client to tell the server what +outcome it expects from this communication, such as pushing or +pulling data from the server. + +--- +list-of-goals= *goal +goal = PKT-LINE(action-line) +action-line = action *(SP action-parameter) +action = noop / ls-remote / fetch / push / fetch-shallow This is interesting, in that it implies that you can connect to a service and after learning what is running on the other hand then decide you would be fetching or pushing. Which is *never* be possible with v1 where you first connect to a specific service that knows how to handle fetch. If we are going in this in-protocol message switches the service route, we should also support archive as one of the actions, no? Yes, I know you named the document pack-protocol and archive does not give you packs, but ls-remote does not transfer pack data, either. And when we add archive (and later refer to bundle on CDN to help initial clone), it would become clear that steps #3 and #4 are optional components that are shared by majority of the protocol users (i.e. fetch, push, ls-remote uses #3, fetch, push uses #4.), and other services that also use the protocol need their own equivalents for steps #3 and #4. Of course, we do not have to do it this way and stick to one 'goal' per service is pre-arranged before the protocol exchange happens, either via git-daemon or ssh shell command line interactiosn way of doing things we have always done in v1 protocol. I have to wonder what role, if any, should git daemon (and its equivalent, the shell command line, if the transport is over ssh) play in this new world order. Note again that I am not objecting. I am trying to fathom the ramifications of what you wrote here. +3) Ref advertisement + +3) and 4) are highly dependant on the mode for fetch and push. As currently +only mode version1 is supported, the these phases follow the ref advertisement +in pack protocol version 1 without capabilities on the first line of the refs. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] stash: require a clean index to apply
If you have staged contents in your index and run stash
apply, we may hit a conflict and put new entries into the
index. Recovering to your original state is difficult at
that point, because tools like git reset --keep will blow
away anything staged. We can make this safer by refusing to
apply when there are staged changes.
It's possible we could provide better tooling here, as git
stash apply should be writing only conflicts to the index
(so we know that any stage-0 entries are potentially
precious). But it is the odd duck; most mergy commands
will update the index for cleanly merged entries, and it is
not worth updating our tooling to support this use case
which is unlikely to be of interest (besides which, we would
still need to block a dirty index for stash apply --index,
since that case _would_ be ambiguous).
Signed-off-by: Jeff King p...@peff.net
---
git-stash.sh | 2 ++
t/t3903-stash.sh | 7 +++
2 files changed, 9 insertions(+)
diff --git a/git-stash.sh b/git-stash.sh
index d4cf818..cc28368 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -442,6 +442,8 @@ apply_stash () {
assert_stash_like $@
git update-index -q --refresh || die $(gettext unable to refresh
index)
+ git diff-index --cached --quiet --ignore-submodules HEAD -- ||
+ die $(gettext Cannot apply stash: Your index contains
uncommitted changes.)
# current index state
c_tree=$(git write-tree) ||
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index f179c93..0746eee 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -45,6 +45,13 @@ test_expect_success 'applying bogus stash does nothing' '
test_cmp expect file
'
+test_expect_success 'apply requires a clean index' '
+ test_when_finished git reset --hard
+ echo changed other-file
+ git add other-file
+ test_must_fail git stash apply
+'
+
test_expect_success 'apply does not need clean working directory' '
echo 4 other-file
git stash apply
--
2.4.0.rc2.498.g02440db
--
To unsubscribe from this list: send the line 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/RFC v3 0/4] Improving performance of git clean
On Tue, Apr 21, 2015 at 11:24 PM, Jeff King p...@peff.net wrote: If I understand correctly, the reason that you need per-run setup is that your git clean command actually cleans things, and you need to restore the original state for each time-trial. Can you instead use git clean -n to do a dry-run? I think what you are timing is really the figure out what to clean step, and not the cleaning itself. -Peff Yes, that is the problem. A dry run will spot this particular performance issue but maybe we lose some value as a general performance test if we only do half the clean? Admittedly we clearly lose some value in the current state as well due to the copying taking more time than the cleaning. I could go either way here. /Erik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] t3903: stop hard-coding commit sha1s
When testing the diff output of git stash list, we look
for the stash's subject of WIP on master: $sha1, even
though it's not relevant to the diff output. This makes the
test brittle to refactoring, as any changes to earlier tests
may impact the commit sha1.
Since we don't care about the commit subject here, we can
simply ask stash not to print it.
Signed-off-by: Jeff King p...@peff.net
---
t/t3903-stash.sh | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 1e29962..6da4856 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -695,8 +695,8 @@ test_expect_success 'setup stash with index and worktree
changes' '
'
test_expect_success 'stash list implies --first-parent -m' '
- cat expect -\EOF
- stash@{0}: WIP on master: b27a2bc subdir
+ cat expect -EOF
+ stash@{0}
diff --git a/file b/file
index 257cc56..d26b33d 100644
@@ -706,13 +706,13 @@ test_expect_success 'stash list implies --first-parent
-m' '
-foo
+working
EOF
- git stash list -p actual
+ git stash list --format=%gd -p actual
test_cmp expect actual
'
test_expect_success 'stash list --cc shows combined diff' '
cat expect -\EOF
- stash@{0}: WIP on master: b27a2bc subdir
+ stash@{0}
diff --cc file
index 257cc56,9015a7a..d26b33d
@@ -723,7 +723,7 @@ test_expect_success 'stash list --cc shows combined diff' '
-index
++working
EOF
- git stash list -p --cc actual
+ git stash list --format=%gd -p --cc actual
test_cmp expect actual
'
--
2.4.0.rc2.498.g02440db
--
To unsubscribe from this list: send the line 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/3] t3903: avoid applying onto dirty index
One of the tests in t3903 wants to make sure that applying a stash that touches only file can still happen even if there are working tree changes to other-file. To do so, it adds other-file to the index (since otherwise it is an untracked file, voiding the purpose of the test). But as we are about to refactor the dirty-index handling, and as this test does not actually care about having a dirty index (only a dirty working tree), let's bump the tracking of other-file into the setup phase, so we can have _just_ a dirty working tree here. Signed-off-by: Jeff King p...@peff.net --- t/t3903-stash.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 6da4856..f179c93 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -10,6 +10,8 @@ test_description='Test git stash' test_expect_success 'stash some dirty working directory' ' echo 1 file git add file + echo unrelated other-file + git add other-file test_tick git commit -m initial echo 2 file @@ -45,8 +47,6 @@ test_expect_success 'applying bogus stash does nothing' ' test_expect_success 'apply does not need clean working directory' ' echo 4 other-file - git add other-file - echo 5 other-file git stash apply echo 3 expect test_cmp expect file -- 2.4.0.rc2.498.g02440db -- To unsubscribe from this list: send the line 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: About my git workflow; maybe it's useful for others
On Mon, Dec 29, 2014 at 10:22 PM, Stefan Beller sbel...@google.com wrote: Hi, so I have been sending commits to the git mailing list occasionally for quite some time. In the last couple of weeks I send more and more patches to the mailing list as it's part of my job now. Here is a collection of practices I am following (or want to follow) and they seem to be effective. Most of this is already documented in various documents in Documentation/* and this email is no news for the regular contributors. It may help new comers though. * Split patches up into logical pieces as you go. It's easy to go wild and after hours of hacking you have implemented a cool new feature. This the natural flow of hacking as it's the most fun. But this approach is not easy to be reviewed. So let me explain how reviewing works in the git project. Reviewing works in the git project is quite liberal, anybody is encouraged to comment on patches flying by. Junio, the maintainer then decides which patches he picks up and merges them into the various stages of git (pu, next, master, maint). The decision which patches are worth for inclusion is based on the amount of discussion by the community and generally a patch only makes it if a concensus is met. * git send-email is the only email client I trust for sending patches IMO, sending email is the easiest part. The hard begins when you have to edit your patch and resend with the reviewers' feedback incorporated. For me that is the most tricky and hard part to get right, specially when using GMail as an email client. How do you handle that part of the process? -- Thiago Farina -- To unsubscribe from this list: send the line 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] RFC/Add documentation for version protocol 2
On Wed, Apr 22, 2015 at 12:19 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
This adds the design document for protocol version 2.
It's better to rewrite the design document instead of trying to
squash it into the existing pack-protocol.txt and then differentiating
between version 1 and 2 all the time.
Just a handful of random thoughts, without expressing agreement or
objection at this point.
diff --git a/Documentation/technical/pack-protocol-2.txt
I wonder, if we are really revamping, if we want this to be limited
to pack protocol (see more below).
+General structure
+=
+
+There are four phases involved in the protocol, which are described below:
+
+1) capability negotiation
+2) goal annoncement
+3) reference advertisement
+4) pack transfer
+
+
+1) Capability negotiation
+-
+
+In this phase both client and server send their capabilities to the other
side
+using the following protocol:
+
+---
+list-of-capabilities = *capability flush-pkt
+capability = PKT-LINE(1*(LC_ALPHA / DIGIT / - / _))
+
+
+The capabilities itself are described in protocol-capabilities.txt
+Sending the capabilities to the other side MAY happen concurrently or
+one after another. There is no order who sends first.
Doesn't that set us up for an easy deadlock (i.e. both sides fill
their outgoing pipe because they are not listening)?
I did not think of it that way, but rather was focused on wall clock
time spent waiting for the protocol to be finished. And then we want to have
as much concurrent as possible. I don't know if we ever want to touch threads
in git.
+2) Goal annoncement
+---
+
+The goal of this phase is for the client to tell the server what
+outcome it expects from this communication, such as pushing or
+pulling data from the server.
+
+---
+list-of-goals= *goal
+goal = PKT-LINE(action-line)
+action-line = action *(SP action-parameter)
+action = noop / ls-remote / fetch / push / fetch-shallow
This is interesting, in that it implies that you can connect to a
service and after learning what is running on the other hand then
decide you would be fetching or pushing. Which is *never* be
possible with v1 where you first connect to a specific service that
knows how to handle fetch.
I originally thought about it as an optimisation. Say you only want to do
a ls-remote, you don't need to start pack file creation (possibly in a
background thread?), but you know what is coming and don't need to
prepare for unknown things.
If we are going in this in-protocol message switches the service
route, we should also support archive as one of the actions, no?
Yes, I know you named the document pack-protocol and archive
does not give you packs, but ls-remote does not transfer pack data,
either.
I'll add that. Also I need to incorporate shallow in one way or another.
And when we add archive (and later refer to bundle on CDN to
help initial clone), it would become clear that steps #3 and #4 are
optional components that are shared by majority of the protocol
users (i.e. fetch, push, ls-remote uses #3, fetch, push uses #4.),
and other services that also use the protocol need their own
equivalents for steps #3 and #4.
That is my thinking as well, #3 and following are completely dependent
on the action we negotiated. Just thinking about that we could do that
also with the current protocol by invoking not just {receive, upload}-pack
but any other program on the server side.
Of course, we do not have to do it this way and stick to one 'goal'
per service is pre-arranged before the protocol exchange happens,
either via git-daemon or ssh shell command line interactiosn way of
doing things we have always done in v1 protocol.
I have to wonder what role, if any, should git daemon (and its
equivalent, the shell command line, if the transport is over ssh)
play in this new world order.
So I guess you can still use a daemon to fetch from, and by now
you could also do the authentication with git daemon (with push
certificates)
What I did not talk about in the proposal is the receiving end point.
So I think there may be a git-protocol-2 binary similar to
git-{receive, upload}-pack which you then invoke via ssh?
Note again that I am not objecting. I am trying to fathom the
ramifications of what you wrote here.
Thanks for pointing out ramifications I did not think of yet!
What this new protocol is all about is the future flexibility,
so I think it is good to have lots of possibilities available.
(So for example with having 2 goals as above inside one
protocol exchange, you could also do a partially narrow/shallow
clone. So shallow for the whole repository, but deepened for
a narrow directory you're really interested in. I am not saying
this comes live in the near future, but it is possible to implement
using this protocol and still
Re: [PATCH/RFC v3 0/4] Improving performance of git clean
On Wed, Apr 22, 2015 at 09:30:20PM +0200, erik elfström wrote: On Tue, Apr 21, 2015 at 11:24 PM, Jeff King p...@peff.net wrote: If I understand correctly, the reason that you need per-run setup is that your git clean command actually cleans things, and you need to restore the original state for each time-trial. Can you instead use git clean -n to do a dry-run? I think what you are timing is really the figure out what to clean step, and not the cleaning itself. Yes, that is the problem. A dry run will spot this particular performance issue but maybe we lose some value as a general performance test if we only do half the clean? Admittedly we clearly lose some value in the current state as well due to the copying taking more time than the cleaning. I could go either way here. I guess it is a matter of opinion. I think testing only the find out what to clean half separately is actually beneficial, because it helps us isolate any slowdown. If we want to add a test for the other half, we can, but I do not actually think it is currently that interesting (it is just calling unlink() in a loop). So even leaving the practical matters aside, I do not think it is a bad thing to split it up. When you add in the fact that it is practically much easier to test the first half, it seems to me that testing just that is a good first step. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About my git workflow; maybe it's useful for others
On Wed, Apr 22, 2015 at 4:50 PM, Stefan Beller sbel...@google.com wrote: On Wed, Apr 22, 2015 at 12:38 PM, Thiago Farina tfrans...@gmail.com wrote: IMO, sending email is the easiest part. The hard begins when you have to edit your patch and resend with the reviewers' feedback incorporated. For me that is the most tricky and hard part to get right, specially when using GMail as an email client. How do you handle that part of the process? I try to have as much in git as possible. So when the reviews trickle in, I change my commits (in git) accordingly via rebase and edit and lots of fixup commits. I use git notes to keep track of changes from one version to another. Having the changes of the changes in the git notes, I am (in theory) always able to kick out a new version of the patch series with rm 00* # delete old patches git format-patch --notes --coverletter somebranch...HEAD edit -cover-letter.patch git send-email 00* --to=mailing list --to=j...@doe.org --cc=m...@mustermann.de Is that capable of keeping the next patch set in the same thread that started when you sent the initial patch? Otherwise things get disconnected. -- Thiago Farina -- To unsubscribe from this list: send the line 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] pathspec: adjust prefixlen after striping trailing slash
Jens Lehmann jens.lehm...@web.de writes: Am 21.04.2015 um 23:08 schrieb Junio C Hamano: I looked at the test script update. The new test does (I am rephrasing to make it clearer): mkdir -p dir/ectory git init dir/ectory ;# a new directory inside top-level project ( cd dir/ectory test git add test git commit -m test ) git add dir/ectory to set it up. At this point, the top-level index knows dir/ectory is a submodule. Then the test goes on to turn it a non submodule by mv dir/ectory/.git dir/ectory/dotgit ... We already do (2) in the cases you describe: $ git add subrepo/a fatal: Pathspec 'subrepo/a' is in submodule 'subrepo' $ git -C subrepo add a fatal: Pathspec 'a' is in submodule 'subrepo' ... So I'd vote to have (2) also for git -C subrepo add ., which is what started this thread. Does having mv subrepo/.git subrepo/dotgit before that git add change your conclusion? It is very clear to me that without that mv step, (2) is absolutely the right thing to do, and I agree with you. But it is unclear if we should still do (2) when subrepo/.git is no longer there. That has to be done manually and it may be an indication that is clear enough that the end user wants the directory to be a normal directory without any submodule involved, in which case it may match the expectation of the user better to just nuke the corresponding 16 entry in the index and replace it with files in there. I dunno. In my quick test, it appeared that the behaviour with this patch applied was neither of the two and instead to silently do nothing, and if that is the case I think it is quite wrong. Yep. -- To unsubscribe from this list: send the line 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] pathspec: adjust prefixlen after striping trailing slash
Am 21.04.2015 um 23:08 schrieb Junio C Hamano: Duy Nguyen pclo...@gmail.com writes: On Mon, Apr 20, 2015 at 12:37 PM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: But if you look at it another way, cd subrepo; git add . should be the same as git add subrepo ... Once you cd into subrepo, you are in a different world, a world different from the toplevel project. git add . over there should mean add everything in subproject's working tree to subproject's index, shouldn't it? On the other hand, git add subrepo without leavingin the working tree of the superproject is about binding the submodule to the superproject's index. I do not think these two should be the same. Where am I mistaken? I think I wrote this sentence and deleted it: I didn't say which way was right. OK. I looked at the test script update. The new test does (I am rephrasing to make it clearer): mkdir -p dir/ectory git init dir/ectory ;# a new directory inside top-level project ( cd dir/ectory test git add test git commit -m test ) git add dir/ectory to set it up. At this point, the top-level index knows dir/ectory is a submodule. Then the test goes on to turn it a non submodule by mv dir/ectory/.git dir/ectory/dotgit At this point, I think there are two valid things that can happen. (1) git add test inside dir/ectory removes the submodule entry dir/ectory and then adds dir/ectory/test as an individual entry. After all that is what happens when you replace a file with a directory, e.g. folder git add folder rm folder mkdir folder folder/file git -C folder add file will first register a regular file folder and then replace that with paths underneath. It would be the same if you did any of the following: git -C . add dir/ectory/test git -C dir add ectory/test git -C dir/ectory add test (2) git add test inside dir/ectory would barf, saying dir/ectory is supposed to be a submodule and you have to first remove it. Again, it would be the same if you did so from any directory with relative paths. I think (2) is less optimal than (1), but it could be the best we could do if the submodule infrastracture around .gitmodules and links to $GIT_DIR/modules/$name may have to get involved in an operation like this (I didn't check). We already do (2) in the cases you describe: $ git add subrepo/a fatal: Pathspec 'subrepo/a' is in submodule 'subrepo' $ git -C subrepo add a fatal: Pathspec 'a' is in submodule 'subrepo' And I believe that is better than (1), because when removing a submodule its entry in .gitmodules should also be removed. And I suspect that adding a file in a submodule to the superproject will happen more by accident than on purpose (which cannot happen for files because when you add a new file in a directory for which git still records a file the latter can be safely removed as that entry cannot be a file in the worktree anymore). So I'd vote to have (2) also for git -C subrepo add ., which is what started this thread. In my quick test, it appeared that the behaviour with this patch applied was neither of the two and instead to silently do nothing, and if that is the case I think it is quite wrong. Yep. -- To unsubscribe from this list: send the line 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] stop putting argv[0] dirname at front of PATH
Hi, Jeff King wrote: This was added long ago by by 231af83 (Teach the git command to handle some commands internally, 2006-02-26), with the intent that things would just work if you did something like: cd /opt tar xzf premade-git-package.tar.gz alias git=/opt/git/bin/git as we would then find all of the related external commands in /opt/git/bin. I.e., it made git runtime-relocatable, since at the time of 231af83, we installed all of the git commands into $(bindir). [...] And not just for finding binaries; we want to find $(sharedir), etc, the same way. The RUNTIME_PREFIX build knob does this the right way Makes sense. For the reason you say (templatedir, etc) I am surprised to hear that that was the motivation, but I can't find any other. [...] Signed-off-by: Jeff King p...@peff.net For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com [...] But note that you can't just flip on RUNTIME_PREFIX on non-Windows systems, as some invocations will get the full path to the executable, and others see just git. You'd need to convert that into an absolute path (either by searching the $PATH, or doing something system-specific like looking in /proc/$$/exe). Yep --- /proc/self/exe should work okay if someone wants portable git to work on Linux. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How do I resolve conflict after popping stash without adding the file to index?
On Wed, Apr 22, 2015 at 02:35:40PM -0400, Jeff King wrote:
Ironically, the message before e0e2a9c actually recommends staging
changes before applying the stash, which would lead to this exact
situation! So I think the most trivial patch is:
diff --git a/git-stash.sh b/git-stash.sh
index d4cf818..f1865c9 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -442,6 +442,7 @@ apply_stash () {
assert_stash_like $@
git update-index -q --refresh || die $(gettext unable to refresh
index)
+ git diff-index --cached HEAD || die dirty index; cannot apply stash
# current index state
c_tree=$(git write-tree) ||
but it makes me wonder if somebody would find it annoying that they
cannot apply a stash into their work-in-progress (i.e., it _might_ cause
annoyance, but most of the time it will be convenient to do so).
It does actually fail a test in t3903, but I think that test just
incidentally had a dirty index, and didn't care about that particular
feature.
We also have require_clean_work_tree() in git-sh-setup.sh. We definitely
don't want the first half of that, but we do want the diff-index check.
So probably we'd want to refactor that into two separate functions, and
only call the require_clean_index part.
This turned out to be more work than it was worth. Most of the effort
in that function is about adjusting the messages to handle the cases
when either or both of the working tree and index are dirty. I did pick
up the useful bits from there, though:
- use --quiet to suppress output and so that the exit code actually
matters
- use -- to disambiguate the ref
- I didn't pick up the `rev-parse HEAD` call. I don't think it's
necessary (i.e., diff-index should barf for us if it can't read
HEAD).
Here are the patches.
[1/3]: t3903: stop hard-coding commit sha1s
[2/3]: t3903: avoid applying onto dirty index
[3/3]: stash: require a clean index to apply
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How do I resolve conflict after popping stash without adding the file to index?
Jeff King p...@peff.net writes:
Ironically, the message before e0e2a9c actually recommends staging
changes before applying the stash, which would lead to this exact
situation!
The ancient history is hazy to me, but did we fall back to three-way
merge in old days (or did anything to the index for that matter), I
wonder? In a world git stash apply only applied the change to the
working tree via git apply, that old recommendation would make
perfect sense.
But obviously we do not live in such a world right now. And because
we are doing merge-recursive, we should insist on a clean index;
otherwise there is no way to undo its effect without losing the
changes by the end-user.
So I think the most trivial patch is:
diff --git a/git-stash.sh b/git-stash.sh
index d4cf818..f1865c9 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -442,6 +442,7 @@ apply_stash () {
assert_stash_like $@
git update-index -q --refresh || die $(gettext unable to refresh
index)
+ git diff-index --cached HEAD || die dirty index; cannot apply stash
Yes, that makes sense. The original report from Dmitry was
triggering the safety from one line above and git stash pop doing
the right thing by refusing to touch the index with unresolved mergy
operation before doing anything, and with this additional safety, we
would make it even safer from people who do git add and then git
stash pop (which is somewhat strange thing to do, given that
stash was designed for stash to save away; do other things; come
back to the original commit state that is 'reset --hard' clean;
unstash sequence in the first place).
# current index state
c_tree=$(git write-tree) ||
but it makes me wonder if somebody would find it annoying that they
cannot apply a stash into their work-in-progress (i.e., it _might_ cause
annoyance, but most of the time it will be convenient to do so).
They can always do git stash show -p stash@{n} | git apply if they
want to build changes incrementally X-, but it would be annoying.
So probably we'd want to refactor that into two separate functions, and
only call the require_clean_index part.
Hmph, but what would that helper do, other than a single diff-index
--quiet --cached HEAD call?
--
To unsubscribe from this list: send the line 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: About my git workflow; maybe it's useful for others
On Wed, Apr 22, 2015 at 12:38 PM, Thiago Farina tfrans...@gmail.com wrote: IMO, sending email is the easiest part. The hard begins when you have to edit your patch and resend with the reviewers' feedback incorporated. For me that is the most tricky and hard part to get right, specially when using GMail as an email client. How do you handle that part of the process? I try to have as much in git as possible. So when the reviews trickle in, I change my commits (in git) accordingly via rebase and edit and lots of fixup commits. I use git notes to keep track of changes from one version to another. Having the changes of the changes in the git notes, I am (in theory) always able to kick out a new version of the patch series with rm 00* # delete old patches git format-patch --notes --coverletter somebranch...HEAD edit -cover-letter.patch git send-email 00* --to=mailing list --to=j...@doe.org --cc=m...@mustermann.de which is only a few steps, so there is not much to go wrong here. For me the biggest thing is to know when to send out new patches. (Do I sleep over it and review it again myself, or do I just gun it, believing my patches are good this time?) -- Thiago Farina -- To unsubscribe from this list: send the line 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/RFC v3 0/4] Improving performance of git clean
On Wed, Apr 22, 2015 at 9:46 PM, Jeff King p...@peff.net wrote: On Wed, Apr 22, 2015 at 09:30:20PM +0200, erik elfström wrote: Yes, that is the problem. A dry run will spot this particular performance issue but maybe we lose some value as a general performance test if we only do half the clean? Admittedly we clearly lose some value in the current state as well due to the copying taking more time than the cleaning. I could go either way here. I guess it is a matter of opinion. I think testing only the find out what to clean half separately is actually beneficial, because it helps us isolate any slowdown. If we want to add a test for the other half, we can, but I do not actually think it is currently that interesting (it is just calling unlink() in a loop). So even leaving the practical matters aside, I do not think it is a bad thing to split it up. When you add in the fact that it is practically much easier to test the first half, it seems to me that testing just that is a good first step. -Peff Sounds reasonable to me. I'll make this change in v4, thanks! (Sorry for the duplicate email Jeff, I'm bad at this mailing list thing...) /Erik -- To unsubscribe from this list: send the line 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: How do I resolve conflict after popping stash without adding the file to index?
On Wed, Apr 22, 2015 at 12:45:21PM -0700, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
Ironically, the message before e0e2a9c actually recommends staging
changes before applying the stash, which would lead to this exact
situation!
The ancient history is hazy to me, but did we fall back to three-way
merge in old days (or did anything to the index for that matter), I
wonder? In a world git stash apply only applied the change to the
working tree via git apply, that old recommendation would make
perfect sense.
Hmm, that advice came in 2a79d2f (Clarify how the user can satisfy
stash's 'dirty state' check., 2008-09-29), at which point it looks like
we were already running merge-recursive. So I think it was simply bad
advice. ;)
But obviously we do not live in such a world right now. And because
we are doing merge-recursive, we should insist on a clean index;
otherwise there is no way to undo its effect without losing the
changes by the end-user.
Yeah, agreed.
but it makes me wonder if somebody would find it annoying that they
cannot apply a stash into their work-in-progress (i.e., it _might_ cause
annoyance, but most of the time it will be convenient to do so).
They can always do git stash show -p stash@{n} | git apply if they
want to build changes incrementally X-, but it would be annoying.
I think the best thing to do is introduce this safety, let it cook for a
while, and see what comes up. Perhaps we could add a --force or
similar, but I'd rather see if anybody ever actually runs into the
situation first.
So probably we'd want to refactor that into two separate functions, and
only call the require_clean_index part.
Hmph, but what would that helper do, other than a single diff-index
--quiet --cached HEAD call?
I was wanting to keep the error message and the flags we feed to
diff-index consistent. But yeah, there is little enough duplicate
material and enough added boilerplate that I do not think it is worth it
(see the series I just posted).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] stop putting argv[0] dirname at front of PATH
On Wed, Apr 22, 2015 at 12:23:17PM -0700, Jonathan Nieder wrote: And not just for finding binaries; we want to find $(sharedir), etc, the same way. The RUNTIME_PREFIX build knob does this the right way Makes sense. For the reason you say (templatedir, etc) I am surprised to hear that that was the motivation, but I can't find any other. The Makefile was much less mature, then, so I wondered if maybe those things came later (certainly the munging of $PATH that introduced the problems came later, as at the time we had a special we are exec-ing a git command custom version of execvp). But no, the template directory did indeed exist then. I think it was always a half-baked idea (and got much worse when we stopped putting git-foo into $bindir). The relevant thread (which I think I linked the other day) is: http://article.gmane.org/gmane.comp.version-control.git/16798 -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RFC/Add documentation for version protocol 2
Stefan Beller sbel...@google.com writes: +action = noop / ls-remote / fetch / push / fetch-shallow ... If we are going in this in-protocol message switches the service route, we should also support archive as one of the actions, no? Yes, I know you named the document pack-protocol and archive does not give you packs, but ls-remote does not transfer pack data, either. I'll add that. Also I need to incorporate shallow in one way or another. This level of detail may not matter at this point yet, but it is unclear to me why you have fetch-shallow as a separate thing (while not having push-shallow). The current infrastructure does already allow fetching into shallow repositories witout needing a separate action that is different from fetch (aka upload-pack). I would not be surprised if it were I can deepn you if you want capability, but I do not understand why you are singling out shallow as something that needs such a special treatment. -- To unsubscribe from this list: send the line 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] git-multimail: Add an option to filter on branches
On Wed, Apr 22, 2015 at 7:44 PM, Michael Haggerty mhag...@alum.mit.edu wrote: I think what you are looking for is return any(r.match(branch) for r in branches) Yup, thats exactly what I wanted. I'll submit an updated patch I was also wondering why you decided to support comma-separated lists of regexps *and* multivalued config settings. It seems that supporting only multivalued settings would suffice. In practice we are using gitolite and git-multimail. Allowing both means our gitolite.conf file can look like repo foo # always notify on master config multimailhook.branches = master # notify for the current pre-release branches config multimailhook.branches = v1.7-pre, v1.8-pre -- Dave B -- To unsubscribe from this list: send the line 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] connect: simplify SSH connection code path
The code path used in git_connect pushed the majority of the SSH
connection code into an else block, even though the if block returns.
Simplify the code by eliminating the else block, as it is unneeded.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
connect.c | 42 +-
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/connect.c b/connect.c
index 391d211..749a07b 100644
--- a/connect.c
+++ b/connect.c
@@ -743,28 +743,28 @@ struct child_process *git_connect(int fd[2], const char
*url,
free(path);
free(conn);
return NULL;
- } else {
- ssh = getenv(GIT_SSH_COMMAND);
- if (ssh) {
- conn-use_shell = 1;
- putty = 0;
- } else {
- ssh = getenv(GIT_SSH);
- if (!ssh)
- ssh = ssh;
- putty = !!strcasestr(ssh, plink);
- }
-
- argv_array_push(conn-args, ssh);
- if (putty !strcasestr(ssh, tortoiseplink))
- argv_array_push(conn-args, -batch);
- if (port) {
- /* P is for PuTTY, p is for OpenSSH */
- argv_array_push(conn-args, putty ?
-P : -p);
- argv_array_push(conn-args, port);
- }
- argv_array_push(conn-args, ssh_host);
}
+
+ ssh = getenv(GIT_SSH_COMMAND);
+ if (ssh) {
+ conn-use_shell = 1;
+ putty = 0;
+ } else {
+ ssh = getenv(GIT_SSH);
+ if (!ssh)
+ ssh = ssh;
+ putty = !!strcasestr(ssh, plink);
+ }
+
+ argv_array_push(conn-args, ssh);
+ if (putty !strcasestr(ssh, tortoiseplink))
+ argv_array_push(conn-args, -batch);
+ if (port) {
+ /* P is for PuTTY, p is for OpenSSH */
+ argv_array_push(conn-args, putty ? -P :
-p);
+ argv_array_push(conn-args, port);
+ }
+ argv_array_push(conn-args, ssh_host);
} else {
/* remove repo-local variables from the environment */
conn-env = local_repo_env;
--
2.3.5
--
To unsubscribe from this list: send the line 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] connect: improve check for plink to reduce false positives
The git_connect function has code to handle plink and tortoiseplink
specially, as they require different command line arguments from
OpenSSH. However, the match was done by checking for plink
case-insensitively in the string, which led to false positives when
GIT_SSH contained uplink. Improve the check by looking for plink or
tortoiseplink only at the beginning of the string or immediately after
a directory separator.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
This is essentially just a deindentation.
connect.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/connect.c b/connect.c
index 749a07b..bae76a2 100644
--- a/connect.c
+++ b/connect.c
@@ -724,7 +724,7 @@ struct child_process *git_connect(int fd[2], const char
*url,
conn-in = conn-out = -1;
if (protocol == PROTO_SSH) {
const char *ssh;
- int putty;
+ int putty, tortoiseplink = 0;
char *ssh_host = hostandport;
const char *port = NULL;
get_host_and_port(ssh_host, port);
@@ -750,14 +750,23 @@ struct child_process *git_connect(int fd[2], const char
*url,
conn-use_shell = 1;
putty = 0;
} else {
+ char *plink, *tplink;
+
ssh = getenv(GIT_SSH);
if (!ssh)
ssh = ssh;
- putty = !!strcasestr(ssh, plink);
+
+ plink = strcasestr(ssh, plink);
+ tplink = strcasestr(ssh, tortoiseplink);
+
+ tortoiseplink = tplink == ssh ||
+ (tplink is_dir_sep(tplink[-1]));
+ putty = tortoiseplink || plink == ssh ||
+ (plink is_dir_sep(plink[-1]));
}
argv_array_push(conn-args, ssh);
- if (putty !strcasestr(ssh, tortoiseplink))
+ if (tortoiseplink)
argv_array_push(conn-args, -batch);
if (port) {
/* P is for PuTTY, p is for OpenSSH */
--
2.3.5
--
To unsubscribe from this list: send the line 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 01/16] refs: convert struct ref_entry to use struct object_id
On Wed, Apr 22, 2015 at 4:24 PM, brian m. carlson
sand...@crustytoothpaste.net wrote:
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
refs.c | 44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/refs.c b/refs.c
index 81a455b..522d15d 100644
--- a/refs.c
+++ b/refs.c
@@ -156,7 +156,7 @@ struct ref_value {
* null. If REF_ISSYMREF, then this is the name of the object
* referred to by the last reference in the symlink chain.
*/
- unsigned char sha1[20];
+ struct object_id oid;
/*
* If REF_KNOWS_PEELED, then this field holds the peeled value
@@ -164,7 +164,7 @@ struct ref_value {
* be peelable. See the documentation for peel_ref() for an
* exact definition of peelable.
*/
- unsigned char peeled[20];
+ struct object_id peeled;
};
struct ref_cache;
@@ -346,8 +346,8 @@ static struct ref_entry *create_ref_entry(const char
*refname,
die(Reference has invalid format: '%s', refname);
len = strlen(refname) + 1;
ref = xmalloc(sizeof(struct ref_entry) + len);
- hashcpy(ref-u.value.sha1, sha1);
- hashclr(ref-u.value.peeled);
+ hashcpy(ref-u.value.oid.hash, sha1);
+ oidclr(ref-u.value.peeled);
memcpy(ref-name, refname, len);
ref-flag = flag;
return ref;
@@ -621,7 +621,7 @@ static int is_dup_ref(const struct ref_entry *ref1, const
struct ref_entry *ref2
/* This is impossible by construction */
die(Reference directory conflict: %s, ref1-name);
- if (hashcmp(ref1-u.value.sha1, ref2-u.value.sha1))
+ if (oidcmp(ref1-u.value.oid, ref2-u.value.oid))
die(Duplicated ref, and SHA1s don't match: %s, ref1-name);
So you're switching the code for a possible future
In an earlier series/cover letter you wrote
The goal of this series to improve type-checking in the codebase and to
make it easier to move to a different hash function if the project
decides to do that. This series does not convert all of the codebase,
but only parts. I've dropped some of the patches from earlier (which no
longer apply) and added others.
Which yields the question if you also want to take care of the error message
(It may not be a SHA1 any more but some $HASHFUNCTION)?
That said I'll focus on the type checking part in this review
and not annotate the SHA1s I find any more. ;)
warning(Duplicated ref: %s, ref1-name);
@@ -669,7 +669,7 @@ static int ref_resolves_to_object(struct ref_entry *entry)
{
if (entry-flag REF_ISBROKEN)
return 0;
- if (!has_sha1_file(entry-u.value.sha1)) {
+ if (!has_sha1_file(entry-u.value.oid.hash)) {
error(%s does not point to a valid object!, entry-name);
return 0;
}
@@ -717,7 +717,7 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
- retval = data-fn(entry-name + data-trim, entry-u.value.sha1,
+ retval = data-fn(entry-name + data-trim, entry-u.value.oid.hash,
entry-flag, data-cb_data);
current_ref = old_current_ref;
return retval;
@@ -1193,7 +1193,7 @@ static void read_packed_refs(FILE *f, struct ref_dir
*dir)
line.len == PEELED_LINE_LENGTH
line.buf[PEELED_LINE_LENGTH - 1] == '\n'
!get_sha1_hex(line.buf + 1, sha1)) {
- hashcpy(last-u.value.peeled, sha1);
+ hashcpy(last-u.value.peeled.hash, sha1);
/*
* Regardless of what the file header said,
* we definitely know the value of *this*
@@ -1374,7 +1374,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache
*refs,
if (ref == NULL)
return -1;
- hashcpy(sha1, ref-u.value.sha1);
+ hashcpy(sha1, ref-u.value.oid.hash);
return 0;
}
@@ -1461,7 +1461,7 @@ static int resolve_missing_loose_ref(const char
*refname,
*/
entry = get_packed_ref(refname);
if (entry) {
- hashcpy(sha1, entry-u.value.sha1);
+ hashcpy(sha1, entry-u.value.oid.hash);
if (flags)
*flags |= REF_ISPACKED;
return 0;
@@ -1771,9 +1771,9 @@ static enum peel_status peel_entry(struct ref_entry
*entry, int repeel)
if (entry-flag REF_KNOWS_PEELED) {
if (repeel) {
entry-flag = ~REF_KNOWS_PEELED;
- hashclr(entry-u.value.peeled);
+ oidclr(entry-u.value.peeled);
} else {
-
Re: [PATCH] pathspec: adjust prefixlen after striping trailing slash
On Wed, Apr 22, 2015 at 3:32 PM, Jens Lehmann jens.lehm...@web.de wrote: ... But it is unclear if we should still do (2) when subrepo/.git is no longer there. That has to be done manually and it may be an indication that is clear enough that the end user wants the directory to be a normal directory without any submodule involved, in which case it may match the expectation of the user better to just nuke the corresponding 16 entry in the index and replace it with files in there. I dunno. The user having removed subrepo/.git is just one reason for that. Another is a user adding a file in an unpopulated work tree of a not initialized submodule. I doubt that simply nuking the 16 entry would be the right thing to do in this case, I expect this to be a pilot error we should barf about ;-) OK, that sounds sensible. -- To unsubscribe from this list: send the line 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: About my git workflow; maybe it's useful for others
On Wed, Apr 22, 2015 at 12:57 PM, Thiago Farina tfrans...@gmail.com wrote: On Wed, Apr 22, 2015 at 4:50 PM, Stefan Beller sbel...@google.com wrote: On Wed, Apr 22, 2015 at 12:38 PM, Thiago Farina tfrans...@gmail.com wrote: IMO, sending email is the easiest part. The hard begins when you have to edit your patch and resend with the reviewers' feedback incorporated. For me that is the most tricky and hard part to get right, specially when using GMail as an email client. How do you handle that part of the process? I try to have as much in git as possible. So when the reviews trickle in, I change my commits (in git) accordingly via rebase and edit and lots of fixup commits. I use git notes to keep track of changes from one version to another. Having the changes of the changes in the git notes, I am (in theory) always able to kick out a new version of the patch series with rm 00* # delete old patches git format-patch --notes --coverletter somebranch...HEAD edit -cover-letter.patch git send-email 00* --to=mailing list --to=j...@doe.org --cc=m...@mustermann.de Is that capable of keeping the next patch set in the same thread that started when you sent the initial patch? Otherwise things get disconnected. When typing it out quickly I forgot the --in-reply-to=identifier option for the git send-email command. The identifier needs to be looked u0p manually, which is still a pain point in my workflow. -- Thiago Farina -- To unsubscribe from this list: send the line 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 V3 0/2] git-p4: improve client path detection when branches are used
On 22/04/15 18:11, Junio C Hamano wrote: Vitor Antunes vitor@gmail.com writes: The updates introduced in the third revision of these two patches consist only on updates to the commit messages to better clarify what they implement. Vitor Antunes (2): t9801: check git-p4's branch detection with client spec enabled git-p4: improve client path detection when branches are used git-p4.py| 13 -- t/t9801-git-p4-branch.sh | 106 ++ 2 files changed, 115 insertions(+), 4 deletions(-) Thanks; will re-queue. Luke, could you comment? First off: kudos to Vitor for daring to enter this particular dragon's den. The combination of branch-detection and use-client-spec isn't so bad, but throwing in the handling of excluding bits of the tree via the P4 client spec (like, who would even do that?) makes it into a real mind twister! I've held off commenting as I don't feel I know the branch detection code as well as I would like. The change though seems a lot more robust now that the search is anchored. Having a test case is always good! However, playing around with this (incredibly complex and obscure) scenario, I'm not yet sure about it. I created a depot that had //depot/main and //depot/branch, and a branch mapping between the two. I cloned that in git using --use-client-spec and --branch-detect, and all was well. I then modified my client spec to exclude //depot/main/excluded, and then started adding files in git to the 'excluded' directory. When I submit them, I get: $ echo hello excluded/f1.c $ echo hello f2.c $ git add excluded/f1.c f2.c $ git commit -m 'Partially excluded' $ git-p4.py submit DEBUG: self.useClientSpec = True Perforce checkout for depot path //depot/main/ located at /home/lgd/p4-hacking/cli/main/ Synchronizing p4 checkout... ... - file(s) up-to-date. Applying 51f187b Excluded added from git excluded/c - file(s) not in client view. excluded/c - file(s) not opened on this client. Could not determine file type for excluded/c (result: '') When I reverted this change, it failed differently, and appeared to be extremely confused in the way that I think Vitor originally describes, getting hopelessly baffled by the client spec layout. It's entirely possibly I've messed up my manual testing though. I need to go and have a very strong cup of tea before I can look at this again. Thanks! Luke -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On Wed, Apr 22, 2015 at 05:29:04PM -0400, Jeff King wrote:
Perhaps it would be worthwhile to check instead if the text plink is
the beginning of string or is preceded by a path separator. That would
give us a bit more confidence that the user is looking for plink, but
would still allow people to use plink-0.63 if they like.
Yeah, I think that is a reasonable approach. Note that it needs to
handle the tortoiseplink case from below, too (you can still use your
strategy, you just need to look for either string).
So maybe something like this?
diff --git a/connect.c b/connect.c
index 391d211..ba3ab34 100644
--- a/connect.c
+++ b/connect.c
@@ -749,10 +749,15 @@ struct child_process *git_connect(int fd[2], const char
*url,
conn-use_shell = 1;
putty = 0;
} else {
+ char *plink, *tplink;
+
ssh = getenv(GIT_SSH);
if (!ssh)
ssh = ssh;
- putty = !!strcasestr(ssh, plink);
+ plink = strcasestr(ssh, plink);
+ tplink = strcasestr(ssh,
tortoiseplink);
+ putty = plink == ssh || (plink
is_dir_sep(plink[-1])) ||
+ tplink == ssh || (tplink
is_dir_sep(tplink[-1]));
}
argv_array_push(conn-args, ssh);
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
signature.asc
Description: Digital signature
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On Wed, Apr 22, 2015 at 09:44:45PM +, brian m. carlson wrote:
On Wed, Apr 22, 2015 at 05:29:04PM -0400, Jeff King wrote:
Perhaps it would be worthwhile to check instead if the text plink is
the beginning of string or is preceded by a path separator. That would
give us a bit more confidence that the user is looking for plink, but
would still allow people to use plink-0.63 if they like.
Yeah, I think that is a reasonable approach. Note that it needs to
handle the tortoiseplink case from below, too (you can still use your
strategy, you just need to look for either string).
So maybe something like this?
diff --git a/connect.c b/connect.c
index 391d211..ba3ab34 100644
--- a/connect.c
+++ b/connect.c
@@ -749,10 +749,15 @@ struct child_process *git_connect(int fd[2], const char
*url,
conn-use_shell = 1;
putty = 0;
} else {
+ char *plink, *tplink;
+
ssh = getenv(GIT_SSH);
if (!ssh)
ssh = ssh;
- putty = !!strcasestr(ssh, plink);
+ plink = strcasestr(ssh, plink);
+ tplink = strcasestr(ssh,
tortoiseplink);
+ putty = plink == ssh || (plink
is_dir_sep(plink[-1])) ||
+ tplink == ssh || (tplink
is_dir_sep(tplink[-1]));
Yeah, that looks right to me. You might want to represent the are we
tortoise check as a separate flag, though, and reuse it a few lines
later.
Also, not related to your patch, but I notice the putty declaration is
in a different scope than I would have expected, which made me wonder if
it gets initialized in all code paths. I think is from the recent
addition of CONNECT_DIAG_URL, which pushes the bulk of the code into its
own else clause, even though the first part of the if always returns
early. I wonder if it would be simpler to read like:
diff --git a/connect.c b/connect.c
index 391d211..749a07b 100644
--- a/connect.c
+++ b/connect.c
@@ -743,28 +743,28 @@ struct child_process *git_connect(int fd[2], const char
*url,
free(path);
free(conn);
return NULL;
- } else {
- ssh = getenv(GIT_SSH_COMMAND);
- if (ssh) {
- conn-use_shell = 1;
- putty = 0;
- } else {
- ssh = getenv(GIT_SSH);
- if (!ssh)
- ssh = ssh;
- putty = !!strcasestr(ssh, plink);
- }
-
- argv_array_push(conn-args, ssh);
- if (putty !strcasestr(ssh, tortoiseplink))
- argv_array_push(conn-args, -batch);
- if (port) {
- /* P is for PuTTY, p is for OpenSSH */
- argv_array_push(conn-args, putty ?
-P : -p);
- argv_array_push(conn-args, port);
- }
- argv_array_push(conn-args, ssh_host);
}
+
+ ssh = getenv(GIT_SSH_COMMAND);
+ if (ssh) {
+ conn-use_shell = 1;
+ putty = 0;
+ } else {
+ ssh = getenv(GIT_SSH);
+ if (!ssh)
+ ssh = ssh;
+ putty = !!strcasestr(ssh, plink);
+ }
+
+ argv_array_push(conn-args, ssh);
+ if (putty !strcasestr(ssh, tortoiseplink))
+ argv_array_push(conn-args, -batch);
+ if (port) {
+ /* P is for PuTTY, p is for OpenSSH */
+ argv_array_push(conn-args, putty ? -P :
-p);
+ argv_array_push(conn-args, port);
+ }
+ argv_array_push(conn-args, ssh_host);
} else {
/* remove repo-local variables from the environment */
conn-env = local_repo_env;
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On Wed, Apr 22, 2015 at 10:24:55PM +, brian m. carlson wrote: On Wed, Apr 22, 2015 at 06:00:54PM -0400, Jeff King wrote: Yeah, that looks right to me. You might want to represent the are we tortoise check as a separate flag, though, and reuse it a few lines later. Sounds like a good idea. I'll send a more formal patch a bit later today. Thanks. Also, not related to your patch, but I notice the putty declaration is in a different scope than I would have expected, which made me wonder if it gets initialized in all code paths. I think is from the recent addition of CONNECT_DIAG_URL, which pushes the bulk of the code into its own else clause, even though the first part of the if always returns early. I wonder if it would be simpler to read like: [...] I can drop this in as a preparatory patch if I can have your sign-off. Definitely, thanks. Signed-off-by: Jeff King p...@peff.net -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On Wed, Apr 22, 2015 at 04:29:10PM -0400, Jeff King wrote:
I think you want something like:
diff --git a/connect.c b/connect.c
index 9ae991a..58aad56 100644
--- a/connect.c
+++ b/connect.c
@@ -568,7 +568,8 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
conn-argv = arg = xcalloc(7, sizeof(*arg));
if (protocol == PROTO_SSH) {
const char *ssh = getenv(GIT_SSH);
- int putty = ssh strcasestr(ssh, plink);
+ int putty = ssh (ends_with(ssh, plink) ||
+ ends_with(plink.exe));
if (!ssh) ssh = ssh;
*arg++ = ssh;
though that is not quite enough (we do not have a case-insensitive
version of ends_with). I'm also not sure if matching just plink and
plink.exe at the end of the string is enough (I'm just guessing that
was the original reason for using strstr in the first place).
Note that I don't think just switching the strcasestr to look for
plink.exe is right. For one thing, it just punts on the problem (it
can still happen, it's just less likely to trigger). But for another,
you can have plink (without .exe) on Linux systems.
Perhaps it would be worthwhile to check instead if the text plink is
the beginning of string or is preceded by a path separator. That would
give us a bit more confidence that the user is looking for plink, but
would still allow people to use plink-0.63 if they like.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
signature.asc
Description: Digital signature
[PATCHv3] refs.c: enable large transactions
This is another attempt on enabling large transactions
(large in terms of open file descriptors). We keep track of how many
lock files are opened by the ref_transaction_commit function.
When more than a reasonable amount of files is open, we close
the file descriptors to make sure the transaction can continue.
Another idea I had during implementing this was to move this file
closing into the lock file API, such that only a certain amount of
lock files can be open at any given point in time and we'd be 'garbage
collecting' open fds when necessary in any relevant call to the lock
file API. This would have brought the advantage of having such
functionality available in other users of the lock file API as well.
The downside however is the over complication, you really need to always
check for (lock-fd != -1) all the time, which may slow down other parts
of the code, which did not ask for such a feature.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
* Added error checking when reopening the lock
* Only call close_lock_file when needed.
(This makes the code a bit harder to read, but
might be worth it nevertheless. close_lock_file would
return early anyway, so we're trading off a function call
to some additional check (!(flags REF_HAVE_NEW)
||is_null_sha1(update-new_sha1))
* tuned the number of spare fd to be 25 as in the other occurence.
At least we want to be consistent with our made up ballpark numbers.
* This replaces the latest patch on origin/sb/remove-fd-from-ref-lock
refs.c| 28
t/t1400-update-ref.sh | 4 ++--
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 4f495bd..7ce7b97 100644
--- a/refs.c
+++ b/refs.c
@@ -3041,6 +3041,13 @@ static int write_ref_sha1(struct ref_lock *lock,
errno = EINVAL;
return -1;
}
+ if (lock-lk-fd == -1 reopen_lock_file(lock-lk) == -1) {
+ int save_errno = errno;
+ error(Couldn't reopen %s, lock-lk-filename.buf);
+ unlock_ref(lock);
+ errno = save_errno;
+ return -1;
+ }
if (write_in_full(lock-lk-fd, sha1_to_hex(sha1), 40) != 40 ||
write_in_full(lock-lk-fd, term, 1) != 1 ||
close_ref(lock) 0) {
@@ -3718,6 +3725,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct strbuf *err)
{
int ret = 0, i;
+ unsigned int remaining_fds;
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
struct string_list refs_to_delete = STRING_LIST_INIT_NODUP;
@@ -3733,6 +3741,20 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
return 0;
}
+ /*
+* We need to open many files in a large transaction, so come up with
+* a reasonable maximum. We still keep some spares for stdin/out and
+* other open files. Experiments determined we need more fds when
+* running inside our test suite than directly in the shell. It's
+* unclear where these fds come from. 25 should be a reasonable large
+* number though.
+*/
+ remaining_fds = get_max_fd_limit();
+ if (remaining_fds 25)
+ remaining_fds -= 25;
+ else
+ remaining_fds = 0;
+
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
if (ref_update_reject_duplicates(updates, n, err)) {
@@ -3762,6 +3784,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-refname);
goto cleanup;
}
+ if (!(flags REF_HAVE_NEW) ||
+ is_null_sha1(update-new_sha1) ||
+ remaining_fds == 0)
+ close_lock_file(update-lock-lk);
+ else
+ remaining_fds--;
}
/* Perform updates first so live commits remain referenced */
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7a69f1a..636d3a1 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -1071,7 +1071,7 @@ run_with_limited_open_files () {
test_lazy_prereq ULIMIT_FILE_DESCRIPTORS 'run_with_limited_open_files true'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
+test_expect_success ULIMIT_FILE_DESCRIPTORS 'large transaction creating
branches does not burst open file limit' '
(
for i in $(test_seq 33)
do
@@ -1082,7 +1082,7 @@ test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large
transaction creating branches
)
'
-test_expect_failure ULIMIT_FILE_DESCRIPTORS 'large transaction deleting
branches does not burst open file limit' '
+test_expect_success
[PATCH] ignore: info/exclude should trump core.excludesfile
$GIT_DIR/info/exclude and core.excludesfile (which falls back to
$XDG_HOME/git/ignore) are both ways to override the ignore pattern
lists given by the project in .gitignore files. The former, which
is per-repository personal preference, should take precedence over
the latter, which is a personal preference default across different
repositories that are accessed from that machine. The existing
documentation also agrees.
However, the precedence order was screwed up between these two from
the very beginning when 896bdfa2 (add: Support specifying an
excludes file with a configuration variable, 2007-02-27) introduced
core.excludesfile variable.
Noticed-by: Yohei Endo yoh...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* This is done on an old codebase and may not apply cleanly to more
modern codebase easily.
dir.c | 10 +++---
t/t0008-ignores.sh | 10 ++
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/dir.c b/dir.c
index 23b6de4..e67b6f9 100644
--- a/dir.c
+++ b/dir.c
@@ -1530,15 +1530,19 @@ void setup_standard_excludes(struct dir_struct *dir)
char *xdg_path;
dir-exclude_per_dir = .gitignore;
- path = git_path(info/exclude);
+
+ /* core.excludefile defaulting to $XDG_HOME/git/ignore */
if (!excludes_file) {
home_config_paths(NULL, xdg_path, ignore);
excludes_file = xdg_path;
}
- if (!access_or_warn(path, R_OK, 0))
- add_excludes_from_file(dir, path);
if (excludes_file !access_or_warn(excludes_file, R_OK, 0))
add_excludes_from_file(dir, excludes_file);
+
+ /* per repository user preference */
+ path = git_path(info/exclude);
+ if (!access_or_warn(path, R_OK, 0))
+ add_excludes_from_file(dir, path);
}
int remove_path(const char *name)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index b4d98e6..38405de 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -775,4 +775,14 @@ test_expect_success PIPE 'streaming support for --stdin' '
echo $response | grep ^::two
'
+test_expect_success 'info/exclude trumps core.excludesfile' '
+ echo global-excludes usually-ignored
+ echo .git/info/exclude !usually-ignored
+ usually-ignored
+ echo ?? usually-ignored expect
+
+ git status --porcelain usually-ignored actual
+ test_cmp expect actual
+'
+
test_done
--
2.4.0-rc3-227-gd45ce82
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Performance regression due to #33d4221: write_sha1_file: freshen existing objects
Stefan Saasen ssaa...@atlassian.com writes: Anyway, long story short. We're interested to help but I'm not entirely sure what that would look like at the moment. Are there formed ideas floating around or would you be looking for some form of proposal instead? I am not proposing anything or looking for proposals myself, actually. It is just somebody expressed interest in having tested older maintenance track that is kept alive in the past, so I was merely trying to help connect you with that old thread. If those who are interested in having such LTS track(s) need something specific from me, and if it will not be unrealistic maintenance burden, I am willing to help. That's all. For example, LTS group for whatever reason may nominate 2.2.x track as a base that they want to keep alive longer than other maintenance tracks and promise to test changes to them to keep it stable. Then I can help the effort by making sure people's bugfix patches would apply down to 2.2.x track (often people make mistake of using newer facility to fix or test the fix for an ancient bug, and bugfix topic branch ends up forked at a point much newer than where it should be). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On Wed, Apr 22, 2015 at 06:00:54PM -0400, Jeff King wrote:
Yeah, that looks right to me. You might want to represent the are we
tortoise check as a separate flag, though, and reuse it a few lines
later.
Sounds like a good idea. I'll send a more formal patch a bit later
today.
Also, not related to your patch, but I notice the putty declaration is
in a different scope than I would have expected, which made me wonder if
it gets initialized in all code paths. I think is from the recent
addition of CONNECT_DIAG_URL, which pushes the bulk of the code into its
own else clause, even though the first part of the if always returns
early. I wonder if it would be simpler to read like:
diff --git a/connect.c b/connect.c
index 391d211..749a07b 100644
--- a/connect.c
+++ b/connect.c
@@ -743,28 +743,28 @@ struct child_process *git_connect(int fd[2], const char
*url,
free(path);
free(conn);
return NULL;
- } else {
- ssh = getenv(GIT_SSH_COMMAND);
- if (ssh) {
- conn-use_shell = 1;
- putty = 0;
- } else {
- ssh = getenv(GIT_SSH);
- if (!ssh)
- ssh = ssh;
- putty = !!strcasestr(ssh, plink);
- }
-
- argv_array_push(conn-args, ssh);
- if (putty !strcasestr(ssh, tortoiseplink))
- argv_array_push(conn-args, -batch);
- if (port) {
- /* P is for PuTTY, p is for OpenSSH */
- argv_array_push(conn-args, putty ?
-P : -p);
- argv_array_push(conn-args, port);
- }
- argv_array_push(conn-args, ssh_host);
}
+
+ ssh = getenv(GIT_SSH_COMMAND);
+ if (ssh) {
+ conn-use_shell = 1;
+ putty = 0;
+ } else {
+ ssh = getenv(GIT_SSH);
+ if (!ssh)
+ ssh = ssh;
+ putty = !!strcasestr(ssh, plink);
+ }
+
+ argv_array_push(conn-args, ssh);
+ if (putty !strcasestr(ssh, tortoiseplink))
+ argv_array_push(conn-args, -batch);
+ if (port) {
+ /* P is for PuTTY, p is for OpenSSH */
+ argv_array_push(conn-args, putty ? -P :
-p);
+ argv_array_push(conn-args, port);
+ }
+ argv_array_push(conn-args, ssh_host);
} else {
/* remove repo-local variables from the environment */
conn-env = local_repo_env;
I can drop this in as a preparatory patch if I can have your sign-off.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
signature.asc
Description: Digital signature
[ANNOUNCE] Git v2.4.0-rc3
A release candidate Git v2.4.0-rc3 is now available for testing at the usual places. This hopefully will be the last -rc before the 2.4 final. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/testing/ The following public repositories all have a copy of the 'v2.4.0-rc3' tag and the 'master' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git 2.4 Release Notes (draft) = Backward compatibility warning(s) - This release has a few changes in the user-visible output from Porcelain commands. These are not meant to be parsed by scripts, but the users still may want to be aware of the changes: * Output from git log --decorate (and %d format specifier used in the userformat --format=string parameter git log family of command takes) used to list HEAD just like other tips of branch names, separated with a comma in between. E.g. $ git log --decorate -1 master commit bdb0f6788fa5e3cacc4315e9ff318a27b2676ff4 (HEAD, master) ... This release updates the output slightly when HEAD refers to the tip of a branch whose name is also shown in the output. The above is shown as: $ git log --decorate -1 master commit bdb0f6788fa5e3cacc4315e9ff318a27b2676ff4 (HEAD - master) ... * The phrasing git branch uses to describe a detached HEAD has been updated to match that of git status: - When the HEAD is at the same commit as it was originally detached, they now both show detached at commit object name. - When the HEAD has moved since it was originally detached, they now both show detached from commit object name. Earlier git branch always used from Updates since v2.3 -- Ports * Our default I/O size (8 MiB) for large files was too large for some platforms with smaller SSIZE_MAX, leading to read(2)/write(2) failures. * We did not check the curl library version before using CURLOPT_PROXYAUTH feature that may not exist. * We now detect number of CPUs on older BSD-derived systems. * Portability fixes and workarounds for shell scripts have been added to help BSD-derived systems. UI, Workflows Features * The command usage info strings given by git cmd -h and in documentation have been tweaked for consistency. * The sync subcommand of git p4 now allows users to exclude subdirectories like its clone subcommand does. * git log --invert-grep --grep=WIP will show only commits that do not have the string WIP in their messages. * git push has been taught a --atomic option that makes push to update more than one ref an all-or-none affair. * Extending the push to deploy added in 2.3, the behaviour of git push when updating the branch that is checked out can now be tweaked by push-to-checkout hook. * Using environment variable LANGUAGE and friends on the client side, HTTP-based transports now send Accept-Language when making requests. * git send-email used to accept a mistaken y (or yes) as an answer to What encoding do you want to use [UTF-8]? without questioning. Now it asks for confirmation when the answer looks too short to be a valid encoding name. * When git apply --whitespace=fix fixed whitespace errors in the common context lines, the command reports that it did so. * git status now allows the -v to be given twice to show the differences that are left in the working tree not to be committed. * git cherry-pick used to clean-up the log message even when it is merely replaying an existing commit. It now replays the message verbatim unless you are editing the message of resulting commits. * git archive can now be told to set the 'text' attribute in the resulting zip archive. * Output from git log --decorate mentions HEAD when it points at a tip of an branch differently from a detached HEAD. This is a potentially backward-incompatible change. * git branch on a detached HEAD always said (detached from xyz), even when git status would report detached at xyz. The HEAD is actually at xyz and haven't been moved since it was detached in such a case, but the user cannot read what the current value of HEAD is when detached from is used. * git -C '' subcmd refused to work in the current directory, unlike cd '' which silently behaves as a no-op. (merge 6a536e2 kn/git-cd-to-empty later to maint). * The versionsort.prerelease configuration variable can be used to specify that v1.0-pre1 comes before v1.0. * A new push.followTags configuration turns the --follow-tags option on by default
[PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id
To allow piecemeal conversion of the for_each_*_ref functions, introduce
an additional typedef for a callback function that takes struct
object_id * instead of unsigned char *. Provide an extra field in
struct ref_entry_cb for this callback and ensure at most one is set at a
time. Temporarily suffix these new entries with _oid to distinguish
them. Convert for_each_tag_ref and its callers to use the new _oid
functions, introducing temporary wrapper functions to avoid type
mismatches.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/pack-objects.c | 4 ++--
builtin/rev-parse.c| 7 ++-
builtin/tag.c | 8
refs.c | 34 ++
refs.h | 10 +-
5 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c067107..0c69b0c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -540,11 +540,11 @@ static enum write_one_status write_one(struct sha1file *f,
return WRITE_ONE_WRITTEN;
}
-static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
+static int mark_tagged(const char *path, const struct object_id *oid, int flag,
void *cb_data)
{
unsigned char peeled[20];
- struct object_entry *entry = packlist_find(to_pack, sha1, NULL);
+ struct object_entry *entry = packlist_find(to_pack, oid-hash, NULL);
if (entry)
entry-tagged = 1;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 4d10dd9..7b70650 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -198,6 +198,11 @@ static int show_reference(const char *refname, const
unsigned char *sha1, int fl
return 0;
}
+static int show_reference_oid(const char *refname, const struct object_id
*oid, int flag, void *cb_data)
+{
+ return show_reference(refname, oid-hash, flag, cb_data);
+}
+
static int anti_reference(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
{
show_rev(REVERSED, sha1, refname);
@@ -682,7 +687,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, --tags)) {
- for_each_tag_ref(show_reference, NULL);
+ for_each_tag_ref(show_reference_oid, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 6f07ac6..61399b7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -215,7 +215,7 @@ free_return:
free(buf);
}
-static int show_reference(const char *refname, const unsigned char *sha1,
+static int show_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
struct tag_filter *filter = cb_data;
@@ -224,14 +224,14 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
if (filter-with_commit) {
struct commit *commit;
- commit = lookup_commit_reference_gently(sha1, 1);
+ commit = lookup_commit_reference_gently(oid-hash, 1);
if (!commit)
return 0;
if (!contains(commit, filter-with_commit))
return 0;
}
- if (points_at.nr !match_points_at(refname, sha1))
+ if (points_at.nr !match_points_at(refname, oid-hash))
return 0;
if (!filter-lines) {
@@ -242,7 +242,7 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
return 0;
}
printf(%-15s , refname);
- show_tag_lines(sha1, filter-lines);
+ show_tag_lines(oid-hash, filter-lines);
putchar('\n');
}
diff --git a/refs.c b/refs.c
index 522d15d..95863f2 100644
--- a/refs.c
+++ b/refs.c
@@ -694,6 +694,7 @@ struct ref_entry_cb {
int trim;
int flags;
each_ref_fn *fn;
+ each_ref_fn_oid *fn_oid;
void *cb_data;
};
@@ -717,8 +718,13 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
- retval = data-fn(entry-name + data-trim, entry-u.value.oid.hash,
- entry-flag, data-cb_data);
+ if (data-fn_oid) {
+ retval = data-fn_oid(entry-name + data-trim,
entry-u.value.oid,
+entry-flag, data-cb_data);
+ } else {
+ retval = data-fn(entry-name + data-trim,
entry-u.value.oid.hash,
+
[PATCH v2 01/16] refs: convert struct ref_entry to use struct object_id
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
refs.c | 44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/refs.c b/refs.c
index 81a455b..522d15d 100644
--- a/refs.c
+++ b/refs.c
@@ -156,7 +156,7 @@ struct ref_value {
* null. If REF_ISSYMREF, then this is the name of the object
* referred to by the last reference in the symlink chain.
*/
- unsigned char sha1[20];
+ struct object_id oid;
/*
* If REF_KNOWS_PEELED, then this field holds the peeled value
@@ -164,7 +164,7 @@ struct ref_value {
* be peelable. See the documentation for peel_ref() for an
* exact definition of peelable.
*/
- unsigned char peeled[20];
+ struct object_id peeled;
};
struct ref_cache;
@@ -346,8 +346,8 @@ static struct ref_entry *create_ref_entry(const char
*refname,
die(Reference has invalid format: '%s', refname);
len = strlen(refname) + 1;
ref = xmalloc(sizeof(struct ref_entry) + len);
- hashcpy(ref-u.value.sha1, sha1);
- hashclr(ref-u.value.peeled);
+ hashcpy(ref-u.value.oid.hash, sha1);
+ oidclr(ref-u.value.peeled);
memcpy(ref-name, refname, len);
ref-flag = flag;
return ref;
@@ -621,7 +621,7 @@ static int is_dup_ref(const struct ref_entry *ref1, const
struct ref_entry *ref2
/* This is impossible by construction */
die(Reference directory conflict: %s, ref1-name);
- if (hashcmp(ref1-u.value.sha1, ref2-u.value.sha1))
+ if (oidcmp(ref1-u.value.oid, ref2-u.value.oid))
die(Duplicated ref, and SHA1s don't match: %s, ref1-name);
warning(Duplicated ref: %s, ref1-name);
@@ -669,7 +669,7 @@ static int ref_resolves_to_object(struct ref_entry *entry)
{
if (entry-flag REF_ISBROKEN)
return 0;
- if (!has_sha1_file(entry-u.value.sha1)) {
+ if (!has_sha1_file(entry-u.value.oid.hash)) {
error(%s does not point to a valid object!, entry-name);
return 0;
}
@@ -717,7 +717,7 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
- retval = data-fn(entry-name + data-trim, entry-u.value.sha1,
+ retval = data-fn(entry-name + data-trim, entry-u.value.oid.hash,
entry-flag, data-cb_data);
current_ref = old_current_ref;
return retval;
@@ -1193,7 +1193,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
line.len == PEELED_LINE_LENGTH
line.buf[PEELED_LINE_LENGTH - 1] == '\n'
!get_sha1_hex(line.buf + 1, sha1)) {
- hashcpy(last-u.value.peeled, sha1);
+ hashcpy(last-u.value.peeled.hash, sha1);
/*
* Regardless of what the file header said,
* we definitely know the value of *this*
@@ -1374,7 +1374,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache
*refs,
if (ref == NULL)
return -1;
- hashcpy(sha1, ref-u.value.sha1);
+ hashcpy(sha1, ref-u.value.oid.hash);
return 0;
}
@@ -1461,7 +1461,7 @@ static int resolve_missing_loose_ref(const char *refname,
*/
entry = get_packed_ref(refname);
if (entry) {
- hashcpy(sha1, entry-u.value.sha1);
+ hashcpy(sha1, entry-u.value.oid.hash);
if (flags)
*flags |= REF_ISPACKED;
return 0;
@@ -1771,9 +1771,9 @@ static enum peel_status peel_entry(struct ref_entry
*entry, int repeel)
if (entry-flag REF_KNOWS_PEELED) {
if (repeel) {
entry-flag = ~REF_KNOWS_PEELED;
- hashclr(entry-u.value.peeled);
+ oidclr(entry-u.value.peeled);
} else {
- return is_null_sha1(entry-u.value.peeled) ?
+ return is_null_oid(entry-u.value.peeled) ?
PEEL_NON_TAG : PEEL_PEELED;
}
}
@@ -1782,7 +1782,7 @@ static enum peel_status peel_entry(struct ref_entry
*entry, int repeel)
if (entry-flag REF_ISSYMREF)
return PEEL_IS_SYMREF;
- status = peel_object(entry-u.value.sha1, entry-u.value.peeled);
+ status = peel_object(entry-u.value.oid.hash,
entry-u.value.peeled.hash);
if (status == PEEL_PEELED || status == PEEL_NON_TAG)
entry-flag |= REF_KNOWS_PEELED;
return status;
@@ -1797,7 +1797,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
|| !strcmp(current_ref-name, refname))) {
if
[PATCH v2 07/16] revision: remove unused _oid helper.
Now that all the callers of handle_refs are gone, rename handle_refs_oid
to handle_refs and update the callers accordingly.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
revision.c | 22 +++---
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/revision.c b/revision.c
index 825fbba..6fb499f 100644
--- a/revision.c
+++ b/revision.c
@@ -1264,14 +1264,6 @@ void add_ref_exclusion(struct string_list
**ref_excludes_p, const char *exclude)
}
static void handle_refs(const char *submodule, struct rev_info *revs, unsigned
flags,
- int (*for_each)(const char *, each_ref_fn, void *))
-{
- struct all_refs_cb cb;
- init_all_refs_cb(cb, revs, flags);
- for_each(submodule, handle_one_ref, cb);
-}
-
-static void handle_refs_oid(const char *submodule, struct rev_info *revs,
unsigned flags,
int (*for_each)(const char *, each_ref_fn_oid, void *))
{
struct all_refs_cb cb;
@@ -2116,21 +2108,21 @@ static int handle_revision_pseudo_opt(const char
*submodule,
* register it in the list at the top of handle_revision_opt.
*/
if (!strcmp(arg, --all)) {
- handle_refs_oid(submodule, revs, *flags,
for_each_ref_submodule);
- handle_refs_oid(submodule, revs, *flags, head_ref_submodule);
+ handle_refs(submodule, revs, *flags, for_each_ref_submodule);
+ handle_refs(submodule, revs, *flags, head_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --branches)) {
- handle_refs_oid(submodule, revs, *flags,
for_each_branch_ref_submodule);
+ handle_refs(submodule, revs, *flags,
for_each_branch_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --bisect)) {
- handle_refs_oid(submodule, revs, *flags,
for_each_bad_bisect_ref);
- handle_refs_oid(submodule, revs, *flags ^ (UNINTERESTING |
BOTTOM), for_each_good_bisect_ref);
+ handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
+ handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM),
for_each_good_bisect_ref);
revs-bisect = 1;
} else if (!strcmp(arg, --tags)) {
- handle_refs_oid(submodule, revs, *flags,
for_each_tag_ref_submodule);
+ handle_refs(submodule, revs, *flags,
for_each_tag_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --remotes)) {
- handle_refs_oid(submodule, revs, *flags,
for_each_remote_ref_submodule);
+ handle_refs(submodule, revs, *flags,
for_each_remote_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if ((argcount = parse_long_opt(glob, argv, optarg))) {
struct all_refs_cb cb;
--
2.3.5
--
To unsubscribe from this list: send the line 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 03/16] refs: convert remaining users of for_each_ref_in to object_id
Remove the temporary for_each_ref_in_oid function and update the users
of it. Convert the users of for_each_branch_ref and
for_each_remote_ref (which use for_each_ref_in under the hood) as well.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
bisect.c| 8
builtin/rev-parse.c | 10 +-
refs.c | 13 -
refs.h | 6 +++---
4 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/bisect.c b/bisect.c
index 10f5e57..03d5cd9 100644
--- a/bisect.c
+++ b/bisect.c
@@ -400,16 +400,16 @@ struct commit_list *find_bisection(struct commit_list
*list,
return best;
}
-static int register_ref(const char *refname, const unsigned char *sha1,
+static int register_ref(const char *refname, const struct object_id *oid,
int flags, void *cb_data)
{
if (!strcmp(refname, bad)) {
current_bad_oid = xmalloc(sizeof(*current_bad_oid));
- hashcpy(current_bad_oid-hash, sha1);
+ oidcpy(current_bad_oid, oid);
} else if (starts_with(refname, good-)) {
- sha1_array_append(good_revs, sha1);
+ sha1_array_append(good_revs, oid-hash);
} else if (starts_with(refname, skip-)) {
- sha1_array_append(skipped_revs, sha1);
+ sha1_array_append(skipped_revs, oid-hash);
}
return 0;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 7b70650..c9f2c93 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -203,9 +203,9 @@ static int show_reference_oid(const char *refname, const
struct object_id *oid,
return show_reference(refname, oid-hash, flag, cb_data);
}
-static int anti_reference(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+static int anti_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
- show_rev(REVERSED, sha1, refname);
+ show_rev(REVERSED, oid-hash, refname);
return 0;
}
@@ -665,7 +665,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, --bisect)) {
- for_each_ref_in(refs/bisect/bad,
show_reference, NULL);
+ for_each_ref_in(refs/bisect/bad,
show_reference_oid, NULL);
for_each_ref_in(refs/bisect/good,
anti_reference, NULL);
continue;
}
@@ -676,7 +676,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, --branches)) {
- for_each_branch_ref(show_reference, NULL);
+ for_each_branch_ref(show_reference_oid, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
@@ -703,7 +703,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, --remotes)) {
- for_each_remote_ref(show_reference, NULL);
+ for_each_remote_ref(show_reference_oid, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
diff --git a/refs.c b/refs.c
index 95863f2..a81c301 100644
--- a/refs.c
+++ b/refs.c
@@ -2019,16 +2019,11 @@ int for_each_ref_submodule(const char *submodule,
each_ref_fn fn, void *cb_data)
return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
}
-static int for_each_ref_in_oid(const char *prefix, each_ref_fn_oid fn, void
*cb_data)
+int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
{
return do_for_each_ref_oid(ref_cache, prefix, fn, strlen(prefix), 0,
cb_data);
}
-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
-{
- return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0,
cb_data);
-}
-
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn fn, void *cb_data)
{
@@ -2037,7 +2032,7 @@ int for_each_ref_in_submodule(const char *submodule,
const char *prefix,
int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
{
- return for_each_ref_in_oid(refs/tags/, fn, cb_data);
+ return for_each_ref_in(refs/tags/, fn, cb_data);
}
int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
@@ -2045,7 +2040,7 @@ int for_each_tag_ref_submodule(const char *submodule,
each_ref_fn fn, void *cb_d
return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
}
-int
[PATCH v2 10/16] refs: convert namespaced ref iteration functions to object_id
Convert head_ref_namespaced and for_each_namespaced_ref to use struct
object_id. Update the various callbacks to use struct object_id
internally as well.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
http-backend.c | 14 +++---
refs.c | 12 ++--
refs.h | 4 ++--
upload-pack.c | 30 +++---
4 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index b6c0484..e0d6627 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -350,16 +350,16 @@ static void run_service(const char **argv)
exit(1);
}
-static int show_text_ref(const char *name, const unsigned char *sha1,
+static int show_text_ref(const char *name, const struct object_id *oid,
int flag, void *cb_data)
{
const char *name_nons = strip_namespace(name);
struct strbuf *buf = cb_data;
- struct object *o = parse_object(sha1);
+ struct object *o = parse_object(oid-hash);
if (!o)
return 0;
- strbuf_addf(buf, %s\t%s\n, sha1_to_hex(sha1), name_nons);
+ strbuf_addf(buf, %s\t%s\n, oid_to_hex(oid), name_nons);
if (o-type == OBJ_TAG) {
o = deref_tag(o, name, 0);
if (!o)
@@ -402,21 +402,21 @@ static void get_info_refs(char *arg)
strbuf_release(buf);
}
-static int show_head_ref(const char *refname, const unsigned char *sha1,
+static int show_head_ref(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
struct strbuf *buf = cb_data;
if (flag REF_ISSYMREF) {
- unsigned char unused[20];
+ struct object_id unused;
const char *target = resolve_ref_unsafe(refname,
RESOLVE_REF_READING,
- unused, NULL);
+ unused.hash, NULL);
const char *target_nons = strip_namespace(target);
strbuf_addf(buf, ref: %s\n, target_nons);
} else {
- strbuf_addf(buf, %s\n, sha1_to_hex(sha1));
+ strbuf_addf(buf, %s\n, oid_to_hex(oid));
}
return 0;
diff --git a/refs.c b/refs.c
index 89606a7..19ce65a 100644
--- a/refs.c
+++ b/refs.c
@@ -2065,27 +2065,27 @@ int for_each_replace_ref(each_ref_fn_oid fn, void
*cb_data)
return do_for_each_ref_oid(ref_cache, refs/replace/, fn, 13, 0,
cb_data);
}
-int head_ref_namespaced(each_ref_fn fn, void *cb_data)
+int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
{
struct strbuf buf = STRBUF_INIT;
int ret = 0;
- unsigned char sha1[20];
+ struct object_id oid;
int flag;
strbuf_addf(buf, %sHEAD, get_git_namespace());
- if (!read_ref_full(buf.buf, RESOLVE_REF_READING, sha1, flag))
- ret = fn(buf.buf, sha1, flag, cb_data);
+ if (!read_ref_full(buf.buf, RESOLVE_REF_READING, oid.hash, flag))
+ ret = fn(buf.buf, oid, flag, cb_data);
strbuf_release(buf);
return ret;
}
-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data)
{
struct strbuf buf = STRBUF_INIT;
int ret;
strbuf_addf(buf, %srefs/, get_git_namespace());
- ret = do_for_each_ref(ref_cache, buf.buf, fn, 0, 0, cb_data);
+ ret = do_for_each_ref_oid(ref_cache, buf.buf, fn, 0, 0, cb_data);
strbuf_release(buf);
return ret;
}
diff --git a/refs.h b/refs.h
index 9c6bc06..2b2a688 100644
--- a/refs.h
+++ b/refs.h
@@ -104,8 +104,8 @@ extern int for_each_tag_ref_submodule(const char
*submodule, each_ref_fn_oid fn,
extern int for_each_branch_ref_submodule(const char *submodule,
each_ref_fn_oid fn, void *cb_data);
extern int for_each_remote_ref_submodule(const char *submodule,
each_ref_fn_oid fn, void *cb_data);
-extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);
-extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);
+extern int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data);
+extern int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data);
static inline const char *has_glob_specials(const char *pattern)
{
diff --git a/upload-pack.c b/upload-pack.c
index 745fda8..b60086f 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -681,9 +681,9 @@ static void receive_needs(void)
}
/* return non-zero if the ref is hidden, otherwise 0 */
-static int mark_our_ref(const char *refname, const unsigned char *sha1)
+static int mark_our_ref(const char *refname, const struct object_id *oid)
{
- struct object *o = lookup_unknown_object(sha1);
+ struct object *o = lookup_unknown_object(oid-hash);
if (ref_is_hidden(refname)) {
o-flags |= HIDDEN_REF;
@@ -693,9 +693,9 @@ static int mark_our_ref(const char *refname, const
[PATCH v2 08/16] refs: convert for_each_ref to struct object_id
Convert for_each_ref, for_each_glob_ref, and for_each_glob_ref_in to use
struct object_id, as the latter two call the former with the function
pointer they are provided.
Convert callers to refer to properly-typed functions. Convert uses of
the constant 20 to GIT_SHA1_RAWSZ. Where possible, convert modified
functions to use struct object_id instead of unsigned char [20].
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/checkout.c | 4 ++--
builtin/fetch.c| 6 +++---
builtin/name-rev.c | 6 +++---
builtin/pack-objects.c | 10 +-
builtin/receive-pack.c | 4 ++--
builtin/reflog.c | 6 +++---
builtin/remote.c | 14 +++---
builtin/rev-parse.c| 10 +-
builtin/show-branch.c | 24
builtin/show-ref.c | 4 ++--
fetch-pack.c | 18 ++
help.c | 2 +-
log-tree.c | 2 +-
notes.c| 2 +-
reachable.c| 2 +-
refs.c | 16
refs.h | 6 +++---
remote.c | 8
revision.c | 8
server-info.c | 6 +++---
sha1_name.c| 4 ++--
shallow.c | 6 +++---
submodule.c| 4 ++--
transport.c| 10 +-
walker.c | 4 ++--
25 files changed, 98 insertions(+), 88 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2f92328..9b49f0e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -702,10 +702,10 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
}
static int add_pending_uninteresting_ref(const char *refname,
-const unsigned char *sha1,
+const struct object_id *oid,
int flags, void *cb_data)
{
- add_pending_sha1(cb_data, refname, sha1, UNINTERESTING);
+ add_pending_sha1(cb_data, refname, oid-hash, UNINTERESTING);
return 0;
}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7910419..2e792f3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -179,13 +179,13 @@ static void add_merge_config(struct ref **head,
}
}
-static int add_existing(const char *refname, const unsigned char *sha1,
+static int add_existing(const char *refname, const struct object_id *oid,
int flag, void *cbdata)
{
struct string_list *list = (struct string_list *)cbdata;
struct string_list_item *item = string_list_insert(list, refname);
- item-util = xmalloc(20);
- hashcpy(item-util, sha1);
+ item-util = xmalloc(GIT_SHA1_RAWSZ);
+ hashcpy(item-util, oid-hash);
return 0;
}
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 9736d44..248a3eb 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -138,9 +138,9 @@ static int tipcmp(const void *a_, const void *b_)
return hashcmp(a-sha1, b-sha1);
}
-static int name_ref(const char *path, const unsigned char *sha1, int flags,
void *cb_data)
+static int name_ref(const char *path, const struct object_id *oid, int flags,
void *cb_data)
{
- struct object *o = parse_object(sha1);
+ struct object *o = parse_object(oid-hash);
struct name_ref_data *data = cb_data;
int can_abbreviate_output = data-tags_only data-name_only;
int deref = 0;
@@ -160,7 +160,7 @@ static int name_ref(const char *path, const unsigned char
*sha1, int flags, void
}
}
- add_to_tip_table(sha1, path, can_abbreviate_output);
+ add_to_tip_table(oid-hash, path, can_abbreviate_output);
while (o o-type == OBJ_TAG) {
struct tag *t = (struct tag *) o;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0c69b0c..80fe8c7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2097,14 +2097,14 @@ static void ll_find_deltas(struct object_entry **list,
unsigned list_size,
#define ll_find_deltas(l, s, w, d, p) find_deltas(l, s, w, d, p)
#endif
-static int add_ref_tag(const char *path, const unsigned char *sha1, int flag,
void *cb_data)
+static int add_ref_tag(const char *path, const struct object_id *oid, int
flag, void *cb_data)
{
- unsigned char peeled[20];
+ struct object_id peeled;
if (starts_with(path, refs/tags/) /* is a tag? */
- !peel_ref(path, peeled) /* peelable? */
- packlist_find(to_pack, peeled, NULL)) /* object packed? */
- add_object_entry(sha1, OBJ_TAG, NULL, 0);
+ !peel_ref(path, peeled.hash) /* peelable? */
+ packlist_find(to_pack, peeled.hash, NULL)) /* object packed?
*/
+ add_object_entry(oid-hash, OBJ_TAG, NULL, 0);
return 0;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d2ec52b..85e60e2 100644
---
[PATCH v2 12/16] refs: rename do_for_each_ref_oid to do_for_each_ref
do_for_each_ref was unused due to previous patches, so rename
do_for_each_ref_oid to do_for_each_ref. Similarly, remove the unused fn
member from struct ref_entry in favor of renaming the fn_oid member.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
refs.c | 43 +++
1 file changed, 11 insertions(+), 32 deletions(-)
diff --git a/refs.c b/refs.c
index 185b40f..9f687e8 100644
--- a/refs.c
+++ b/refs.c
@@ -693,8 +693,7 @@ struct ref_entry_cb {
const char *base;
int trim;
int flags;
- each_ref_fn *fn;
- each_ref_fn_oid *fn_oid;
+ each_ref_fn_oid *fn;
void *cb_data;
};
@@ -718,13 +717,8 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
/* Store the old value, in case this is a recursive call: */
old_current_ref = current_ref;
current_ref = entry;
- if (data-fn_oid) {
- retval = data-fn_oid(entry-name + data-trim,
entry-u.value.oid,
-entry-flag, data-cb_data);
- } else {
- retval = data-fn(entry-name + data-trim,
entry-u.value.oid.hash,
-entry-flag, data-cb_data);
- }
+ retval = data-fn(entry-name + data-trim, entry-u.value.oid,
+entry-flag, data-cb_data);
current_ref = old_current_ref;
return retval;
}
@@ -1949,28 +1943,13 @@ static int do_for_each_entry(struct ref_cache *refs,
const char *base,
* 0.
*/
static int do_for_each_ref(struct ref_cache *refs, const char *base,
- each_ref_fn fn, int trim, int flags, void *cb_data)
-{
- struct ref_entry_cb data;
- data.base = base;
- data.trim = trim;
- data.flags = flags;
- data.fn = fn;
- data.fn_oid = NULL;
- data.cb_data = cb_data;
-
- return do_for_each_entry(refs, base, do_one_ref, data);
-}
-
-static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
each_ref_fn_oid fn, int trim, int flags, void
*cb_data)
{
struct ref_entry_cb data;
data.base = base;
data.trim = trim;
data.flags = flags;
- data.fn = NULL;
- data.fn_oid = fn;
+ data.fn = fn;
data.cb_data = cb_data;
if (ref_paranoia 0)
@@ -2011,23 +1990,23 @@ int head_ref_submodule(const char *submodule,
each_ref_fn_oid fn, void *cb_data)
int for_each_ref(each_ref_fn_oid fn, void *cb_data)
{
- return do_for_each_ref_oid(ref_cache, , fn, 0, 0, cb_data);
+ return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
}
int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
{
- return do_for_each_ref_oid(get_ref_cache(submodule), , fn, 0, 0,
cb_data);
+ return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
}
int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
{
- return do_for_each_ref_oid(ref_cache, prefix, fn, strlen(prefix), 0,
cb_data);
+ return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0,
cb_data);
}
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn_oid fn, void *cb_data)
{
- return do_for_each_ref_oid(get_ref_cache(submodule), prefix, fn,
strlen(prefix), 0, cb_data);
+ return do_for_each_ref(get_ref_cache(submodule), prefix, fn,
strlen(prefix), 0, cb_data);
}
int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
@@ -2062,7 +2041,7 @@ int for_each_remote_ref_submodule(const char *submodule,
each_ref_fn_oid fn, voi
int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
{
- return do_for_each_ref_oid(ref_cache, refs/replace/, fn, 13, 0,
cb_data);
+ return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
}
int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
@@ -2085,7 +2064,7 @@ int for_each_namespaced_ref(each_ref_fn_oid fn, void
*cb_data)
struct strbuf buf = STRBUF_INIT;
int ret;
strbuf_addf(buf, %srefs/, get_git_namespace());
- ret = do_for_each_ref_oid(ref_cache, buf.buf, fn, 0, 0, cb_data);
+ ret = do_for_each_ref(ref_cache, buf.buf, fn, 0, 0, cb_data);
strbuf_release(buf);
return ret;
}
@@ -2127,7 +2106,7 @@ int for_each_glob_ref(each_ref_fn_oid fn, const char
*pattern, void *cb_data)
int for_each_rawref(each_ref_fn_oid fn, void *cb_data)
{
- return do_for_each_ref_oid(ref_cache, , fn, 0,
+ return do_for_each_ref(ref_cache, , fn, 0,
DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
}
--
2.3.5
--
To unsubscribe from this list: send the line 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 15/16] Remove unneeded *_oid functions.
While these functions were needed during the intermediate steps of
converting for_each_ref and friends to struct object_id, there is no
longer any need to have these wrapper functions. Update each of the
functions that the wrapper functions call and remove the _oid wrapper
functions themselves.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/rev-parse.c | 27 +++
builtin/show-ref.c | 23 +--
log-tree.c | 19 +++
reachable.c | 13 -
shallow.c | 33 ++---
5 files changed, 41 insertions(+), 74 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 4aa7a25..b623239 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -190,19 +190,14 @@ static int show_default(void)
return 0;
}
-static int show_reference(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+static int show_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
if (ref_excluded(ref_excludes, refname))
return 0;
- show_rev(NORMAL, sha1, refname);
+ show_rev(NORMAL, oid-hash, refname);
return 0;
}
-static int show_reference_oid(const char *refname, const struct object_id
*oid, int flag, void *cb_data)
-{
- return show_reference(refname, oid-hash, flag, cb_data);
-}
-
static int anti_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
show_rev(REVERSED, oid-hash, refname);
@@ -657,7 +652,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, --all)) {
- for_each_ref(show_reference_oid, NULL);
+ for_each_ref(show_reference, NULL);
continue;
}
if (starts_with(arg, --disambiguate=)) {
@@ -665,45 +660,45 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
if (!strcmp(arg, --bisect)) {
- for_each_ref_in(refs/bisect/bad,
show_reference_oid, NULL);
+ for_each_ref_in(refs/bisect/bad,
show_reference, NULL);
for_each_ref_in(refs/bisect/good,
anti_reference, NULL);
continue;
}
if (starts_with(arg, --branches=)) {
- for_each_glob_ref_in(show_reference_oid, arg +
11,
+ for_each_glob_ref_in(show_reference, arg + 11,
refs/heads/, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (!strcmp(arg, --branches)) {
- for_each_branch_ref(show_reference_oid, NULL);
+ for_each_branch_ref(show_reference, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (starts_with(arg, --tags=)) {
- for_each_glob_ref_in(show_reference_oid, arg +
7,
+ for_each_glob_ref_in(show_reference, arg + 7,
refs/tags/, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (!strcmp(arg, --tags)) {
- for_each_tag_ref(show_reference_oid, NULL);
+ for_each_tag_ref(show_reference, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (starts_with(arg, --glob=)) {
- for_each_glob_ref(show_reference_oid, arg + 7,
NULL);
+ for_each_glob_ref(show_reference, arg + 7,
NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (starts_with(arg, --remotes=)) {
- for_each_glob_ref_in(show_reference_oid, arg +
10,
+ for_each_glob_ref_in(show_reference, arg + 10,
refs/remotes/, NULL);
clear_ref_exclusion(ref_excludes);
continue;
}
if (!strcmp(arg, --remotes)) {
-
[PATCH v2 09/16] refs: convert for_each_replace_ref to struct object_id
Update callbacks to take the proper parameters and use struct object_id
elsewhere in the callbacks.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/replace.c | 14 +++---
refs.c| 4 ++--
refs.h| 2 +-
replace_object.c | 4 ++--
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 54bf01a..a569420 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -35,7 +35,7 @@ struct show_data {
enum replace_format format;
};
-static int show_reference(const char *refname, const unsigned char *sha1,
+static int show_reference(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
struct show_data *data = cb_data;
@@ -44,19 +44,19 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
if (data-format == REPLACE_FORMAT_SHORT)
printf(%s\n, refname);
else if (data-format == REPLACE_FORMAT_MEDIUM)
- printf(%s - %s\n, refname, sha1_to_hex(sha1));
+ printf(%s - %s\n, refname, oid_to_hex(oid));
else { /* data-format == REPLACE_FORMAT_LONG */
- unsigned char object[20];
+ struct object_id object;
enum object_type obj_type, repl_type;
- if (get_sha1(refname, object))
+ if (get_sha1(refname, object.hash))
return error(Failed to resolve '%s' as a valid
ref., refname);
- obj_type = sha1_object_info(object, NULL);
- repl_type = sha1_object_info(sha1, NULL);
+ obj_type = sha1_object_info(object.hash, NULL);
+ repl_type = sha1_object_info(oid-hash, NULL);
printf(%s (%s) - %s (%s)\n, refname,
typename(obj_type),
- sha1_to_hex(sha1), typename(repl_type));
+ oid_to_hex(oid), typename(repl_type));
}
}
diff --git a/refs.c b/refs.c
index 6bf7abc..89606a7 100644
--- a/refs.c
+++ b/refs.c
@@ -2060,9 +2060,9 @@ int for_each_remote_ref_submodule(const char *submodule,
each_ref_fn_oid fn, voi
return for_each_ref_in_submodule(submodule, refs/remotes/, fn,
cb_data);
}
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
{
- return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
+ return do_for_each_ref_oid(ref_cache, refs/replace/, fn, 13, 0,
cb_data);
}
int head_ref_namespaced(each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index c041cd1..9c6bc06 100644
--- a/refs.h
+++ b/refs.h
@@ -92,7 +92,7 @@ extern int for_each_ref_in(const char *, each_ref_fn_oid,
void *);
extern int for_each_tag_ref(each_ref_fn_oid, void *);
extern int for_each_branch_ref(each_ref_fn_oid, void *);
extern int for_each_remote_ref(each_ref_fn_oid, void *);
-extern int for_each_replace_ref(each_ref_fn, void *);
+extern int for_each_replace_ref(each_ref_fn_oid, void *);
extern int for_each_glob_ref(each_ref_fn_oid, const char *pattern, void *);
extern int for_each_glob_ref_in(each_ref_fn_oid, const char *pattern, const
char* prefix, void *);
diff --git a/replace_object.c b/replace_object.c
index 0ab2dc1..f0b39f0 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -53,7 +53,7 @@ static int register_replace_object(struct replace_object
*replace,
}
static int register_replace_ref(const char *refname,
- const unsigned char *sha1,
+ const struct object_id *oid,
int flag, void *cb_data)
{
/* Get sha1 from refname */
@@ -68,7 +68,7 @@ static int register_replace_ref(const char *refname,
}
/* Copy sha1 from the read ref */
- hashcpy(repl_obj-replacement, sha1);
+ hashcpy(repl_obj-replacement, oid-hash);
/* Register new object */
if (register_replace_object(repl_obj, 1))
--
2.3.5
--
To unsubscribe from this list: send the line 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 05/16] refs: convert head_ref to struct object_id
Convert head_ref and head_ref_submodule to use struct object_id.
Introduce some wrappers in some of the callers to handle
incompatibilities between each_ref_fn and each_ref_fn_oid.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/show-ref.c | 7 ++-
log-tree.c | 7 ++-
reachable.c| 7 ++-
refs.c | 16
refs.h | 4 ++--
revision.c | 2 +-
shallow.c | 19 ---
7 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index afb1030..c6c5939 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -88,6 +88,11 @@ match:
return 0;
}
+static int show_ref_oid(const char *refname, const struct object_id *oid, int
flag, void *cbdata)
+{
+ return show_ref(refname, oid-hash, flag, cbdata);
+}
+
static int add_existing(const char *refname, const unsigned char *sha1, int
flag, void *cbdata)
{
struct string_list *list = (struct string_list *)cbdata;
@@ -225,7 +230,7 @@ int cmd_show_ref(int argc, const char **argv, const char
*prefix)
}
if (show_head)
- head_ref(show_ref, NULL);
+ head_ref(show_ref_oid, NULL);
for_each_ref(show_ref, NULL);
if (!found_match) {
if (verify !quiet)
diff --git a/log-tree.c b/log-tree.c
index cf4646b..a29c17e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -135,6 +135,11 @@ static int add_ref_decoration(const char *refname, const
unsigned char *sha1, in
return 0;
}
+static int add_ref_decoration_oid(const char *refname, const struct object_id
*oid, int flags, void *cb_data)
+{
+ return add_ref_decoration(refname, oid-hash, flags, cb_data);
+}
+
static int add_graft_decoration(const struct commit_graft *graft, void
*cb_data)
{
struct commit *commit = lookup_commit(graft-oid.hash);
@@ -150,7 +155,7 @@ void load_ref_decorations(int flags)
if (!loaded) {
loaded = 1;
for_each_ref(add_ref_decoration, flags);
- head_ref(add_ref_decoration, flags);
+ head_ref(add_ref_decoration_oid, flags);
for_each_commit_graft(add_graft_decoration, NULL);
}
}
diff --git a/reachable.c b/reachable.c
index 69fa685..110ce92 100644
--- a/reachable.c
+++ b/reachable.c
@@ -32,6 +32,11 @@ static int add_one_ref(const char *path, const unsigned char
*sha1, int flag, vo
return 0;
}
+static int add_one_ref_oid(const char *path, const struct object_id *oid, int
flag, void *cb_data)
+{
+ return add_one_ref(path, oid-hash, flag, cb_data);
+}
+
/*
* The traversal will have already marked us as SEEN, so we
* only need to handle any progress reporting here.
@@ -171,7 +176,7 @@ void mark_reachable_objects(struct rev_info *revs, int
mark_reflog,
for_each_ref(add_one_ref, revs);
/* detached HEAD is not included in the list above */
- head_ref(add_one_ref, revs);
+ head_ref(add_one_ref_oid, revs);
/* Add all reflog info */
if (mark_reflog)
diff --git a/refs.c b/refs.c
index aa13cc2..7a8b579 100644
--- a/refs.c
+++ b/refs.c
@@ -1981,30 +1981,30 @@ static int do_for_each_ref_oid(struct ref_cache *refs,
const char *base,
return do_for_each_entry(refs, base, do_one_ref, data);
}
-static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int do_head_ref(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
{
- unsigned char sha1[20];
+ struct object_id oid;
int flag;
if (submodule) {
- if (resolve_gitlink_ref(submodule, HEAD, sha1) == 0)
- return fn(HEAD, sha1, 0, cb_data);
+ if (resolve_gitlink_ref(submodule, HEAD, oid.hash) == 0)
+ return fn(HEAD, oid, 0, cb_data);
return 0;
}
- if (!read_ref_full(HEAD, RESOLVE_REF_READING, sha1, flag))
- return fn(HEAD, sha1, flag, cb_data);
+ if (!read_ref_full(HEAD, RESOLVE_REF_READING, oid.hash, flag))
+ return fn(HEAD, oid, flag, cb_data);
return 0;
}
-int head_ref(each_ref_fn fn, void *cb_data)
+int head_ref(each_ref_fn_oid fn, void *cb_data)
{
return do_head_ref(NULL, fn, cb_data);
}
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
{
return do_head_ref(submodule, fn, cb_data);
}
diff --git a/refs.h b/refs.h
index 62eb553..a14e7ef 100644
--- a/refs.h
+++ b/refs.h
@@ -86,7 +86,7 @@ typedef int each_ref_fn_oid(const char *refname,
* modifies the reference also returns a nonzero value to immediately
* stop the iteration.
*/
-extern int head_ref(each_ref_fn, void *);
+extern int head_ref(each_ref_fn_oid, void *);
extern int for_each_ref(each_ref_fn, void *);
extern
[PATCH v2 14/16] refs: rename each_ref_fn_oid to each_ref_fn
each_ref_fn is no longer used, so rename each_ref_fn_oid to each_ref_fn.
Update the documentation to note the change in function signature.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
Documentation/technical/api-ref-iteration.txt | 2 +-
refs.c| 48 +--
refs.h| 44 +++-
revision.c| 6 ++--
4 files changed, 47 insertions(+), 53 deletions(-)
diff --git a/Documentation/technical/api-ref-iteration.txt
b/Documentation/technical/api-ref-iteration.txt
index 02adfd4..37379d8 100644
--- a/Documentation/technical/api-ref-iteration.txt
+++ b/Documentation/technical/api-ref-iteration.txt
@@ -6,7 +6,7 @@ Iteration of refs is done by using an iterate function which
will call a
callback function for every ref. The callback function has this
signature:
- int handle_one_ref(const char *refname, const unsigned char *sha1,
+ int handle_one_ref(const char *refname, const struct object_id *oid,
int flags, void *cb_data);
There are different kinds of iterate functions which all take a
diff --git a/refs.c b/refs.c
index 38ecc2a..9e61b32 100644
--- a/refs.c
+++ b/refs.c
@@ -693,7 +693,7 @@ struct ref_entry_cb {
const char *base;
int trim;
int flags;
- each_ref_fn_oid *fn;
+ each_ref_fn *fn;
void *cb_data;
};
@@ -1669,7 +1669,7 @@ char *resolve_refdup(const char *ref, int resolve_flags,
unsigned char *sha1, in
/* The argument to filter_refs */
struct ref_filter {
const char *pattern;
- each_ref_fn_oid *fn;
+ each_ref_fn *fn;
void *cb_data;
};
@@ -1943,7 +1943,7 @@ static int do_for_each_entry(struct ref_cache *refs,
const char *base,
* 0.
*/
static int do_for_each_ref(struct ref_cache *refs, const char *base,
- each_ref_fn_oid fn, int trim, int flags, void
*cb_data)
+ each_ref_fn fn, int trim, int flags, void *cb_data)
{
struct ref_entry_cb data;
data.base = base;
@@ -1960,7 +1960,7 @@ static int do_for_each_ref(struct ref_cache *refs, const
char *base,
return do_for_each_entry(refs, base, do_one_ref, data);
}
-static int do_head_ref(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
+static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
{
struct object_id oid;
int flag;
@@ -1978,73 +1978,73 @@ static int do_head_ref(const char *submodule,
each_ref_fn_oid fn, void *cb_data)
return 0;
}
-int head_ref(each_ref_fn_oid fn, void *cb_data)
+int head_ref(each_ref_fn fn, void *cb_data)
{
return do_head_ref(NULL, fn, cb_data);
}
-int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
{
return do_head_ref(submodule, fn, cb_data);
}
-int for_each_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_ref(each_ref_fn fn, void *cb_data)
{
return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
}
-int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
{
return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
}
-int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
{
return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0,
cb_data);
}
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
- each_ref_fn_oid fn, void *cb_data)
+ each_ref_fn fn, void *cb_data)
{
return do_for_each_ref(get_ref_cache(submodule), prefix, fn,
strlen(prefix), 0, cb_data);
}
-int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_tag_ref(each_ref_fn fn, void *cb_data)
{
return for_each_ref_in(refs/tags/, fn, cb_data);
}
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
+int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
{
return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
}
-int for_each_branch_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_branch_ref(each_ref_fn fn, void *cb_data)
{
return for_each_ref_in(refs/heads/, fn, cb_data);
}
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn,
void *cb_data)
+int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
{
return for_each_ref_in_submodule(submodule, refs/heads/, fn, cb_data);
}
-int for_each_remote_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_remote_ref(each_ref_fn fn, void *cb_data)
{
[PATCH v2 13/16] refs: convert for_each_reflog to struct object_id
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/fsck.c | 2 +-
builtin/reflog.c | 4 ++--
refs.c | 10 +-
refs.h | 2 +-
revision.c | 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 85238a7..6659f81 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -476,7 +476,7 @@ static int fsck_handle_reflog_ent(unsigned char *osha1,
unsigned char *nsha1,
return 0;
}
-static int fsck_handle_reflog(const char *logname, const unsigned char *sha1,
int flag, void *cb_data)
+static int fsck_handle_reflog(const char *logname, const struct object_id
*oid, int flag, void *cb_data)
{
for_each_reflog_ent(logname, fsck_handle_reflog_ent, NULL);
return 0;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index f3f9201..da890f0 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -379,14 +379,14 @@ static void reflog_expiry_cleanup(void *cb_data)
}
}
-static int collect_reflog(const char *ref, const unsigned char *sha1, int
unused, void *cb_data)
+static int collect_reflog(const char *ref, const struct object_id *oid, int
unused, void *cb_data)
{
struct collected_reflog *e;
struct collect_reflog_cb *cb = cb_data;
size_t namelen = strlen(ref);
e = xmalloc(sizeof(*e) + namelen + 1);
- hashcpy(e-sha1, sha1);
+ hashcpy(e-sha1, oid-hash);
memcpy(e-reflog, ref, namelen + 1);
ALLOC_GROW(cb-e, cb-nr + 1, cb-alloc);
cb-e[cb-nr++] = e;
diff --git a/refs.c b/refs.c
index 9f687e8..38ecc2a 100644
--- a/refs.c
+++ b/refs.c
@@ -3484,7 +3484,7 @@ int for_each_reflog_ent(const char *refname,
each_reflog_ent_fn fn, void *cb_dat
* must be empty or end with '/'. Name will be used as a scratch
* space, but its contents will be restored before return.
*/
-static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void
*cb_data)
+static int do_for_each_reflog(struct strbuf *name, each_ref_fn_oid fn, void
*cb_data)
{
DIR *d = opendir(git_path(logs/%s, name-buf));
int retval = 0;
@@ -3509,11 +3509,11 @@ static int do_for_each_reflog(struct strbuf *name,
each_ref_fn fn, void *cb_data
strbuf_addch(name, '/');
retval = do_for_each_reflog(name, fn, cb_data);
} else {
- unsigned char sha1[20];
- if (read_ref_full(name-buf, 0, sha1, NULL))
+ struct object_id oid;
+ if (read_ref_full(name-buf, 0, oid.hash, NULL))
retval = error(bad ref for %s,
name-buf);
else
- retval = fn(name-buf, sha1, 0,
cb_data);
+ retval = fn(name-buf, oid, 0,
cb_data);
}
if (retval)
break;
@@ -3524,7 +3524,7 @@ static int do_for_each_reflog(struct strbuf *name,
each_ref_fn fn, void *cb_data
return retval;
}
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+int for_each_reflog(each_ref_fn_oid fn, void *cb_data)
{
int retval;
struct strbuf name;
diff --git a/refs.h b/refs.h
index 6df7d8a..abdfb00 100644
--- a/refs.h
+++ b/refs.h
@@ -222,7 +222,7 @@ int for_each_reflog_ent_reverse(const char *refname,
each_reflog_ent_fn fn, void
* Calls the specified function for each reflog file until it returns nonzero,
* and returns the value
*/
-extern int for_each_reflog(each_ref_fn, void *);
+extern int for_each_reflog(each_ref_fn_oid, void *);
#define REFNAME_ALLOW_ONELEVEL 1
#define REFNAME_REFSPEC_PATTERN 2
diff --git a/revision.c b/revision.c
index 29af759..94eb94b 100644
--- a/revision.c
+++ b/revision.c
@@ -1298,7 +1298,7 @@ static int handle_one_reflog_ent(unsigned char *osha1,
unsigned char *nsha1,
return 0;
}
-static int handle_one_reflog(const char *path, const unsigned char *sha1, int
flag, void *cb_data)
+static int handle_one_reflog(const char *path, const struct object_id *oid,
int flag, void *cb_data)
{
struct all_refs_cb *cb = cb_data;
cb-warned_bad_reflog = 0;
--
2.3.5
--
To unsubscribe from this list: send the line 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 11/16] refs: convert for_each_rawref to struct object_id.
Convert callbacks to use struct object_id internally as well.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
builtin/branch.c | 4 ++--
builtin/describe.c | 12 ++--
builtin/for-each-ref.c | 4 ++--
builtin/fsck.c | 16
refs.c | 10 +-
refs.h | 2 +-
6 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 258fe2f..ee3e909 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -328,7 +328,7 @@ static int match_patterns(const char **pattern, const char
*refname)
return 0;
}
-static int append_ref(const char *refname, const unsigned char *sha1, int
flags, void *cb_data)
+static int append_ref(const char *refname, const struct object_id *oid, int
flags, void *cb_data)
{
struct append_ref_cb *cb = (struct append_ref_cb *)(cb_data);
struct ref_list *ref_list = cb-ref_list;
@@ -365,7 +365,7 @@ static int append_ref(const char *refname, const unsigned
char *sha1, int flags,
commit = NULL;
if (ref_list-verbose || ref_list-with_commit || merge_filter !=
NO_FILTER) {
- commit = lookup_commit_reference_gently(sha1, 1);
+ commit = lookup_commit_reference_gently(oid-hash, 1);
if (!commit) {
cb-ret = error(_(branch '%s' does not point at a
commit), refname);
return 0;
diff --git a/builtin/describe.c b/builtin/describe.c
index e00a75b..a36c829 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -119,10 +119,10 @@ static void add_to_known_names(const char *path,
}
}
-static int get_name(const char *path, const unsigned char *sha1, int flag,
void *cb_data)
+static int get_name(const char *path, const struct object_id *oid, int flag,
void *cb_data)
{
int is_tag = starts_with(path, refs/tags/);
- unsigned char peeled[20];
+ struct object_id peeled;
int is_annotated, prio;
/* Reject anything outside refs/tags/ unless --all */
@@ -134,10 +134,10 @@ static int get_name(const char *path, const unsigned char
*sha1, int flag, void
return 0;
/* Is it annotated? */
- if (!peel_ref(path, peeled)) {
- is_annotated = !!hashcmp(sha1, peeled);
+ if (!peel_ref(path, peeled.hash)) {
+ is_annotated = !!oidcmp(oid, peeled);
} else {
- hashcpy(peeled, sha1);
+ oidcpy(peeled, oid);
is_annotated = 0;
}
@@ -154,7 +154,7 @@ static int get_name(const char *path, const unsigned char
*sha1, int flag, void
else
prio = 0;
- add_to_known_names(all ? path + 5 : path + 10, peeled, prio, sha1);
+ add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio,
oid-hash);
return 0;
}
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..50c3fbe 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -840,7 +840,7 @@ struct grab_ref_cbdata {
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+static int grab_single_ref(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
struct grab_ref_cbdata *cb = cb_data;
struct refinfo *ref;
@@ -878,7 +878,7 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
*/
ref = xcalloc(1, sizeof(*ref));
ref-refname = xstrdup(refname);
- hashcpy(ref-objectname, sha1);
+ hashcpy(ref-objectname, oid-hash);
ref-flag = flag;
cnt = cb-grab_cnt;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4783896..85238a7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -25,7 +25,7 @@ static int include_reflogs = 1;
static int check_full = 1;
static int check_strict;
static int keep_cache_objects;
-static unsigned char head_sha1[20];
+static struct object_id head_oid;
static const char *head_points_at;
static int errors_found;
static int write_lost_and_found;
@@ -482,13 +482,13 @@ static int fsck_handle_reflog(const char *logname, const
unsigned char *sha1, in
return 0;
}
-static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int
flag, void *cb_data)
+static int fsck_handle_ref(const char *refname, const struct object_id *oid,
int flag, void *cb_data)
{
struct object *obj;
- obj = parse_object(sha1);
+ obj = parse_object(oid-hash);
if (!obj) {
- error(%s: invalid sha1 pointer %s, refname,
sha1_to_hex(sha1));
+ error(%s: invalid sha1 pointer %s, refname, oid_to_hex(oid));
errors_found |= ERROR_REACHABLE;
/* We'll continue with the rest despite the error.. */
[PATCH v2 16/16] refs: convert struct ref_lock to struct object_id
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
refs.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/refs.c b/refs.c
index 9e61b32..6c04189 100644
--- a/refs.c
+++ b/refs.c
@@ -10,7 +10,7 @@ struct ref_lock {
char *ref_name;
char *orig_ref_name;
struct lock_file *lk;
- unsigned char old_sha1[20];
+ struct object_id old_oid;
int lock_fd;
};
@@ -2159,16 +2159,16 @@ static struct ref_lock *verify_lock(struct ref_lock
*lock,
{
if (read_ref_full(lock-ref_name,
mustexist ? RESOLVE_REF_READING : 0,
- lock-old_sha1, NULL)) {
+ lock-old_oid.hash, NULL)) {
int save_errno = errno;
error(Can't verify ref %s, lock-ref_name);
unlock_ref(lock);
errno = save_errno;
return NULL;
}
- if (hashcmp(lock-old_sha1, old_sha1)) {
+ if (hashcmp(lock-old_oid.hash, old_sha1)) {
error(Ref %s is at %s but expected %s, lock-ref_name,
- sha1_to_hex(lock-old_sha1), sha1_to_hex(old_sha1));
+ oid_to_hex(lock-old_oid), sha1_to_hex(old_sha1));
unlock_ref(lock);
errno = EBUSY;
return NULL;
@@ -2313,7 +2313,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
}
refname = resolve_ref_unsafe(refname, resolve_flags,
-lock-old_sha1, type);
+lock-old_oid.hash, type);
if (!refname errno == EISDIR) {
/* we are trying to lock foo but we used to
* have foo/bar which now does not exist;
@@ -2327,7 +2327,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
goto error_return;
}
refname = resolve_ref_unsafe(orig_refname, resolve_flags,
-lock-old_sha1, type);
+lock-old_oid.hash, type);
}
if (type_p)
*type_p = type;
@@ -2343,7 +2343,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* refname, nor a packed ref whose name is a proper prefix of
* our refname.
*/
- if (is_null_sha1(lock-old_sha1)
+ if (is_null_oid(lock-old_oid)
!is_refname_available(refname, skip, get_packed_refs(ref_cache)))
{
last_errno = ENOTDIR;
goto error_return;
@@ -2849,7 +2849,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
error(unable to lock %s for update, newrefname);
goto rollback;
}
- hashcpy(lock-old_sha1, orig_sha1);
+ hashcpy(lock-old_oid.hash, orig_sha1);
if (write_ref_sha1(lock, orig_sha1, logmsg)) {
error(unable to write current sha1 into %s, newrefname);
goto rollback;
@@ -3091,9 +3091,9 @@ static int write_ref_sha1(struct ref_lock *lock,
return -1;
}
clear_loose_ref_cache(ref_cache);
- if (log_ref_write(lock-ref_name, lock-old_sha1, sha1, logmsg) 0 ||
+ if (log_ref_write(lock-ref_name, lock-old_oid.hash, sha1, logmsg) 0
||
(strcmp(lock-ref_name, lock-orig_ref_name)
-log_ref_write(lock-orig_ref_name, lock-old_sha1, sha1, logmsg)
0)) {
+log_ref_write(lock-orig_ref_name, lock-old_oid.hash, sha1,
logmsg) 0)) {
unlock_ref(lock);
return -1;
}
@@ -3117,7 +3117,7 @@ static int write_ref_sha1(struct ref_lock *lock,
head_sha1, head_flag);
if (head_ref (head_flag REF_ISSYMREF)
!strcmp(head_ref, lock-ref_name))
- log_ref_write(HEAD, lock-old_sha1, sha1, logmsg);
+ log_ref_write(HEAD, lock-old_oid.hash, sha1, logmsg);
}
if (commit_ref(lock)) {
error(Couldn't set %s, lock-ref_name);
@@ -3814,7 +3814,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
(update-flags
REF_NODEREF));
if (!overwriting_symref
-!hashcmp(update-lock-old_sha1,
update-new_sha1)) {
+!hashcmp(update-lock-old_oid.hash,
update-new_sha1)) {
/*
* The reference already has the desired
* value, so we don't need to write it.
--
2.3.5
--
To unsubscribe from this list: send the line 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 04/16] refs: convert for_each_ref_in_submodule to object_id
Convert for_each_ref_in_submodule and all of its caller. Introduce two
temporary wrappers in revision.c to handle the incompatibilities between
each_ref_fn and each_ref_fn_oid.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
refs.c | 10 +-
refs.h | 8
revision.c | 28 +---
submodule.c | 2 +-
4 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/refs.c b/refs.c
index a81c301..aa13cc2 100644
--- a/refs.c
+++ b/refs.c
@@ -2025,9 +2025,9 @@ int for_each_ref_in(const char *prefix, each_ref_fn_oid
fn, void *cb_data)
}
int for_each_ref_in_submodule(const char *submodule, const char *prefix,
- each_ref_fn fn, void *cb_data)
+ each_ref_fn_oid fn, void *cb_data)
{
- return do_for_each_ref(get_ref_cache(submodule), prefix, fn,
strlen(prefix), 0, cb_data);
+ return do_for_each_ref_oid(get_ref_cache(submodule), prefix, fn,
strlen(prefix), 0, cb_data);
}
int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
@@ -2035,7 +2035,7 @@ int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
return for_each_ref_in(refs/tags/, fn, cb_data);
}
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
+int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
{
return for_each_ref_in_submodule(submodule, refs/tags/, fn, cb_data);
}
@@ -2045,7 +2045,7 @@ int for_each_branch_ref(each_ref_fn_oid fn, void *cb_data)
return for_each_ref_in(refs/heads/, fn, cb_data);
}
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
+int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn,
void *cb_data)
{
return for_each_ref_in_submodule(submodule, refs/heads/, fn, cb_data);
}
@@ -2055,7 +2055,7 @@ int for_each_remote_ref(each_ref_fn_oid fn, void *cb_data)
return for_each_ref_in(refs/remotes/, fn, cb_data);
}
-int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
+int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn,
void *cb_data)
{
return for_each_ref_in_submodule(submodule, refs/remotes/, fn,
cb_data);
}
diff --git a/refs.h b/refs.h
index d6ef917..62eb553 100644
--- a/refs.h
+++ b/refs.h
@@ -99,10 +99,10 @@ extern int for_each_glob_ref_in(each_ref_fn, const char
*pattern, const char* pr
extern int head_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data);
extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data);
extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
- each_ref_fn fn, void *cb_data);
-extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn,
void *cb_data);
-extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn
fn, void *cb_data);
-extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn
fn, void *cb_data);
+ each_ref_fn_oid fn, void *cb_data);
+extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid
fn, void *cb_data);
+extern int for_each_branch_ref_submodule(const char *submodule,
each_ref_fn_oid fn, void *cb_data);
+extern int for_each_remote_ref_submodule(const char *submodule,
each_ref_fn_oid fn, void *cb_data);
extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);
extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);
diff --git a/revision.c b/revision.c
index 7ddbaa0..93d9311 100644
--- a/revision.c
+++ b/revision.c
@@ -1232,6 +1232,12 @@ static int handle_one_ref(const char *path, const
unsigned char *sha1, int flag,
return 0;
}
+static int handle_one_ref_oid(const char *path, const struct object_id *oid,
+ int flag, void *cb_data)
+{
+ return handle_one_ref(path, oid-hash, flag, cb_data);
+}
+
static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
unsigned flags)
{
@@ -1265,6 +1271,14 @@ static void handle_refs(const char *submodule, struct
rev_info *revs, unsigned f
for_each(submodule, handle_one_ref, cb);
}
+static void handle_refs_oid(const char *submodule, struct rev_info *revs,
unsigned flags,
+ int (*for_each)(const char *, each_ref_fn_oid, void *))
+{
+ struct all_refs_cb cb;
+ init_all_refs_cb(cb, revs, flags);
+ for_each(submodule, handle_one_ref_oid, cb);
+}
+
static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
{
struct all_refs_cb *cb = cb_data;
@@ -2073,12 +2087,12 @@ void parse_revision_opt(struct rev_info *revs, struct
parse_opt_ctx_t *ctx,
ctx-argc -= n;
}
-static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void
*cb_data)
+static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn_oid fn,
void *cb_data)
{
return
[PATCH v2 00/16] Convert parts of refs.c to struct object_id
This is a conversion of parts of refs.c to use struct object_id. refs.c, and the for_each_ref series of functions explicitly, is the source for many instances of object IDs in the codebase. Therefore, it makes sense to convert this series of functions to provide a basis for further conversions. This series is essentially just for_each_ref and friends, the callbacks, and callers. Other parts of refs.c will be converted in a later series, so as to keep the number of patches to a reasonable size. There should be no functional change from this patch series. Changes from v1: * Rebase onto next to pick up the first set of object_id patches. * Fix a series of nasty conflicts that occurred due to other topics in flight to promote easier testing and integration. Comments and reviews on this series would be greatly appreciated. brian m. carlson (16): refs: convert struct ref_entry to use struct object_id refs: convert for_each_tag_ref to struct object_id refs: convert remaining users of for_each_ref_in to object_id refs: convert for_each_ref_in_submodule to object_id refs: convert head_ref to struct object_id refs: convert for_each_ref_submodule to struct object_id revision: remove unused _oid helper. refs: convert for_each_ref to struct object_id refs: convert for_each_replace_ref to struct object_id refs: convert namespaced ref iteration functions to object_id refs: convert for_each_rawref to struct object_id. refs: rename do_for_each_ref_oid to do_for_each_ref refs: convert for_each_reflog to struct object_id refs: rename each_ref_fn_oid to each_ref_fn Remove unneeded *_oid functions. refs: convert struct ref_lock to struct object_id Documentation/technical/api-ref-iteration.txt | 2 +- bisect.c | 8 +- builtin/branch.c | 4 +- builtin/checkout.c| 4 +- builtin/describe.c| 12 +-- builtin/fetch.c | 6 +- builtin/for-each-ref.c| 4 +- builtin/fsck.c| 18 ++--- builtin/name-rev.c| 6 +- builtin/pack-objects.c| 14 ++-- builtin/receive-pack.c| 4 +- builtin/reflog.c | 10 +-- builtin/remote.c | 14 ++-- builtin/replace.c | 14 ++-- builtin/rev-parse.c | 8 +- builtin/show-branch.c | 24 +++--- builtin/show-ref.c| 16 ++-- builtin/tag.c | 8 +- fetch-pack.c | 18 - help.c| 2 +- http-backend.c| 14 ++-- log-tree.c| 10 +-- notes.c | 2 +- reachable.c | 4 +- refs.c| 104 +- refs.h| 4 +- remote.c | 8 +- replace_object.c | 4 +- revision.c| 18 +++-- server-info.c | 6 +- sha1_name.c | 4 +- shallow.c | 8 +- submodule.c | 6 +- transport.c | 10 +-- upload-pack.c | 30 walker.c | 4 +- 36 files changed, 225 insertions(+), 207 deletions(-) -- 2.3.5 -- To unsubscribe from this list: send the line 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 06/16] refs: convert for_each_ref_submodule to struct object_id
Convert the callers as well.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
refs.c | 4 ++--
refs.h | 2 +-
revision.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 7a8b579..68d0af8 100644
--- a/refs.c
+++ b/refs.c
@@ -2014,9 +2014,9 @@ int for_each_ref(each_ref_fn fn, void *cb_data)
return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
}
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data)
+int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data)
{
- return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
+ return do_for_each_ref_oid(get_ref_cache(submodule), , fn, 0, 0,
cb_data);
}
int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
diff --git a/refs.h b/refs.h
index a14e7ef..2476951 100644
--- a/refs.h
+++ b/refs.h
@@ -97,7 +97,7 @@ extern int for_each_glob_ref(each_ref_fn, const char
*pattern, void *);
extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char*
prefix, void *);
extern int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void
*cb_data);
-extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void
*cb_data);
+extern int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn,
void *cb_data);
extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
each_ref_fn_oid fn, void *cb_data);
extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid
fn, void *cb_data);
diff --git a/revision.c b/revision.c
index 19005e8..825fbba 100644
--- a/revision.c
+++ b/revision.c
@@ -2116,7 +2116,7 @@ static int handle_revision_pseudo_opt(const char
*submodule,
* register it in the list at the top of handle_revision_opt.
*/
if (!strcmp(arg, --all)) {
- handle_refs(submodule, revs, *flags, for_each_ref_submodule);
+ handle_refs_oid(submodule, revs, *flags,
for_each_ref_submodule);
handle_refs_oid(submodule, revs, *flags, head_ref_submodule);
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --branches)) {
--
2.3.5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On 04/22/2015 09:12 PM, Patrick Sharp wrote: Johannes, You’re correct, looking back over it, I was pretty vague. In truth, we are not using Windows OR putty at all. Running git on an Ubuntu system, but we are setting the GIT_SSH environment variable to point to a shell script to use. Upon attempting to run git ls-remote, the system was spitting out getaddrinfo errors for ‘atch’ . Setting GIT_TRACE=2 showed that -batch was being added to the git command. This was seen on several different servers with git versions 1.8.5.2, 1.9.1 and 2.3.5 After a bit we realized that it was the string ‘uplink’ in the GIT_SSH variable that was linked to the extra -batch flag. Finally, after searching the git source, we narrowed it down to the ‘plink’ portion of the string. https://github.com/git/git/blob/7c597ef345aed345576de616c51f27e6f4b342b3/connect.c#L747-L7 Brian, I got your patch, but can't see it in the list yet 1/2 looks good, thanks. (And add msygit) My feeling is that patch 2/2 may break things for an unknown amount of users which e.g. use myplink. Patrick, did you ever tell us, what you put into $GIT_SSH ? Can your use case be covered by using $GIT_SSH_COMMAND ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
utf-8 filename woes
Hi, I have a git version 1.7.1 running on Scientific Linux 6. When a filename with UTF-8 is present in a tree-ish the git archive command crashes: %% *** buffer overflow detected ***: git terminated === Backtrace: = /lib64/libc.so.6(__fortify_fail+0x37)[0x32c1502527] /lib64/libc.so.6[0x32c1500410] /lib64/libc.so.6[0x32c14ff869] /lib64/libc.so.6(_IO_default_xsputn+0xc9)[0x32c1474639] /lib64/libc.so.6(_IO_vfprintf+0x11d8)[0x32c14451a8] /lib64/libc.so.6(__vsprintf_chk+0x9d)[0x32c14ff90d] /lib64/libc.so.6(__sprintf_chk+0x7f)[0x32c14ff84f] git[0x4620bc] git[0x4616bc] git[0x4bd832] git[0x4bda79] git[0x4bda79] git[0x4613e1] git[0x4623ed] git[0x4612ed] git[0x40caf3] git[0x4041a1] git[0x404382] /lib64/libc.so.6(__libc_start_main+0xfd)[0x32c141ed5d] git[0x403ba9] === Memory map: 0040-0051 r-xp fd:00 4121 /usr/bin/git %% Installing the git-debuginfo package gave no additional information. The symptom does not show on a Fedora 21 system with git 2.1.0 (and I note that gitk properly shows those filenames on f21, and does not on sl6). Is this a known and fixed issue, or something my sl6 system needs installing? -- Thanks, Jeremy -- To unsubscribe from this list: send the line 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] test-lib: turn on GIT_TEST_CHAIN_LINT by default
Now that the feature has had time to prove itself, and any
topics in flight have had a chance to clean up any broken
-chains, we can flip this feature on by default. This
makes one less thing submitters need to configure or check
before sending their patches.
Signed-off-by: Jeff King p...@peff.net
---
t/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4ea99a2..39da9c2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -529,7 +529,7 @@ test_run_ () {
test_cleanup=:
expecting_failure=$2
- if test ${GIT_TEST_CHAIN_LINT:-0} != 0; then
+ if test ${GIT_TEST_CHAIN_LINT:-1} != 0; then
# 117 is magic because it is unlikely to match the exit
# code of other programs
test_eval_ (exit 117) $1
--
2.4.0.rc2.498.g02440db
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Another approach to large transactions
On Wed, Apr 22, 2015 at 12:14:08PM -0700, Stefan Beller wrote: FWIW, we already use a magic value of 25 extra in open_packed_git_1. I don't know if that means the number has been proven in practice, or if it is simply that nobody actually exercises the pack_max_fds code. I suspect it is the latter, especially since d131b7a (sha1_file.c: Don't retain open fds on small packs, 2011-03-02). 25 is equally sound as I could not find any hard calculation on that number in the history or code. I will change it to 25 in the next version of the patch. FWIW, I think 32 is just fine, too, and the patch doesn't need re-rolled because of this. I mostly wanted to point out that yes, indeed, we use this eh, a few dozen is probably enough strategy elsewhere. Which maybe, sort-of validates it. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] refs.c: enable large transactions
On 04/22/2015 09:09 PM, Stefan Beller wrote: On Wed, Apr 22, 2015 at 7:11 AM, Michael Haggerty mhag...@alum.mit.edu wrote: + if (lock-lk-fd == -1) + reopen_lock_file(lock-lk); You should check that reopen_lock_file() was successful. ok @@ -3762,6 +3779,10 @@ int ref_transaction_commit(struct ref_transaction *transaction, update-refname); goto cleanup; } + if (remaining_fds 0) + remaining_fds--; + else + close_lock_file(update-lock-lk); I consider this code a stopgap, and simplicity is more important than optimization. Can you explain a bit why you think this is a stopgap? At the point the lockfile is created, we have all the information we need to write the new SHA-1 to it and close it immediately. It seems more straightforward to do it that way than the way it is done in the current code, where the locking and writing are separated in time and space and now there is the small extra complication of maybe-closing-maybe-not. But getting to the final destination requires more refactoring than would be prudent for the upcoming release. In other words, I think your fix is OK but that the whole area of code has still not reached its final form. I am working on a patch series that does what I have in mind, but it's not ready yet. As I remember I got stuck when I realized that the reflog for HEAD is updated somewhere out of the blue without proper locking and I haven't gotten around to sorting it out yet. [...] But just for the sake of discussion, if we planned to keep this code around, it could be improved by not wasting open file descriptors for references that are only being verified or deleted, like so: I'll pick that up for the resend. OK. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
forcing a user@ into the URL if not present
Hello, I'm using git 2.3.2 with Kerberos for authentication and gito-lite for authorization. This works: $ git clone https://dvl@ repo.example.org/git/testing Cloning into 'testing'... warning: You appear to have cloned an empty repository. Checking connectivity... done. My goal: have it work without supplying dvl@ as shown here: $ git clone https://repo.example.org/git/testing Cloning into 'testing'... Username for 'https://repo.example.org': I don't want to be prompted for a password. I want Kerberos to kick in. Following http://git-scm.com/docs/gitcredentials, the following seems to have nil effect. Anyone used this feature already? git config --global credential.https://repo.example.org.username dvl $ cat ~/.gitconfig [credential https://repo.example.org;] username = dvl [http] sslCAInfo = /usr/local/etc/trusted-certificates.pem With the above, I still get prompted for a password Given my use of Kerberos for authorization, is this option feasible? Should I be taking a different approach? Thank you. -- Dan Langille Infrastructure Operations Talos Group Sourcefire, Inc. -- To unsubscribe from this list: send the line 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: utf-8 filename woes
On Wed, Apr 22, 2015 at 08:50:26PM +0100, Jeremy Harris wrote: Installing the git-debuginfo package gave no additional information. The symptom does not show on a Fedora 21 system with git 2.1.0 (and I note that gitk properly shows those filenames on f21, and does not on sl6). Is this a known and fixed issue, or something my sl6 system needs installing? I'm not sure. v1.7.1 is 5 years old, and there have been a lot of fixes since then. I couldn't replicate your problem on v1.7.1 with a trivial test. If you can reproduce it at will and you really want to know where the fix is, you can try reverse-bisecting: git clone git://git.kernel.org/pub/scm/git/git.git cd git git bisect start git checkout v1.7.1 make [confirm that it shows the breakage] git bisect good git checkout v2.1.0 make [confirm that it does not show the problem] git bisect bad ... follow the instructions, testing teach ... Note that good and bad here are reversed of what you might expect. bisect was designed for finding regressions, not fixes, and you have to manually flip the two. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On Wed, Apr 22, 2015 at 02:12:53PM -0500, Patrick Sharp wrote:
Johannes,
You’re correct, looking back over it, I was pretty vague.
In truth, we are not using Windows OR putty at all. Running git on an Ubuntu
system, but we are setting the GIT_SSH environment variable to point to a
shell script to use.
Upon attempting to run git ls-remote, the system was spitting out getaddrinfo
errors for ‘atch’ .
Setting GIT_TRACE=2 showed that -batch was being added to the git command.
This was seen on several different servers with git versions 1.8.5.2, 1.9.1
and 2.3.5
After a bit we realized that it was the string ‘uplink’ in the GIT_SSH
variable that was linked to the extra -batch flag.
Finally, after searching the git source, we narrowed it down to the ‘plink’
portion of the string.
https://github.com/git/git/blob/7c597ef345aed345576de616c51f27e6f4b342b3/connect.c#L747-L756
I think you want something like:
diff --git a/connect.c b/connect.c
index 9ae991a..58aad56 100644
--- a/connect.c
+++ b/connect.c
@@ -568,7 +568,8 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
conn-argv = arg = xcalloc(7, sizeof(*arg));
if (protocol == PROTO_SSH) {
const char *ssh = getenv(GIT_SSH);
- int putty = ssh strcasestr(ssh, plink);
+ int putty = ssh (ends_with(ssh, plink) ||
+ ends_with(plink.exe));
if (!ssh) ssh = ssh;
*arg++ = ssh;
though that is not quite enough (we do not have a case-insensitive
version of ends_with). I'm also not sure if matching just plink and
plink.exe at the end of the string is enough (I'm just guessing that
was the original reason for using strstr in the first place).
Note that I don't think just switching the strcasestr to look for
plink.exe is right. For one thing, it just punts on the problem (it
can still happen, it's just less likely to trigger). But for another,
you can have plink (without .exe) on Linux systems.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
On Wed, Apr 22, 2015 at 09:19:15PM +, brian m. carlson wrote: Note that I don't think just switching the strcasestr to look for plink.exe is right. For one thing, it just punts on the problem (it can still happen, it's just less likely to trigger). But for another, you can have plink (without .exe) on Linux systems. Perhaps it would be worthwhile to check instead if the text plink is the beginning of string or is preceded by a path separator. That would give us a bit more confidence that the user is looking for plink, but would still allow people to use plink-0.63 if they like. Yeah, I think that is a reasonable approach. Note that it needs to handle the tortoiseplink case from below, too (you can still use your strategy, you just need to look for either string). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pathspec: adjust prefixlen after striping trailing slash
Am 22.04.2015 um 21:58 schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Am 21.04.2015 um 23:08 schrieb Junio C Hamano: I looked at the test script update. The new test does (I am rephrasing to make it clearer): mkdir -p dir/ectory git init dir/ectory ;# a new directory inside top-level project ( cd dir/ectory test git add test git commit -m test ) git add dir/ectory to set it up. At this point, the top-level index knows dir/ectory is a submodule. Then the test goes on to turn it a non submodule by mv dir/ectory/.git dir/ectory/dotgit ... We already do (2) in the cases you describe: $ git add subrepo/a fatal: Pathspec 'subrepo/a' is in submodule 'subrepo' $ git -C subrepo add a fatal: Pathspec 'a' is in submodule 'subrepo' ... So I'd vote to have (2) also for git -C subrepo add ., which is what started this thread. Does having mv subrepo/.git subrepo/dotgit before that git add change your conclusion? It is very clear to me that without that mv step, (2) is absolutely the right thing to do, and I agree with you. Huh? Without that mv all files would simply be added to the submodule repo and this would be a non-issue ... what am I missing? I'm just advocating to let git add . in a submodule without a .git behave like git add file already does. But it is unclear if we should still do (2) when subrepo/.git is no longer there. That has to be done manually and it may be an indication that is clear enough that the end user wants the directory to be a normal directory without any submodule involved, in which case it may match the expectation of the user better to just nuke the corresponding 16 entry in the index and replace it with files in there. I dunno. The user having removed subrepo/.git is just one reason for that. Another is a user adding a file in an unpopulated work tree of a not initialized submodule. I doubt that simply nuking the 16 entry would be the right thing to do in this case, I expect this to be a pilot error we should barf about ;-) -- To unsubscribe from this list: send the line 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: support git+mosh for unreliable connections
On Wed, 15 Apr 2015 21:25:44 +, Dennis Kaarsemaker wrote: ... It does not and cannot work. The way mosh works, is that it uses ssh to log in and launch a mosh-server daemon. This daemon and the mosh client then communicate via a custom UDP protocol. The SSH connection is closed after the mosh-server has been launched as it is no longer needed. The communication between the mosh client and server synchronizes terminal state, somewhat like what screen/tmux do. I object to the 'can not' part a bit. There is (1) the terminal state prediction and (2) the reliable-over-reconnects communication, and for a noninteractive usage you'd need only (2). Once upon a time I implemented a simple UDP server and client; the client to be used as a ProxyCommand in ssh, and the server just talks to the local ssh server. This pretty much does what the OP wants, and it works just as a transport for ssh, so all ssh features are there (but of course there is no terminal prediction). Unfortunately it needs to be ported to libev/libuv before it could be released. It's *much* simpler than mosh, although the use-ssh-to-start-server trick would be nice.) Andreas -- Totally trivial. Famous last words. From: Linus Torvalds torvalds@*.org Date: Fri, 22 Jan 2010 07:29:21 -0800 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
How can I configure zsh to autocomplete branch names in zsh?
How can I configure zsh to autocomplete branch names in zsh? I have tried a lot of methods via google, but it was never succeed. -- View this message in context: http://git.661346.n2.nabble.com/How-can-I-configure-zsh-to-autocomplete-branch-names-in-zsh-tp7629174.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] git-multimail: Add an option to filter on branches
On 04/22/2015 01:04 AM, Dave Boutcher wrote: Add a branches option to the config. Only changes pushed to specified branches will generate emails. Changes to tags will continue to generate emails. Thanks for the patches. Patches 2 and 3 seem uncontroversial, so I already merged them. Patch 1 is more interesting, and there have been other proposals for similar features, so I created a pull request for it: https://github.com/git-multimail/git-multimail/pull/75 (Note the new URL--I just created a GitHub organization for git-multimail to make it easier for other people to get involved. More info soon...) Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line 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] git-multimail: Add an option to filter on branches
Thanks Michael, The only code I'm not fond of is matching on a list of regular expressions. There must be a more pythonic way to do: + return [x for x in [r.match(branch) for r in branches] if x] which basically returns true if branch matches any regular expression in the list. I pushed this change out to our production git server (its good to be the king.) I'll obviously update here if it does anything too unfortunate. On Wed, Apr 22, 2015 at 6:39 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/22/2015 01:04 AM, Dave Boutcher wrote: Add a branches option to the config. Only changes pushed to specified branches will generate emails. Changes to tags will continue to generate emails. Thanks for the patches. Patches 2 and 3 seem uncontroversial, so I already merged them. Patch 1 is more interesting, and there have been other proposals for similar features, so I created a pull request for it: https://github.com/git-multimail/git-multimail/pull/75 (Note the new URL--I just created a GitHub organization for git-multimail to make it easier for other people to get involved. More info soon...) Michael -- Michael Haggerty mhag...@alum.mit.edu -- Dave B 包小龙 -- To unsubscribe from this list: send the line 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's directory is _prepended_ to PATH when called with an absolute path
On 22/04/15 02:47, Andreas Krey wrote: On Tue, 21 Apr 2015 18:37:29 +, David Rodríguez wrote: ... This causes issues with Ruby git hooks, because Ruby version managers rely on custom settings in PATH to select the Ruby executable, Even if git wouldn't modify PATH this is still a broken way to do that. What ruby to execute a hook with is a property of the hook, not of the user context invoking it. Andreas I probably shouldn't have mentioned Ruby version managers since they are not directly related to this issue. I'll try to elaborate on the issue: * User is relying on a custom path to select their Ruby version. For example, let's say the first folder in path is ~/.rubies/2.2.2/bin. * User runs /usr/bin/git commit and a pre-commit hook is triggered. * The pre-commit hook starts with #!/us/bin/env ruby to select the Ruby to be used in the hook, but since the path has been changed by /usr/bin/git, the selected ruby will be /usr/bin/ruby and not ~/.rubies/2.2.2/bin/ruby as the user would expect. What's the proper way to do whatever you're saying is done in a broken way? -- To unsubscribe from this list: send the line 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] git-multimail: Add an option to filter on branches
On 04/22/2015 12:46 PM, Dave Boutcher wrote: The only code I'm not fond of is matching on a list of regular expressions. There must be a more pythonic way to do: + return [x for x in [r.match(branch) for r in branches] if x] which basically returns true if branch matches any regular expression in the list. I think what you are looking for is return any(r.match(branch) for r in branches) This also has the advantage of stopping processing as soon as it finds a match. I was also wondering why you decided to support comma-separated lists of regexps *and* multivalued config settings. It seems that supporting only multivalued settings would suffice. Maybe you are following the precedent of our email configuration settings, which support comma-separated lists of email addresses? If so, I don't think that is necessary. The email support is there for backwards compatibility and because comma-separated email addresses are a thing in RFC 2822. Neither of these arguments apply to branch regexps. I pushed this change out to our production git server (its good to be the king.) I'll obviously update here if it does anything too unfortunate. Thanks. Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Proposal for git stash : add --staged option
Hello, There's some feature of git that I have been missing. When you have a lot of unstaged files, and would like to test what happens if you undo some of the changes that you think are unecessary, you would rather keep a copy of those changes somewhere. For example Changed but not updated: M config_test.xml M config_real.xml I have changed both config_test.xml and config_real.xml, but I think the changes made in config_test.xml are unnecessary. However, I would still like to keep them somewhere in case it breaks something. In this case for example, I would like to be able to stash only the file config_test.xml Eg: git add config_test.xml git stash --staged So that after this, my git looks like this: Changed but not updated: M config_real.xml and my stash contains only the changes introduced in config_test.xml `git stash --keep-index` doesn't give the necessary control, because it will still stash everything (and create unnecessary merge complications if I change the files and apply the stash) Best, Edgar -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Why does git log -Gregex works with regexp-ignore-case but not with other regexp-related options?
Junio C Hamano venit, vidit, dixit 21.04.2015 18:59: Michael J Gruber g...@drmicha.warpmail.net writes: We have engine-switching options and engine-modification options. The latter are certainly good in the expression itself. Maybe even the former, though I don't know how to switch away from fixed-strings in that way... I do not think mixing matching engines in a single request makes much sense. As the internal machinery is not even prepared to do that, even though it is prepared to apply engine-modifications ones to each grep term AFAIK, let's not go there. From a user perspective, we mix engines already: fixed strings for -S, regexp for the rest (by default). The user can switch one, but not the other. And there are options that modify both engines at the same time. That is the kind of confusion that (triggered OP's request and that) I would like to resolve. Michael -- To unsubscribe from this list: send the line 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: Proposal for git stash : add --staged option
Hi Edgar, On 2015-04-22 10:30, edgar.h...@netapsys.fr wrote: When you have a lot of unstaged files, and would like to test what happens if you undo some of the changes that you think are unecessary, you would rather keep a copy of those changes somewhere. For example Changed but not updated: M config_test.xml M config_real.xml I have changed both config_test.xml and config_real.xml, but I think the changes made in config_test.xml are unnecessary. However, I would still like to keep them somewhere in case it breaks something. In this case for example, I would like to be able to stash only the file config_test.xml Eg: git add config_test.xml git stash --staged So that after this, my git looks like this: Changed but not updated: M config_real.xml and my stash contains only the changes introduced in config_test.xml `git stash --keep-index` doesn't give the necessary control, because it will still stash everything (and create unnecessary merge complications if I change the files and apply the stash) I often have the same problem. How about doing this: ```sh git add config_real.xml git stash -k git reset ``` The difference between our approaches is that I keep thinking of the staging area as the place to put changes I want to *keep*, not that I want to forget for a moment. Having said that, I am sympathetic to your cause, although I would rather have `git stash [--patch] -- [file...]` that would be used like `git add -p` except that the selected changes are *not* staged, but stashed instead. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
