Re: `git stash pop` UX Problem
Matthieu Moy writes: Holger Hellmuth writes: Am 24.02.2014 17:21, schrieb Matthieu Moy: $ git add foo.txt $ git status On branch master Changes to be committed: (use "git reset HEAD ..." to unstage) modified: foo.txt Maybe status should display a stash count if that count is > 0, as this is part of the state of the repo. Maybe it would help some users, but not me for example. My main use of "git stash" is a safe replacement for "git reset --hard": when I want to discard changes, but keep them safe just in case. So, my stash count is almost always >0, and I don't want to hear about it. "status" is about reminding the user what changes are already in the index (i.e. what you would commit) and what changes are in the working tree, from which you could further update the index with (i.e. what you could commit). One _could_ argue that stashed changes are what could be reflected to the working tree and form the source of the latter, but my gut feeling is that it is a rather weak argument. At that point you are talking about what you could potentially change in the working tree, and the way to do so is not limited to "stash pop" (i.e. you can "git cherry-pick --no-commit $a_commit", or "edit" any file in the working tree for that matter, with the same ease). So, I tend to agree with you, while I do understand where "I want to know about what is in stash" is coming from (and that is why we do have "git stash list" command). Same comment. Everyone will have his own opinion. As long as the messages are not customizable, we can debate for hours and everybody has a valid point. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Am 24.02.2014 17:21, schrieb Matthieu Moy: $ git add foo.txt $ git status On branch master Changes to be committed: (use "git reset HEAD ..." to unstage) modified: foo.txt Maybe status should display a stash count if that count is > 0, as this is part of the state of the repo. $ git status On branch master Stashes: 1 <-- Changes to be committed: (use "git reset HEAD ..." to unstage) modified: foo.txt It would be in Omars example case a clear message that git kept the stash. And generally a reminder that there is still a stash around that might or might not be obsolete. Again, the same comment: If there is a way to customize git's messages by turning them on/off (or, even cooler, the ability to change their wording) then this is also a nice option to have and we can turn it off by default if we find that most people (here at least) don't like it. I don't know whether you guys have discussed this option before (or does it exist? I doubt, but I don't know), because having such an option (the ability to turn messages on/off or change their wording and what internal status information they manifest) will really resolve all kinds of such potential conflicts of preferences. Even cooler, people will be able to change the wording to their native languages for example. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
[omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt [omar_othman main (trunk*)]$ Note how the status message has changed to show that git is now happy. It is at that moment that the stash reference should be dropped Dropping the stash on a "git add" operation would be really, really weird... (or the user (somehow) is notified to do that herself if desired), because this means that the popping operation has succeeded. But how would you expect to "be notified"? Answering the last question, your previous comments are fine with me: If there's any change that should be made it should be purely providing more detailed instructions to the user about how to deal with it. Yes, there may be room for improvement, but that does not seem so easy. Today, we have: $ git stash pop Auto-merging foo.txt CONFLICT (content): Merge conflict in foo.txt $ git status On branch master Unmerged paths: (use "git reset HEAD ..." to unstage) (use "git add ..." to mark resolution) both modified: foo.txt => The advices shown here are OK. Then: $ git add foo.txt $ git status On branch master Changes to be committed: (use "git reset HEAD ..." to unstage) modified: foo.txt => here, "git status" could have hinted the user "you may now run 'git stash drop' if you are satisfied with your merge". Though I don't know why you think this is important: Now, the real question is: when would Git stop showing this advice. I don't see a real way to answer this, and I'd rather avoid doing just a guess. If it is really annoying for the user, we can just have a configuration parameter to switch this message on/off. I don't know whether git has such customizations (in general) currently. This is very useful (maybe we can agree on wording later): One easy thing to do OTOH would be to show a hint at the end of "git stash pop"'s output, like $ git stash pop Auto-merging foo.txt CONFLICT (content): Merge conflict in foo.txt 'stash pop' failed. Please resolve the conflicts manually. The stash was not dropped in case you need to restart the operation. When you are done resolving the merge, you may run the following to drop the stash reference: git stash drop -- To unsubscribe from this list: send the line "unsubscribe 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 10/25] Add new environment variable $GIT_COMMON_DIR
On Tue, Feb 18, 2014 at 8:39 AM, Nguyễn Thái Ngọc Duy wrote: > This variable is intended to support multiple working directories > attached to a repository. Such a repository may have a main working > directory, created by either "git init" or "git clone" and one or more > linked working directories. These working directories and the main > repository share the same repository directory. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > diff --git a/Documentation/git.txt b/Documentation/git.txt > index 02bbc08..2c4a055 100644 > --- a/Documentation/git.txt > +++ b/Documentation/git.txt > @@ -773,6 +773,14 @@ Git so take care if using Cogito etc. > an explicit repository directory set via 'GIT_DIR' or on the > command line. > > +'GIT_COMMON_DIR':: > + If this variable is set to a path, non-worktree files that are > + normally in $GIT_DIR will be taken from this path > + instead. Worktree-specific files such as HEAD or index are > + taken from $GIT_DIR. This variable has lower precedence than > + other path variables such as GIT_INDEX_FILE, > + GIT_OBJECT_DIRECTORY... For a person not familiar with "git checkout --to" or its underlying implementation, this description may be lacking. Such a reader may be left wondering about GIT_COMMON_DIR's overall purpose, and when and how it should be used. Perhaps it would make sense to talk a bit about "git checkout --to" here? > Git Commits > ~~~ > 'GIT_AUTHOR_NAME':: -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
On Mon, Feb 24, 2014 at 05:21:40PM +0100, Matthieu Moy wrote: > One easy thing to do OTOH would be to show a hint at the end of "git > stash pop"'s output, like I think that's a good idea. It makes it obvious that Git has kept the stash and that the user should drop it when he's done - if he wants to. > $ git stash pop > Auto-merging foo.txt > CONFLICT (content): Merge conflict in foo.txt > 'stash pop' failed. Please, resolve the conflicts manually. The stash > was not dropped in case you need to restart the operation. When you are > done resolving the merge, you may run the following to drop the stash: > > git stash drop Maybe just the following to keep the output on a single line: Use 'git stash drop' to remove the stash after resolving the conflicts. But maybe that's too short as it doesn't mention explicitly, that the stash was kept. Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
On Tue, Feb 25, 2014 at 01:33:56PM +0100, Matthieu Moy wrote: > Holger Hellmuth writes: > > Maybe status should display a stash count if that count is > 0, as > > this is part of the state of the repo. > > Maybe it would help some users, but not me for example. My main use of > "git stash" is a safe replacement for "git reset --hard": when I want to > discard changes, but keep them safe just in case. > > So, my stash count is almost always >0, and I don't want to hear about > it. I concur with this. Sometimes the stashed changes are remnants of a small hack or a very brief start to an aborted project that I stashed when I needed to change branches. I figure that they might be useful in the future, but I don't care about them right now. I may pick them up, I may not, but I certainly don't want to be reminded of them constantly. -- 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
What's cooking in git.git (Feb 2014, #07; Tue, 25)
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
The tip of 'next' has been rewound. There are healthy number of
topics in there that have been well-cooked during the 1.9.0
development cycle that can graduate to 'master' (see the announce in
http://article.gmane.org/gmane.comp.version-control.git/242658).
After they are merged to 'master', I'm planning to go back to the
list archive to pick up patches I may have missed in the meantime.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[New Topics]
* ak/gitweb-fit-image (2014-02-20) 1 commit
- gitweb: Avoid overflowing page body frame with large images
Instead of allowing an to be shown in whatever size, force
scaling it to fit on the page with max-height/max-width css style
attributes.
Will merge to 'next'.
* da/difftool-git-files (2014-02-24) 1 commit
- difftool: support repositories with .git-files
"git difftool" misbehaved when the repository is bound to the
working tree with the ".git file" mechanism, where a textual
file ".git" tells us where it is.
Will merge to 'next'.
* jk/commit-dates-parsing-fix (2014-02-24) 5 commits
- log: do not segfault on gmtime errors
- log: handle integer overflow in timestamps
- date: check date overflow against time_t
- fsck: report integer overflow in author timestamps
- t4212: test bogus timestamps with git-log
Will merge to 'next'.
* jk/diff-filespec-cleanup (2014-02-24) 1 commit
- diffcore.h: be explicit about the signedness of is_binary
Will merge to 'next' and then to 'master' and 'maint'.
* jk/remote-pushremote-config-reading (2014-02-24) 1 commit
- remote: handle pushremote config in any order
Will merge to 'next'.
* jk/repack-pack-keep-objects (2014-02-24) 1 commit
- repack: add `repack.packKeepObjects` config var
(this branch uses jk/pack-bitmap.)
Names?
* jm/stash-doc-k-for-keep (2014-02-24) 1 commit
- stash doc: mention short form -k in save description
Will merge to 'next'.
* jn/am-doc-hooks (2014-02-24) 1 commit
- am doc: add a pointer to relevant hooks
Will merge to 'next'.
* mh/object-code-cleanup (2014-02-24) 4 commits
- sha1_file.c: document a bunch of functions defined in the file
- sha1_file_name(): declare to return a const string
- find_pack_entry(): document last_found_pack
- replace_object: use struct members instead of an array
Will merge to 'next'.
* nd/i18n-progress (2014-02-24) 1 commit
- i18n: mark all progress lines for translation
Will merge to 'next'.
* nd/sha1-file-delta-stack-leakage-fix (2014-02-24) 1 commit
- sha1_file: fix delta_stack memory leak in unpack_entry
Will merge to 'next' and then to 'master' and 'maint'.
* tc/commit-dry-run-exit-status-tests (2014-02-24) 1 commit
- demonstrate git-commit --dry-run exit code behaviour
--
[Stalled]
* kb/fast-hashmap-pack-struct (2014-02-24) 1 commit
- hashmap.h: make sure map entries are tightly packed
(this branch uses kb/fast-hashmap.)
I am inclined to drop this; an alternative is to replace it with
the "more portable" one that uses #pragma, which I am willing to
try doing so on 'pu', though.
* po/everyday-doc (2014-01-27) 1 commit
- Make 'git help everyday' work
This may make the said command to emit something, but the source is
not meant to be formatted into a manual pages to begin with, and
also its contents are a bit stale. It may be a good first step in
the right direction, but needs more work to at least get the
mark-up right before public consumption.
Will hold.
* jk/branch-at-publish-rebased (2014-01-17) 5 commits
- t1507 (rev-parse-upstream): fix typo in test title
- implement @{publish} shorthand
- branch_get: provide per-branch pushremote pointers
- branch_get: return early on error
- sha1_name: refactor upstream_mark
Give an easier access to the tracking branches from "other" side in
a triangular workflow by introducing B@{publish} that works in a
similar way to how B@{upstream} does.
Meant to be used as a basis for whatever Ram wants to build on.
Will hold.
* rb/merge-prepare-commit-msg-hook (2014-01-10) 4 commits
- merge: drop unused arg from abort_commit method signature
- merge: make prepare_to_commit responsible for write_merge_state
- t7505: ensure cleanup after hook blocks merge
- t7505: add missing &&
Expose more merge states (e.g. $GIT_DIR/MERGE_MODE) to hooks that
run during "git merge". The log message stresses too much on one
hook, prepare-commit-msg, but it would equally apply to other hooks
like post-merge, I think.
Waiting for a reroll.
* jl/submodule-recursive-checkout (2013-12-26) 5 commits
- Teach checkout to recursively checkout submodules
- submodule: teach unpack_tre
Re: [PATCH] help: include list of aliases in git-help --all
Joel Nothman writes:
> Git help --all had listed all git commands, but no configured aliases.
> This includes aliases as a separate listing, after commands in the main
> git directory and other $PATH directories.
>
> Signed-off-by: Joel Nothman gmail.com>
> ---
Thanks.
> diff --git a/help.c b/help.c
> index df7d16d..3c14af4 100644
> --- a/help.c
> +++ b/help.c
> @@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
> int i;
>
> for (i = 0; i < cmds->cnt; i++)
> - string_list_append(&list, cmds->names[i]->name);
> + string_list_append(&list, cmds->names[i]->name);
Why?
> @@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
> exclude_cmds(other_cmds, main_cmds);
> }
>
> +static struct cmdnames aliases;
Instead of using a static global variable, perhaps make this an
on-stack variable in load_commands_and_aliases() that is passed as a
callback parameter to load_aliases_cb() thru git_config()?
> +static int load_aliases_cb(const char *var, const char *value, void *cb)
> +{
That is, cb here is the second parameter you gave to git_config().
> void list_commands(unsigned int colopts,
> -struct cmdnames *main_cmds, struct cmdnames *other_cmds)
> +struct cmdnames *main_cmds, struct cmdnames *other_cmds,
> +struct cmdnames *alias_cmds)
> {
> if (main_cmds->cnt) {
> const char *exec_path = git_exec_path();
> @@ -219,6 +259,13 @@ void list_commands(unsigned int colopts,
> pretty_print_string_list(other_cmds, colopts);
> putchar('\n');
> }
> +
> + if (alias_cmds->cnt) {
> + printf_ln(_("aliases defined in git configuration"));
This will not break the use of "git help -a" in our completion
script, because it ignores anything that does not begin with two SP
followed by alphanumerics.
It may however break scripts that read from "help -a" done by other
people that may remove the lines in the output that are known to
them as not names of commands (i.e. "available git commands..." and
"git commands avaliable elsewhere...")---they haven't seen this new
string and would not know that this line must be skipped.
Other than that, looks reasonably cleanly done. We'd need a test to
cover this so that other people will not break it in future patches.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] help: include list of aliases in git-help --all
From: "Joel Nothman"
On 26 February 2014 06:15, Junio C Hamano wrote:
Joel Nothman writes:
Git help --all had listed all git commands, but no configured
aliases.
This includes aliases as a separate listing, after commands in the
main
git directory and other $PATH directories.
... and why is this a good thing?
A fair question: I had considered it a conspicuous absence from the
list of commands in git help.
Surely one alternative would be to add an --alias or --aliases option to
the help command so the user can chose which ones s/he desires to be
helped about.
At least that's the way I did it with the --guides option.
After all, aliases are treated like
commands for a few purposes: they are executed as commands, they are
listed as possible command spelling corrections, and they are valid
arguments to git help. They are also like commands in that it is
possible to forget their name, or whether they are defined on a
particular workstation, and to hence want a listing. A user may also
not recall whether they had implemented a particular shortcut as an
alias or as a script (one may initially implement a script, and
realise an alias is sufficient; one may at first implement an alias
and realise it is insufficient, and that a script is needed).
In short, for many of the purposes in which one would seek git help
-a, there is no reason to *exclude* aliases from a list of commands
executed likewise.
Signed-off-by: Joel Nothman gmail.com>
---
Documentation/git-help.txt | 4 +--
builtin/help.c | 7 ++---
help.c | 64
+++---
help.h | 7 -
4 files changed, 61 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index b21e9d7..c9fe791 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -40,8 +40,8 @@ OPTIONS
---
-a::
--all::
- Prints all the available commands on the standard output. This
- option overrides any given command or guide name.
+ Prints all the available commands and aliases on the standard
output.
+ This option overrides any given command or guide name.
-g::
--guides::
diff --git a/builtin/help.c b/builtin/help.c
index 1fdefeb..d1dfecd 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -38,7 +38,7 @@ static int show_guides = 0;
static unsigned int colopts;
static enum help_format help_format = HELP_FORMAT_NONE;
static struct option builtin_help_options[] = {
- OPT_BOOL('a', "all", &show_all, N_("print all available
commands")),
+ OPT_BOOL('a', "all", &show_all, N_("print all available
commands and aliases")),
OPT_BOOL('g', "guides", &show_guides, N_("print list of useful
guides")),
OPT_SET_INT('m', "man", &help_format, N_("show man page"),
HELP_FORMAT_MAN),
OPT_SET_INT('w', "web", &help_format, N_("show manual in web
browser"),
@@ -453,6 +453,7 @@ int cmd_help(int argc, const char **argv, const
char *prefix)
int nongit;
const char *alias;
enum help_format parsed_help_format;
+ struct cmdnames alias_cmds;
argc = parse_options(argc, argv, prefix, builtin_help_options,
builtin_help_usage, 0);
@@ -461,8 +462,8 @@ int cmd_help(int argc, const char **argv, const
char *prefix)
if (show_all) {
git_config(git_help_config, NULL);
printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
- load_command_list("git-", &main_cmds, &other_cmds);
- list_commands(colopts, &main_cmds, &other_cmds);
+ load_commands_and_aliases("git-", &main_cmds,
&other_cmds, &alias_cmds);
+ list_commands(colopts, &main_cmds, &other_cmds,
&alias_cmds);
}
if (show_guides)
diff --git a/help.c b/help.c
index df7d16d..3c14af4 100644
--- a/help.c
+++ b/help.c
@@ -86,7 +86,7 @@ static void pretty_print_string_list(struct
cmdnames *cmds,
int i;
for (i = 0; i < cmds->cnt; i++)
- string_list_append(&list, cmds->names[i]->name);
+ string_list_append(&list, cmds->names[i]->name);
/*
* always enable column display, we only consult column.*
* about layout strategy and stuff
@@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
exclude_cmds(other_cmds, main_cmds);
}
+static struct cmdnames aliases;
+
+static void add_cmd_list(struct cmdnames *cmds, struct cmdnames
*old)
+{
+ int i;
+ ALLOC_GROW(cmds->names, cmds->cnt + old->cnt, cmds->alloc);
+
+ for (i = 0; i < old->cnt; i++)
+ cmds->names[cmds->cnt++] = old->names[i];
+ free(old->names);
+ old->cnt = 0;
+ old->names = NULL;
+}
+
+static int load_aliases_cb(const char *var, const char *value, void
*cb)
+{
+ if (starts_with(var, "alias."))
+ add_cmdname(&aliases, var + 6, strlen(var + 6));
+ return 0;
+}
+
+void load_commands_and_aliases(const char *prefix,
+
Re: [PATCH v3 07/25] reflog: avoid constructing .lock path with git_path
Nguyễn Thái Ngọc Duy writes:
> git_path() soon understands the path given to it. Some paths "abc" may
> become "def" while other "ghi" may become "ijk". We don't want
> git_path() to interfere with .lock path construction. Concatenate
> ".lock" after the path has been resolved by git_path() so if "abc"
> becomes "def", we'll have "def.lock", not "ijk".
Hmph. I am not sure if the above is readable (or if I am reading it
correctly).
If "abc" becomes "def", it would take deliberate work to make
"abc.lock" into "ijk", and it would be natural to expect that
"abc.lock" would become "def.lock" without any fancy trick, no?
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> builtin/reflog.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 852cff6..ccf2cf6 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -372,7 +372,7 @@ static int expire_reflog(const char *ref, const unsigned
> char *sha1, int unused,
> if (!file_exists(log_file))
> goto finish;
> if (!cmd->dry_run) {
> - newlog_path = git_pathdup("logs/%s.lock", ref);
> + newlog_path = mkpathdup("%s.lock", log_file);
> cb.newlog = fopen(newlog_path, "w");
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] help: include list of aliases in git-help --all
On 26 February 2014 08:51, Junio C Hamano wrote: > Joel Nothman writes: > >> arguments to git help. They are also like commands in that it is >> possible to forget their name, or whether they are defined on a >> particular workstation, and to hence want a listing. > > I did envision that it would be useful for the last case, but then > the users would be helped even more if they can get a list of ONLY > aliases, not buried in many standard commands they already know > about. The list is partitioned. It is partitioned already between git-installed commands and others on the path. This patch adds a third partition when required. So they *do* see only aliases... after all the commands. Note also that any command on the path will override an alias with the same name. So in order to list (effective) aliases, you need to calculate the list of commands as well. If someone defines an alias overridden by a command, git help -a now makes that apparent by excluding the alias and including the command above it, while `git config --get-regexp ^alias` does not. > In other words, I was not fundamentally opposed to *a* way to get a > list that includes aliases, but I was not convinced if it is a good > idea to *change* the output, which people knew would consist of > commands but not aliases, to suddenly start including aliases. I don't think this will concern most users for whom aliases are non-existent, and hence no section will be shown. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Stephen Leake writes: >> Dropping the stash on a "git add" operation would be really, really >> weird... > > Why? That is when the merge conflicts are resolved, which is what > logically indicates that the stash is no longer needed,... Not necessarily. Imagine a case where you used stash to quickly save away a tangled mess that was not ready for a logically single commit and now you are in the process of creating the first commit by applying it piece-by-piece to create multiple resulting ones. After you commit the result, you would still want to keep the parts of that stashed change you did not include in the first commit so that you can go back, no? You may run "git add", but that does not say anything about what you are going to use the rest of the stash for. Not even "git commit" may be a good enough sign. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Stephen Leake writes: >> One _could_ argue that stashed changes are what could be reflected >> to the working tree and form the source of the latter, but my gut >> feeling is that it is a rather weak argument. At that point you are >> talking about what you could potentially change in the working tree, > > No, I saved things in the stash on purpose. For example, I had changes > that were not ready to commit, but I wanted to do a merge from upstream. I often save things by running "git diff >P.diff" on purpose. Should "git status" read these patches and tell me what paths I could change in the working tree by applying it? Where does it end? > There are workflows where the stash is not important; provide an option > to 'git status' that means "ignore stash". How is that different to tell those who want to know what are in the stash to type "git stash list" when they want to learn that information? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: support repositories with .git-files
Jens Lehmann writes: >>> +test_expect_success PERL 'difftool properly honours gitlink and >>> core.worktree' ' >>> + git submodule add ./. submod/ule && >>> + ( >>> + cd submod/ule && >>> + git difftool --tool=echo --dir-diff --cached >> >> In the context of this fix, finishing with 0 exit status may be all >> we care about, but do we also care about things like in what >> directory the tool is invoked in, what arguments and extra >> environment settings (if any) it is given, and stuff like that? > > Sure. But I just intended to test the fix (and the test can easily > be extended by people who know more about difftool than I do). Yes, we need to start somewhere and I'd agree that it was a good starting point. > Right, using echo was not the best choice here. I used it to avoid > the dependency to meld... Perhaps like this then? This is an "a monkey sees what difftool_test_setup does and then mimics" patch ;-). t/t7800-difftool.sh | 13 + 1 file changed, 13 insertions(+) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 2418528..595f808 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -434,4 +434,17 @@ test_expect_success PERL 'difftool --no-symlinks detects conflict ' ' ) ' +test_expect_success PERL 'difftool properly honours gitlink and core.worktree' ' + git submodule add ./. submod/ule && + ( + cd submod/ule && + git config diff.tool checktrees && + git config difftool.checktrees.cmd '\'' + test -d "$LOCAL" && test -d "$REMOTE" + '\'' && + echo further >>file && + git difftool --tool=checktrees --dir-diff + ) +' + test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] help: include list of aliases in git-help --all
Joel Nothman writes: > arguments to git help. They are also like commands in that it is > possible to forget their name, or whether they are defined on a > particular workstation, and to hence want a listing. I did envision that it would be useful for the last case, but then the users would be helped even more if they can get a list of ONLY aliases, not buried in many standard commands they already know about. In other words, I was not fundamentally opposed to *a* way to get a list that includes aliases, but I was not convinced if it is a good idea to *change* the output, which people knew would consist of commands but not aliases, to suddenly start including aliases. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re* [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote
Linus Torvalds writes:
> Thinking some more about the tag_name issue, I realize that the other
> patch ("Make request-pull able to take a refspec of form
> local:remote") broke another thing.
>
> The first patch pretty-printed the local branch-name, removing "refs/"
> and possibly "heads/" from the local refname. So for a branch, it
> would ask people to just pull from the branch-name, and for a tag it
> would ask people to pull from "tags/name", which is good policy. So if
> you had a tag called "for-linus", it would say so (using
> "tags/for-linus").
>
> But the local:remote syntax thing ends up breaking that nice feature.
> The old find_matching_refs would actually cause us to show the "tags"
> part if it existed on the remote, but that had become pointless and
> counter-productive with the first patch. But with the second patch,
> maybe we should reinstate that logic..
Sorry for back-burnering this topic so long.
I think the following does what you suggested in the message I am
responding to.
Now, hopefully the only thing we need is a documentation update and
the series should be ready to go.
-- >8 --
Subject: request-pull: resurrect "pretty refname" feature
When asking to fetch/pull a branch whose name is B or a tag whose
name is T, we used to show the command to run as:
git pull $URL B
git pull $URL tags/T
even when B and T were spelled in a more qualified way in order to
disambiguate, e.g. heads/B or refs/tags/T, but the recent update
lost this feature. Resurrect it.
Signed-off-by: Junio C Hamano
---
git-request-pull.sh | 4 +++-
t/t5150-request-pull.sh | 8 +++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/git-request-pull.sh b/git-request-pull.sh
index 93b4135..b67513a 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -53,6 +53,8 @@ fi
local=${3%:*}
local=${local:-HEAD}
remote=${3#*:}
+pretty_remote=${remote#refs/}
+pretty_remote=${pretty_remote#heads/}
head=$(git symbolic-ref -q "$local")
head=${head:-$(git show-ref --heads --tags "$local" | cut -d' ' -f2)}
head=${head:-$(git rev-parse --quiet --verify "$local")}
@@ -124,7 +126,7 @@ git show -s --format='The following changes since commit %H:
are available in the git repository at:
' $merge_base &&
-echo " $url $remote" &&
+echo " $url $pretty_remote" &&
git show -s --format='
for you to fetch changes up to %H:
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 2622057..75d6b38 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -216,8 +216,14 @@ test_expect_success 'pull request format' '
git request-pull initial "$downstream_url" tags/full >../request
) &&
request.fuzzy &&
- test_i18ncmp expect request.fuzzy
+ test_i18ncmp expect request.fuzzy &&
+ (
+ cd local &&
+ git request-pull initial "$downstream_url"
tags/full:refs/tags/full
+ ) >request &&
+ sed -nf fuzz.sed request.fuzzy &&
+ test_i18ncmp expect request.fuzzy
'
test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] help: include list of aliases in git-help --all
On 26 February 2014 06:15, Junio C Hamano wrote:
> Joel Nothman writes:
>
>> Git help --all had listed all git commands, but no configured aliases.
>> This includes aliases as a separate listing, after commands in the main
>> git directory and other $PATH directories.
>
> ... and why is this a good thing?
A fair question: I had considered it a conspicuous absence from the
list of commands in git help. After all, aliases are treated like
commands for a few purposes: they are executed as commands, they are
listed as possible command spelling corrections, and they are valid
arguments to git help. They are also like commands in that it is
possible to forget their name, or whether they are defined on a
particular workstation, and to hence want a listing. A user may also
not recall whether they had implemented a particular shortcut as an
alias or as a script (one may initially implement a script, and
realise an alias is sufficient; one may at first implement an alias
and realise it is insufficient, and that a script is needed).
In short, for many of the purposes in which one would seek git help
-a, there is no reason to *exclude* aliases from a list of commands
executed likewise.
>>
>> Signed-off-by: Joel Nothman gmail.com>
>> ---
>> Documentation/git-help.txt | 4 +--
>> builtin/help.c | 7 ++---
>> help.c | 64
>> +++---
>> help.h | 7 -
>> 4 files changed, 61 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
>> index b21e9d7..c9fe791 100644
>> --- a/Documentation/git-help.txt
>> +++ b/Documentation/git-help.txt
>> @@ -40,8 +40,8 @@ OPTIONS
>> ---
>> -a::
>> --all::
>> - Prints all the available commands on the standard output. This
>> - option overrides any given command or guide name.
>> + Prints all the available commands and aliases on the standard output.
>> + This option overrides any given command or guide name.
>>
>> -g::
>> --guides::
>> diff --git a/builtin/help.c b/builtin/help.c
>> index 1fdefeb..d1dfecd 100644
>> --- a/builtin/help.c
>> +++ b/builtin/help.c
>> @@ -38,7 +38,7 @@ static int show_guides = 0;
>> static unsigned int colopts;
>> static enum help_format help_format = HELP_FORMAT_NONE;
>> static struct option builtin_help_options[] = {
>> - OPT_BOOL('a', "all", &show_all, N_("print all available commands")),
>> + OPT_BOOL('a', "all", &show_all, N_("print all available commands and
>> aliases")),
>> OPT_BOOL('g', "guides", &show_guides, N_("print list of useful
>> guides")),
>> OPT_SET_INT('m', "man", &help_format, N_("show man page"),
>> HELP_FORMAT_MAN),
>> OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
>> @@ -453,6 +453,7 @@ int cmd_help(int argc, const char **argv, const char
>> *prefix)
>> int nongit;
>> const char *alias;
>> enum help_format parsed_help_format;
>> + struct cmdnames alias_cmds;
>>
>> argc = parse_options(argc, argv, prefix, builtin_help_options,
>> builtin_help_usage, 0);
>> @@ -461,8 +462,8 @@ int cmd_help(int argc, const char **argv, const char
>> *prefix)
>> if (show_all) {
>> git_config(git_help_config, NULL);
>> printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
>> - load_command_list("git-", &main_cmds, &other_cmds);
>> - list_commands(colopts, &main_cmds, &other_cmds);
>> + load_commands_and_aliases("git-", &main_cmds, &other_cmds,
>> &alias_cmds);
>> + list_commands(colopts, &main_cmds, &other_cmds, &alias_cmds);
>> }
>>
>> if (show_guides)
>> diff --git a/help.c b/help.c
>> index df7d16d..3c14af4 100644
>> --- a/help.c
>> +++ b/help.c
>> @@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
>> int i;
>>
>> for (i = 0; i < cmds->cnt; i++)
>> - string_list_append(&list, cmds->names[i]->name);
>> + string_list_append(&list, cmds->names[i]->name);
>> /*
>>* always enable column display, we only consult column.*
>>* about layout strategy and stuff
>> @@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
>> exclude_cmds(other_cmds, main_cmds);
>> }
>>
>> +static struct cmdnames aliases;
>> +
>> +static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
>> +{
>> + int i;
>> + ALLOC_GROW(cmds->names, cmds->cnt + old->cnt, cmds->alloc);
>> +
>> + for (i = 0; i < old->cnt; i++)
>> + cmds->names[cmds->cnt++] = old->names[i];
>> + free(old->names);
>> + old->cnt = 0;
>> + old->names = NULL;
>> +}
>> +
>> +static int load_aliases_cb(const char *var, const char *value, void *cb)
>> +{
>> + if (starts_with(var, "alias."))
>> + add_cmdname(&aliases, var + 6, strlen(var + 6));
>> + return 0;
>> +}
>> +
>> +void load_commands_an
Re: `git stash pop` UX Problem
Junio C Hamano writes: > "status" is about reminding the user what changes are already in the > index (i.e. what you would commit) and what changes are in the > working tree, from which you could further update the index with > (i.e. what you could commit). I believe "status" should tell me everything git knows about the current workspace in a resonably concise way. That includes the stash. > One _could_ argue that stashed changes are what could be reflected > to the working tree and form the source of the latter, but my gut > feeling is that it is a rather weak argument. At that point you are > talking about what you could potentially change in the working tree, No, I saved things in the stash on purpose. For example, I had changes that were not ready to commit, but I wanted to do a merge from upstream. There are workflows where the stash is not important; provide an option to 'git status' that means "ignore stash". > So, I tend to agree with you, while I do understand where "I want to > know about what is in stash" is coming from (and that is why we do > have "git stash list" command). My Emacs front end currently checks both 'git status' and 'git stash list' to build "the status of the current workspace". -- -- Stephe -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Matthieu Moy writes: > Omar Othman writes: > >> [omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt >> [omar_othman main (trunk*)]$ >> >> Note how the status message has changed to show that git is now happy. >> It is at that moment that the stash reference should be dropped > > Dropping the stash on a "git add" operation would be really, really > weird... Why? That is when the merge conflicts are resolved, which is what logically indicates that the stash is no longer needed, _if_ the merge conflicts are related to the stash, which is true in this use case. There are other uses for 'git add' that don't indicate that; we'd have to be very careful to not throw away the stash at the wrong time. >> (or the user (somehow) is notified to do that herself if desired), >> because this means that the popping operation has succeeded. > > But how would you expect to "be notified"? When 'git add' checks to see if all merge conflicts are now resolved, and those merge conflicts were related to the stash, it can either pop the stash, or issue a message telling the user it is now safe to do so. We would need a config setting to indicate which to do. Maybe that check is hard to do in general? -- -- Stephe -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] difftool: support repositories with .git-files
Am 25.02.2014 18:02, schrieb Junio C Hamano: > Jens Lehmann writes: > >> Am 24.02.2014 17:55, schrieb Junio C Hamano: >>> David Aguilar writes: >>> Modern versions of "git submodule" use .git-files to setup the submodule directory. When run in a "git submodule"-created repository "git difftool --dir-diff" dies with the following error: $ git difftool -d HEAD~ fatal: This operation must be run in a work tree diff --raw --no-abbrev -z HEAD~: command returned error: 128 core.worktree is relative to the .git directory but the logic in find_worktree() does not account for it. Use `git rev-parse --show-toplevel` to find the worktree so that the dir-diff feature works inside a submodule. Reported-by: Gábor Lipták Helped-by: Jens Lehmann Helped-by: John Keeping Signed-off-by: David Aguilar --- >>> >>> Looks good; thanks. >> >> >> FWIW: >> Tested-by: Jens Lehmann >> >> What about squashing this in to detect any future regressions? >> >> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh >> index 2418528..d86ad68 100755 >> --- a/t/t7800-difftool.sh >> +++ b/t/t7800-difftool.sh >> @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks >> detects conflict ' ' >> ) >> ' >> >> +test_expect_success PERL 'difftool properly honours gitlink and >> core.worktree' ' >> +git submodule add ./. submod/ule && >> +( >> +cd submod/ule && >> +git difftool --tool=echo --dir-diff --cached > > In the context of this fix, finishing with 0 exit status may be all > we care about, but do we also care about things like in what > directory the tool is invoked in, what arguments and extra > environment settings (if any) it is given, and stuff like that? Sure. But I just intended to test the fix (and the test can easily be extended by people who know more about difftool than I do). > In fact, the "echo" in the above is very misleading. The test > relies on the fact that immediately after the submod/ule is cloned, > "diff --cached" does not have to call any tool backend---if you > modify some tracked file in its working tree and dropped --cached > on the command line, the command will fail with "Huh? I do not know > what 'echo' diff/merge backend is", no? Right, using echo was not the best choice here. I used it to avoid the dependency to meld in the example of the OP (maybe using "true" as tool would have indicated that the tool is not important here, but looking into this again a simple "git difftool --dir-diff" without any further arguments also shows that the fix is working). Aas mentioned above, I'm not familiar with difftool and just wanted to share an easy way to test the fix. But I do not care too deeply about this test, so feel free to ignore it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] help: include list of aliases in git-help --all
Joel Nothman writes:
> Git help --all had listed all git commands, but no configured aliases.
> This includes aliases as a separate listing, after commands in the main
> git directory and other $PATH directories.
... and why is this a good thing?
>
> Signed-off-by: Joel Nothman gmail.com>
> ---
> Documentation/git-help.txt | 4 +--
> builtin/help.c | 7 ++---
> help.c | 64
> +++---
> help.h | 7 -
> 4 files changed, 61 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
> index b21e9d7..c9fe791 100644
> --- a/Documentation/git-help.txt
> +++ b/Documentation/git-help.txt
> @@ -40,8 +40,8 @@ OPTIONS
> ---
> -a::
> --all::
> - Prints all the available commands on the standard output. This
> - option overrides any given command or guide name.
> + Prints all the available commands and aliases on the standard output.
> + This option overrides any given command or guide name.
>
> -g::
> --guides::
> diff --git a/builtin/help.c b/builtin/help.c
> index 1fdefeb..d1dfecd 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -38,7 +38,7 @@ static int show_guides = 0;
> static unsigned int colopts;
> static enum help_format help_format = HELP_FORMAT_NONE;
> static struct option builtin_help_options[] = {
> - OPT_BOOL('a', "all", &show_all, N_("print all available commands")),
> + OPT_BOOL('a', "all", &show_all, N_("print all available commands and
> aliases")),
> OPT_BOOL('g', "guides", &show_guides, N_("print list of useful
> guides")),
> OPT_SET_INT('m', "man", &help_format, N_("show man page"),
> HELP_FORMAT_MAN),
> OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
> @@ -453,6 +453,7 @@ int cmd_help(int argc, const char **argv, const char
> *prefix)
> int nongit;
> const char *alias;
> enum help_format parsed_help_format;
> + struct cmdnames alias_cmds;
>
> argc = parse_options(argc, argv, prefix, builtin_help_options,
> builtin_help_usage, 0);
> @@ -461,8 +462,8 @@ int cmd_help(int argc, const char **argv, const char
> *prefix)
> if (show_all) {
> git_config(git_help_config, NULL);
> printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
> - load_command_list("git-", &main_cmds, &other_cmds);
> - list_commands(colopts, &main_cmds, &other_cmds);
> + load_commands_and_aliases("git-", &main_cmds, &other_cmds,
> &alias_cmds);
> + list_commands(colopts, &main_cmds, &other_cmds, &alias_cmds);
> }
>
> if (show_guides)
> diff --git a/help.c b/help.c
> index df7d16d..3c14af4 100644
> --- a/help.c
> +++ b/help.c
> @@ -86,7 +86,7 @@ static void pretty_print_string_list(struct cmdnames *cmds,
> int i;
>
> for (i = 0; i < cmds->cnt; i++)
> - string_list_append(&list, cmds->names[i]->name);
> + string_list_append(&list, cmds->names[i]->name);
> /*
>* always enable column display, we only consult column.*
>* about layout strategy and stuff
> @@ -202,8 +202,48 @@ void load_command_list(const char *prefix,
> exclude_cmds(other_cmds, main_cmds);
> }
>
> +static struct cmdnames aliases;
> +
> +static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
> +{
> + int i;
> + ALLOC_GROW(cmds->names, cmds->cnt + old->cnt, cmds->alloc);
> +
> + for (i = 0; i < old->cnt; i++)
> + cmds->names[cmds->cnt++] = old->names[i];
> + free(old->names);
> + old->cnt = 0;
> + old->names = NULL;
> +}
> +
> +static int load_aliases_cb(const char *var, const char *value, void *cb)
> +{
> + if (starts_with(var, "alias."))
> + add_cmdname(&aliases, var + 6, strlen(var + 6));
> + return 0;
> +}
> +
> +void load_commands_and_aliases(const char *prefix,
> + struct cmdnames *main_cmds,
> + struct cmdnames *other_cmds,
> + struct cmdnames *alias_cmds)
> +{
> + load_command_list(prefix, main_cmds, other_cmds);
> +
> + /* reuses global aliases from unknown command functionality */
> + git_config(load_aliases_cb, NULL);
> +
> + add_cmd_list(alias_cmds, &aliases);
> + qsort(alias_cmds->names, alias_cmds->cnt,
> + sizeof(*alias_cmds->names), cmdname_compare);
> + uniq(alias_cmds);
> + exclude_cmds(alias_cmds, main_cmds);
> + exclude_cmds(alias_cmds, other_cmds);
> +}
> +
> void list_commands(unsigned int colopts,
> -struct cmdnames *main_cmds, struct cmdnames *other_cmds)
> +struct cmdnames *main_cmds, struct cmdnames *other_cmds,
> +struct cmdnames *alias_cmds)
> {
> if (main_cmds->cnt) {
> const char *exec_path = git_exec_path();
> @@ -219,6 +259,13 @@ void list_commands(unsigned int colopts,
>
Re: `git stash pop` UX Problem
Matthieu Moy writes: > Holger Hellmuth writes: > >> Am 24.02.2014 17:21, schrieb Matthieu Moy: >>> $ git add foo.txt >>> $ git status >>> On branch master >>> Changes to be committed: >>>(use "git reset HEAD ..." to unstage) >>> >>> modified: foo.txt >> >> Maybe status should display a stash count if that count is > 0, as >> this is part of the state of the repo. > > Maybe it would help some users, but not me for example. My main use of > "git stash" is a safe replacement for "git reset --hard": when I want to > discard changes, but keep them safe just in case. > > So, my stash count is almost always >0, and I don't want to hear about > it. "status" is about reminding the user what changes are already in the index (i.e. what you would commit) and what changes are in the working tree, from which you could further update the index with (i.e. what you could commit). One _could_ argue that stashed changes are what could be reflected to the working tree and form the source of the latter, but my gut feeling is that it is a rather weak argument. At that point you are talking about what you could potentially change in the working tree, and the way to do so is not limited to "stash pop" (i.e. you can "git cherry-pick --no-commit $a_commit", or "edit" any file in the working tree for that matter, with the same ease). So, I tend to agree with you, while I do understand where "I want to know about what is in stash" is coming from (and that is why we do have "git stash list" 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
Re: [PATCH] difftool: support repositories with .git-files
Jens Lehmann writes: > Am 24.02.2014 17:55, schrieb Junio C Hamano: >> David Aguilar writes: >> >>> Modern versions of "git submodule" use .git-files to setup the >>> submodule directory. When run in a "git submodule"-created >>> repository "git difftool --dir-diff" dies with the following >>> error: >>> >>> $ git difftool -d HEAD~ >>> fatal: This operation must be run in a work tree >>> diff --raw --no-abbrev -z HEAD~: command returned error: 128 >>> >>> core.worktree is relative to the .git directory but the logic >>> in find_worktree() does not account for it. >>> >>> Use `git rev-parse --show-toplevel` to find the worktree so that >>> the dir-diff feature works inside a submodule. >>> >>> Reported-by: Gábor Lipták >>> Helped-by: Jens Lehmann >>> Helped-by: John Keeping >>> Signed-off-by: David Aguilar >>> --- >> >> Looks good; thanks. > > > FWIW: > Tested-by: Jens Lehmann > > What about squashing this in to detect any future regressions? > > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh > index 2418528..d86ad68 100755 > --- a/t/t7800-difftool.sh > +++ b/t/t7800-difftool.sh > @@ -434,4 +434,12 @@ test_expect_success PERL 'difftool --no-symlinks detects > conflict ' ' > ) > ' > > +test_expect_success PERL 'difftool properly honours gitlink and > core.worktree' ' > + git submodule add ./. submod/ule && > + ( > + cd submod/ule && > + git difftool --tool=echo --dir-diff --cached In the context of this fix, finishing with 0 exit status may be all we care about, but do we also care about things like in what directory the tool is invoked in, what arguments and extra environment settings (if any) it is given, and stuff like that? In fact, the "echo" in the above is very misleading. The test relies on the fact that immediately after the submod/ule is cloned, "diff --cached" does not have to call any tool backend---if you modify some tracked file in its working tree and dropped --cached on the command line, the command will fail with "Huh? I do not know what 'echo' diff/merge backend is", no? > + ) > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] builtin/blame.c::find_copy_in_blob: no need to scan for region end
David Kastrup writes:
> The region end can be looked up just like its beginning.
>
> Signed-off-by: David Kastrup
> ---
> builtin/blame.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
Yay, code reduction! Thanks.
> diff --git a/builtin/blame.c b/builtin/blame.c
> index e44a6bb..96716dd 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -939,7 +939,6 @@ static void find_copy_in_blob(struct scoreboard *sb,
> mmfile_t *file_p)
> {
> const char *cp;
> - int cnt;
> mmfile_t file_o;
> struct handle_split_cb_data d;
>
> @@ -950,13 +949,7 @@ static void find_copy_in_blob(struct scoreboard *sb,
>*/
> cp = nth_line(sb, ent->lno);
> file_o.ptr = (char *) cp;
> - cnt = ent->num_lines;
> -
> - while (cnt && cp < sb->final_buf + sb->final_buf_size) {
> - if (*cp++ == '\n')
> - cnt--;
> - }
> - file_o.size = cp - file_o.ptr;
> + file_o.size = nth_line(sb, ent->lno + ent->num_lines) - cp;
>
> /*
>* file_o is a part of final image we are annotating.
--
To unsubscribe from this list: send the line "unsubscribe 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] tag: support --sort=
Nguyễn Thái Ngọc Duy writes: > versioncmp() is copied from string/strverscmp.c in glibc commit > ee9247c38a8def24a59eb5cfb7196a98bef8cfdc, reformatted to Git coding > style. The implementation is under LGPL-2.1 and according to [1] I can > relicense it to GPLv2. I'd propose this trivial change squashed in to record the above in the file in question. Thanks. diff --git a/versioncmp.c b/versioncmp.c index 18a9632..8f5a388 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -1,6 +1,13 @@ #include "git-compat-util.h" /* + * versioncmp(): copied from string/strverscmp.c in glibc commit + * ee9247c38a8def24a59eb5cfb7196a98bef8cfdc, reformatted to Git coding + * style. The implementation is under LGPL-2.1 and Git relicenses it + * to GPLv2. + */ + +/* * states: S_N: normal, S_I: comparing integral part, S_F: comparing * fractionnal parts, S_Z: idem but with leading Zeroes only */ -- To unsubscribe from this list: send the line "unsubscribe 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 in GSoC 2014
On 02/25/2014 04:41 PM, Jeff King wrote: > I'm pleased to announce that Git has been accepted to this year's Google > Summer of Code. Cool! Thanks to Peff and Thomas and Vicent and whomever else was involved in getting our application done! For those who don't know, the application covers both Git core and libgit2. > We didn't discuss earlier whether we would have any specific > requirements for students during the proposal period (e.g., having a > patch accepted). It would be good to put together rules (or barring any > specific requirements, guidelines to help students put together a good > proposal) as soon as possible. Suggestions are welcome. Requiring students to submit a reasonable patch and follow up on review comments seems like it would be a good way to filter out non-serious students. (I hesitate to require that the patch be accepted because it can take quite a while for a patch to make it to master, despite of the student's efforts.) Does anybody know whether other organizations have had good experience with criteria like that? Does it chase *all* the applicants away? If we wanted to impose such a hurdle, then we would definitely have to make up a list of microprojects so that the students don't have to start from nothing. I imagine it shouldn't be too hard to find tiny projects estimated at 10-30 minutes of actual work, which should be plenty difficult for a student who also has to figure out how to check out the code, conform to our coding standards, run the unit tests, create a patch submission, etc. If the reaction is positive to this idea then I volunteer to spend several hours tomorrow looking for microprojects, and I suggest other core developers do so as well. They should presumably be submitted as patches to the ideas repository [1]. What do you think? Michael [1] https://github.com/git/git.github.io -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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 in GSoC 2014
Hi. I was just going to write an email about that I would like to participate in GSoC and contribute to Git project. I don't have wide experience in C programming, but I could be start as a "janitor". I found several tasks here https://git.wiki.kernel.org/index.php/Janitor. For example "Refactor binary search functions". If this task is actual, I could be to start from it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git in GSoC 2014
I'm pleased to announce that Git has been accepted to this year's Google Summer of Code. Student proposals will start coming in on March 22. In the meantime students will be reading our Ideas page[1] and enquiring about the program on the mailing list and on irc. There are many ways that existing git developers can help: - continue to add to and polish the Ideas page - interact with prospective students; besides responding to questions on the list, hanging around on #git and #git-devel on freenode is helpful - volunteer to mentor and/or rank student proposals. You can sign up at http://google-melange.com, and request to be added as a mentor for the git project. We didn't discuss earlier whether we would have any specific requirements for students during the proposal period (e.g., having a patch accepted). It would be good to put together rules (or barring any specific requirements, guidelines to help students put together a good proposal) as soon as possible. Suggestions are welcome. -Peff [1] http://git.github.io/SoC-2014-Ideas.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Can't use difftool to selectively revert changes
I have a branch called topic1 that is based on 'master'. For a particular file in my topic branch, I want to revert some changes by using my diff tool. I do this by comparing the original revision of the file with HEAD like so: $ git difftool $(git merge-base topic1 master) HEAD -- Path/SourceFile.cpp When I make changes to the right side (HEAD) through my diff tool, and exit, the changes aren't picked up and applied to my working copy. Since I'm modifying HEAD, I think it should carry over the changes I make. Is this by design or a defect? I don't know how else to selectively revert changes to a file through my diff viewer. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can't use difftool to selectively revert changes
Sorry I'm going to go ahead and answer my own question: $ git difftool $(git merge-base topic1 master) -- Path/SourceFile.cpp I removed 'HEAD' from the command and now it picks up my changes and compares to my working copy version (which is actually what I wanted). I thought HEAD would point to my working copy version but that's wrong. Sorry for the superfluous post! On Tue, Feb 25, 2014 at 9:22 AM, Robert Dailey wrote: > I have a branch called topic1 that is based on 'master'. For a > particular file in my topic branch, I want to revert some changes by > using my diff tool. I do this by comparing the original revision of > the file with HEAD like so: > > $ git difftool $(git merge-base topic1 master) HEAD -- Path/SourceFile.cpp > > When I make changes to the right side (HEAD) through my diff tool, and > exit, the changes aren't picked up and applied to my working copy. > Since I'm modifying HEAD, I think it should carry over the changes I > make. Is this by design or a defect? I don't know how else to > selectively revert changes to a file through my diff viewer. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Omar Othman writes: > [omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt > [omar_othman main (trunk*)]$ > > Note how the status message has changed to show that git is now happy. > It is at that moment that the stash reference should be dropped Dropping the stash on a "git add" operation would be really, really weird... > (or the user (somehow) is notified to do that herself if desired), > because this means that the popping operation has succeeded. But how would you expect to "be notified"? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] Document a bunch of functions defined in sha1_file.c
On 02/24/2014 09:08 PM, Jonathan Nieder wrote: > Michael Haggerty wrote: >> [...] I've been documenting public functions in the >> header files above the declaration, and private ones where they are >> defined. [...] >> >> Let me know if you think I've made it less helpful. > > In the present state of the codebase, where many important functions > have no documentation or out-of-date documentation, the first place I > look to understand a function is its point of definition. So I do > think that moving docs to the header file makes it less helpful. I'd > prefer a mass move in the opposite direction (from header files to the > point of definition). > > On the other hand I don't feel strongly about it. Jonathan, I see your point. But I'd rather that we, as a project, strive to make our header files good tables of contents of the publicly-accessible functionality, including decent documentation for each function. I try to add comments to everything I touch, and I wish other developers would too. [What we really need is a comment fascist who patrols patch submissions making sure that they add docstrings for new functions. If I only had the time and the jackboots for it...] So I'd rather leave the comments for public functions in the header files. But if other regular developers prefer that comments be by the function definitions, of course I can live with that, too. 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: `git stash pop` UX Problem
Please note that what I am asking for is not always dropping the stash, but doing that *only* when the merge conflict is resolved. This is simply getting the whole command to be consistent. If you do `git stash pop` and it succeeds, the stash reference is dropped. If you do git stash pop` and it succeeds *after resolving the merge conflict*, the stash reference is *not* dropped. This is *not* consistent and *is* a user experience problem. I'm not asking about dumbing git down by any means. Can you describe precisely what you would expect, e.g. what Git's output should look like after such and such command? Sure. This is my current command prompt (which shows git's internal status): [omar_othman main (trunk*)]$ I do a git stash pop, which causes a merge conflict: Auto-merging path/to/file.txt CONFLICT (content): Merge conflict in path/to/file.txt [omar_othman main (trunk|MERGING*)]$ vi path/to/file.txt [omar_othman main (trunk|MERGING*)]$ git add path/to/file.txt [omar_othman main (trunk*)]$ Note how the status message has changed to show that git is now happy. It is at that moment that the stash reference should be dropped (or the user (somehow) is notified to do that herself if desired), because this means that the popping operation has succeeded. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Omar Othman writes: > Brandon: Please, don't top-post on this list. Look how other people answer to each other and follow the use. > Please note that what I am asking for is not always dropping the > stash, but doing that *only* when the merge conflict is resolved. This > is simply getting the whole command to be consistent. If you do `git > stash pop` and it succeeds, the stash reference is dropped. If you do > git stash pop` and it succeeds *after resolving the merge conflict*, > the stash reference is *not* dropped. This is *not* consistent and > *is* a user experience problem. I'm not asking about dumbing git down > by any means. Can you describe precisely what you would expect, e.g. what Git's output should look like after such and such command? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Brandon: Please note that what I am asking for is not always dropping the stash, but doing that *only* when the merge conflict is resolved. This is simply getting the whole command to be consistent. If you do `git stash pop` and it succeeds, the stash reference is dropped. If you do `git stash pop` and it succeeds *after resolving the merge conflict*, the stash reference is *not* dropped. This is *not* consistent and *is* a user experience problem. I'm not asking about dumbing git down by any means. On 24-02-14 17:04, Brandon McCaig wrote: Omar: On Mon, Feb 24, 2014 at 3:32 AM, Omar Othman wrote: In general, whenever something a user "should" do, git always tells. So, for example, when things go wrong with a merge, you have the option to abort. When you are doing a rebase, git tells you to do git commit --amend, and then git rebase --continue... and so on. The point is: Because of this, git is expected to always instruct you on what to do next in a multilevel operation, or instructing you what to do when an operation has gone wrong. Now comes the problem. When you do a git stash pop, and a merge conflict happens, git correctly tells you to fix the problems and then git add to resolve the conflict. But once that happens, and the internal status of git tells you that there are no more problems (I have a prompt that tells me git's internal status), the operation is not culminated by dropping the stash reference, which what normally happens automatically after a git stash pop. This has actually confused me for a lot of time, till I ran into a git committer and asked him, and only then were I 100% confident that I did nothing wrong and it is indeed a UX problem. I wasted a lot of time to know why the operation is not completed as expected (since I trusted that git just does the right thing), and it turned out that it is git's fault. If this is accepted, please reply to this email and tell me to start working on it. I've read the Documenation/SubmittingPatches guidelines, but I'll appreciate also telling me where to base my change. My guess is maint, since it's a "bug" in the sense of UX. Unlike a merge, when you pop a stash that history is lost. If you screw up the merge and the stash is dropped then there's generally no reliable way to get it back. I think that it's correct behavior for the stash to not be dropped if the merge conflicts. The user is expected to manually drop the stash when they're done with it. It's been a while since I've relied much on the stash (commits and branches are more powerful to work with) so I'm not really familiar with what help the UI gives when a conflict occurs now. Git's UI never really expects the user to be negligent. It does help to hint to you what is needed, but for the most part it still expects you to know what you're doing and does what you say, not what you mean. If there's any change that should be made it should be purely providing more detailed instructions to the user about how to deal with it. Either resolve the merge conflicts and git-add the conflicting files, or use git-reset to either reset the index (unstaging files nad clear) or reset index and working tree back to HEAD. In general, I almost always git-reset after a git-stash pop because I'm probably not ready to commit those changes yet and generally want to still see those changes with git diff (without --staged). Or perhaps just direct them to the appropriate sections of the man pages. I'm not really in favor of "dumbing down" Git in any way and I think that any step in that direction would be for the worst... Software should do what you say, not what you mean, because it's impossible to reliably guess what you meant. When a git-stash pop operation fails that might make the user rethink popping that stash. That's why it becomes a manual operation to drop it if still desired. And unlike git-reset --continue, which is explicitly the user saying "it is fixed and I accept the consequences, let's move on", there is no such option to git-stash to acknowledge that the merge conflicts have been resolved and you no longer need that stash (aside from git-stash drop, of course). It's not a UI problem. It's maybe a documentation problem, but again I'm not familiar with the current state of that. /not a git dev...yet Regards, -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Well, it's called `git stash` and not `git trash`... :-D That's your own usage of it, but its main usage is different. This is not a solution, but it's better than nothing and I second it. On 25-02-14 13:33, Matthieu Moy wrote: Holger Hellmuth writes: Am 24.02.2014 17:21, schrieb Matthieu Moy: $ git add foo.txt $ git status On branch master Changes to be committed: (use "git reset HEAD ..." to unstage) modified: foo.txt Maybe status should display a stash count if that count is > 0, as this is part of the state of the repo. Maybe it would help some users, but not me for example. My main use of "git stash" is a safe replacement for "git reset --hard": when I want to discard changes, but keep them safe just in case. So, my stash count is almost always >0, and I don't want to hear about it. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Holger Hellmuth writes: > Am 24.02.2014 17:21, schrieb Matthieu Moy: >> $ git add foo.txt >> $ git status >> On branch master >> Changes to be committed: >>(use "git reset HEAD ..." to unstage) >> >> modified: foo.txt > > Maybe status should display a stash count if that count is > 0, as > this is part of the state of the repo. Maybe it would help some users, but not me for example. My main use of "git stash" is a safe replacement for "git reset --hard": when I want to discard changes, but keep them safe just in case. So, my stash count is almost always >0, and I don't want to hear about it. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git stash pop` UX Problem
Am 24.02.2014 17:21, schrieb Matthieu Moy: $ git add foo.txt $ git status On branch master Changes to be committed: (use "git reset HEAD ..." to unstage) modified: foo.txt Maybe status should display a stash count if that count is > 0, as this is part of the state of the repo. $ git status On branch master Stashes: 1 <-- Changes to be committed: (use "git reset HEAD ..." to unstage) modified: foo.txt It would be in Omars example case a clear message that git kept the stash. And generally a reminder that there is still a stash around that might or might not be obsolete. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] tag: support --sort=
--sort=version:refname (or --sort=v:refname for short) sorts tags as
if they are versions. --sort=-refname reverses the order (with or
without ":version").
versioncmp() is copied from string/strverscmp.c in glibc commit
ee9247c38a8def24a59eb5cfb7196a98bef8cfdc, reformatted to Git coding
style. The implementation is under LGPL-2.1 and according to [1] I can
relicense it to GPLv2.
[1] http://www.gnu.org/licenses/gpl-faq.html#AllCompatibility
Signed-off-by: Nguyễn Thái Ngọc Duy
---
v3 tweaks the sorting syntax a bit (version:refname vs refname:version)
and embed strverscmp() to git code (instead of relying on one from glibc).
Documentation/git-tag.txt | 6
Makefile | 1 +
builtin/tag.c | 68 ++---
cache.h | 2 ++
t/t7004-tag.sh| 43
versioncmp.c (new)| 85 +++
6 files changed, 200 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 404257d..9b05931 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -95,6 +95,12 @@ OPTIONS
using fnmatch(3)). Multiple patterns may be given; if any of
them matches, the tag is shown.
+--sort=::
+ Sort in a specific order. Supported type is "refname"
+ (lexicographic order), "version:refname" or "v:refname" (tags
+ name are treated as versions). Prepend "-" to reverse sort
+ order.
+
--column[=]::
--no-column::
Display tag listing in columns. See configuration variable
diff --git a/Makefile b/Makefile
index dddaf4f..16b00a5 100644
--- a/Makefile
+++ b/Makefile
@@ -884,6 +884,7 @@ LIB_OBJS += userdiff.o
LIB_OBJS += utf8.o
LIB_OBJS += varint.o
LIB_OBJS += version.o
+LIB_OBJS += versioncmp.o
LIB_OBJS += walker.o
LIB_OBJS += wildmatch.o
LIB_OBJS += wrapper.o
diff --git a/builtin/tag.c b/builtin/tag.c
index 74d3780..73b8a24 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -27,9 +27,16 @@ static const char * const git_tag_usage[] = {
NULL
};
+#define STRCMP_SORT 0 /* must be zero */
+#define VERCMP_SORT 1
+#define SORT_MASK 0x7fff
+#define REVERSE_SORT0x8000
+
struct tag_filter {
const char **patterns;
int lines;
+ int sort;
+ struct string_list tags;
struct commit_list *with_commit;
};
@@ -166,7 +173,10 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
return 0;
if (!filter->lines) {
- printf("%s\n", refname);
+ if (filter->sort)
+ string_list_append(&filter->tags, refname);
+ else
+ printf("%s\n", refname);
return 0;
}
printf("%-15s ", refname);
@@ -177,17 +187,39 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
return 0;
}
+static int sort_by_version(const void *a_, const void *b_)
+{
+ const struct string_list_item *a = a_;
+ const struct string_list_item *b = b_;
+ return versioncmp(a->string, b->string);
+}
+
static int list_tags(const char **patterns, int lines,
- struct commit_list *with_commit)
+struct commit_list *with_commit, int sort)
{
struct tag_filter filter;
filter.patterns = patterns;
filter.lines = lines;
+ filter.sort = sort;
filter.with_commit = with_commit;
+ memset(&filter.tags, 0, sizeof(filter.tags));
+ filter.tags.strdup_strings = 1;
for_each_tag_ref(show_reference, (void *) &filter);
-
+ if (sort) {
+ int i;
+ if ((sort & SORT_MASK) == VERCMP_SORT)
+ qsort(filter.tags.items, filter.tags.nr,
+ sizeof(struct string_list_item), sort_by_version);
+ if (sort & REVERSE_SORT)
+ for (i = filter.tags.nr - 1; i >= 0; i--)
+ printf("%s\n", filter.tags.items[i].string);
+ else
+ for (i = 0; i < filter.tags.nr; i++)
+ printf("%s\n", filter.tags.items[i].string);
+ string_list_clear(&filter.tags, 0);
+ }
return 0;
}
@@ -427,6 +459,26 @@ static int parse_opt_points_at(const struct option *opt
__attribute__((unused)),
return 0;
}
+static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
+{
+ int *sort = opt->value;
+ if (*arg == '-') {
+ *sort = REVERSE_SORT;
+ arg++;
+ } else
+ *sort = STRCMP_SORT;
+ if (starts_with(arg, "version:")) {
+ *sort |= VERCMP_SORT;
+ arg += 8;
+ } else if (starts_with(arg, "v:"))
git submodule manpage does not document --checkout
Hi, it seems git submodule supports --checkout, which is also mentioned indirectly in the manpage. However, the option itself is not mentioned in the synopsis or detailed option list. Gr. Matthijs signature.asc Description: Digital signature
Re: [PATCH v2 00/19] Multiparent diff tree-walker + combine-diff speedup
On Tue, Feb 25, 2014 at 06:43:24AM +0700, Duy Nguyen wrote:
> On Mon, Feb 24, 2014 at 11:21 PM, Kirill Smelkov wrote:
> > Hello up there.
> >
> > Here go combine-diff speedup patches in form of first reworking diff
> > tree-walker to work in general case - when a commit have several parents,
> > not
> > only one - we are traversing all 1+nparent trees in parallel.
> >
> > Then we are taking advantage of the new diff tree-walker for speeding up
> > combine-diff, which for linux.git results in ~14 times speedup.
>
> I think there is another use case for this n-tree walker (but I'm not
> entirely sure yet as I haven't really read the series). In git-log
> (either with pathspec or --patch) we basically do this
>
> diff HEAD^ HEAD
> diff HEAD^^ HEAD^
> diff HEAD^^^ HEAD^^
> diff HEAD HEAD^^^
> ...
>
> so except HEAD (and the last commit), all commits' tree will be
> read/diff'd twice. With n-tree walker I think we may be able to diff
> them in batch to reduce extra processing: commit lists are split into
> 16-commit blocks where 16 trees are fed to the new tree walker at the
> same time. I hope it would make git-log a bit faster (especially for
> -S). Maybe not much.
Thanks for commenting.
Unfortunately, as it is now, no, and I doubt savings will be
significant. The real speedup comes from the fact that for combined
diff, we can omit recursing into subdirectories, if we know some diff
D(commit,parent_i) is empty. Let me quote myself from
http://article.gmane.org/gmane.comp.version-control.git/242217
On Sun, Feb 16, 2014 at 12:08:29PM +0400, Kirill Smelkov wrote:
> On Fri, Feb 14, 2014 at 09:37:00AM -0800, Junio C Hamano wrote:
> > I wonder if this machinery can be reused for "log -m" as well (or
> > perhaps you do that already?). After all, by performing a single
> > parallel scan, you are gathering all the necessary information to
> > let you pretend that you did N pairwise diff-tree.
>
> Unfortunately, as it is now, no, and let me explain why:
>
> The reason that is not true, is that we omit recursing into directories,
> if we know D(A,some-parent) for that path is empty. That means we don't
> calculate D(A,any-other-parents) for that path and subpaths.
>
> More structured description is that combined diff and "log -m", which
> could be though as all diffs D(A,Pi) are different things:
>
> - the combined diff is D(A,B) generalization based on "^" (sets
> intersection) operator, and
>
> - log -m, aka "all diffs" is D(A,B) generalization based on "v"
> (sets union) operator.
>
> Intersection means, we can omit calculating parts from other sets, if we
> know some set does not have an element (remember "don't recurse into
> subdirectories"?), and unioning does not have this property.
>
> It does so happen, that "^" case (combine-diff) is more interesting,
> because in the end it allows to see new information - the diff a merge
> itself introduces. "log -m" does not have this property and is no more
> interesting to what plain diff(HEAD,HEAD^n) can provide - in other words
> it's just a convenience.
>
> Now, the diff tree-walker could be generalized once more, to allow
> clients specify, which diffs combination operator to use - intersection
> or unioning, but I doubt that for unioning case that would add
> significant speedup - we can't reduce any diff generation based on
> another diff and the only saving is that we traverse resulting commit
> tree once, but for some cases that could be maybe slower, say if result
> and some parents don't have a path and some parent does, we'll be
> recursing into that path and do more work compared to plain D(A,Pi) for
> Pi that lacks the path.
>
> In short: it could be generalized more, if needed, but I propose we
> first establish the ground with generalizing to just combine-diff.
besides
D(HEAD~, HEAD)
D(HEAD~2, HEAD~)
...
D(HEAD~{n}, HEAD~{n-1})
is different even from "log -m" case as now there is no single commit
with several parents.
On a related note, while developing this n-tree walker, I've learned
that it is important to load trees in correct order. Quoting patch 18:
- t1tree = fill_tree_descriptor(&t1, old);
- t2tree = fill_tree_descriptor(&t2, new);
+ /*
+* load parents first, as they are probably already cached.
+*
+* ( log_tree_diff() parses commit->parent before calling here via
+* diff_tree_sha1(parent, commit) )
+*/
+ for (i = 0; i < nparent; ++i)
+ tptree[i] = fill_tree_descriptor(&tp[i], parents_sha1[i]);
+ ttree = fill_tree_descriptor(&t, sha1);
so it loads parent's tree first. If we change this to be the other way,
i.e. load commit's tree first, and then parent's tree, there will be up
to 4% slowdown for whole plain `git log` (without -c).
So maybe what could be done to speedup plain log is for diff tree-walker
to populate some form of recently-loaded trees while walking, and drop
trees from will not-be use
