git blame --reverse doesn't find line in HEAD
I have a repo that reproduces a behavior with `git blame --reverse` that surprises me. https://github.com/nicksnyder/git-blame-bug The readme explains the observed behavior and what I expected to happen. I will inline the readme at the bottom of this message for convenience. Am I misunderstanding --reverse or is this a bug? Thanks! Nick $ git --version git version 2.15.0 Blame of L465 in Tree.tsx at HEAD (ca0fb5) points to L463 at 199ee7 $ git blame -p -L465,465 Tree.tsx 199ee75d1240ae72cd965f62aceeb301ab64e1bd 463 465 1 filename Tree.tsx public shouldComponentUpdate(nextProps: TileProps): boolean { EXPECTED: Reverse blame of L463 at 199ee7 points to L465 at the lastest commit in the repo (at least ca0fb5). ACTUAL: Reverse blame of L463 at 199ee7 points to L463 at 199ee7. $ git blame -p -L463,463 --reverse 199ee7.. Tree.tsx 199ee75d1240ae72cd965f62aceeb301ab64e1bd 463 463 1 boundary previous ca0fb5a2d61cb16909bcb06f49dd5448a26f32b1 Tree.tsx filename Tree.tsx public shouldComponentUpdate(nextProps: TileProps): boolean { The line in question is in the diff (git diff 199ee7..ca0fb5), but that particular line is neither added nor deleted, so I don't know why blame would think it is deleted. Relevant hunk in diff: @@ -452,28 +462,17 @@ export class LayerTile extends React.Component { } } -public validTokenRange(props: TileProps): boolean { -if (props.selectedPath === '') { -return true -} -const token = props.selectedPath.split('/').pop()! -return token >= this.first && token <= this.last -} - public shouldComponentUpdate(nextProps: TileProps): boolean { -const lastValid = this.validTokenRange(this.props) -const nextValid = this.validTokenRange(nextProps) -if (!lastValid && !nextValid) { -// short circuit -return false +if (isEqualOrAncestor(this.props.selectedDir, this.props.currSubpath)) { +return true } -if (isEqualOrAncestor(this.props.selectedDir, this.props.currSubpath) && lastValid) { +if (nextProps.selectedDir === nextProps.currSubpath) { return true } -if (nextProps.selectedDir === nextProps.currSubpath && this.validTokenRange(nextProps)) { +if (getParentDir(nextProps.selectedDir) === nextProps.currSubpath) { return true } -if (getParentDir(nextProps.selectedDir) === nextProps.currSubpath && this.validTokenRange(nextProps)) { +if (!isEqual(nextProps.pathSplits, this.props.pathSplits)) { return true } return false
Antw: Re: bug deleting "unmerged" branch (2.12.3)
> Hi Ulrich, > > On Tue, 28 Nov 2017, Ulrich Windl wrote: > >> During a rebase that turned out to be heavier than expected 8-( I >> decided to keep the old branch by creating a temporary branch name to >> the commit of the branch to rebase (which was still the old commit ID at >> that time). >> >> When done rebasing, I attached a new name to the new (rebased) branch, >> deleted the old name (pointing at the same rebase commit), then >> recreated the old branch from the temporary branch name (created to >> remember the commit id). >> >> When I wanted to delete the temporary branch (which is of no use now), I >> got a message that the branch is unmerged. > > This is actually as designed, at least for performance reasons (it is not > exactly cheap to figure out whether a given commit is contained in any > other branch). > >> I think if more than one branches are pointing to the same commit, one >> should be allowed to delete all but the last one without warning. Do you >> agree? > > No, respectfully disagree, because I have found myself with branches > pointing to the same commit, even if the branches served different > purposes. I really like the current behavior where you can delete a > branch with `git branch -d` as long as it is contained in its upstream > branch. Hi! I'm not talking about the intention of a branch, but of the state of a branch: If multiple branches point (not "contain") the same commit, they are equivalent (besides the name) at that moment. As no program can predict the future or the intentions of the user, it should be safe to delete the branch, because it can easily be recreated (from the remaining branches pointing to the same commit). It shouldn't need a lot of computational power to find out when multiple branches point to the same commit. Regards, Ulrich
Antw: Re: bug deleting "unmerged" branch (2.12.3)
> "Ulrich Windl" writes: > >> I think if more than one branches are pointing to the same commit, >> one should be allowed to delete all but the last one without >> warning. Do you agree? > > That comes from a viewpoint that the only purpose "branch -d" exists > in addition to "branch -D" is to protect objects from "gc". Those > who added the safety feature may have shared that view originally, > but it turns out that it protects another important thing you are > forgetting. > > Imagine that two topics, 'topicA' and 'topicB', were independently > forked from 'master', and then later we wanted to add a feature that > depends on these two topics. Since the 'feature' forked, there may > have been other developments, and we ended up in this topology: > > ---o---o---o---o---o---M > \ \ > \ o---A---o---F > \ / >o---o---o---o---B > > where A, B and F are the tips of 'topicA', 'topicB' and 'feature' > branches right now [*1*]. > > Now imagine we are on 'master' and just made 'topicB' graduate. We > would have this topology. > > ---o---o---o---o---o---o---M > \ \ / > \ o---A---o---F / > \ / / >o---o---o---o---B > > While we do have 'topicA' and 'feature' branches still in flight, > we are done with 'topicB'. Even though the tip of 'topicA' is > reachable from the tip of 'feature', the fact that the branch points > at 'A' is still relevant. If we lose that information right now, > we'd have to go find it when we (1) want to further enhance the > topic by checking out and building on 'topicA', and (2) want to > finally get 'topicA' graduate to 'master'. > > Because removal of a topic (in this case 'topicB') is often done > after a merge of that topic is made into an integration branch, > "branch -d" that protects branches that are yet to be merged to the > current branch catches you if you said "branch -d topic{A,B}" (or > other equivalent forms, most likely you'd have a script that spits > out list of branches and feed it to "xargs branch -d"). > > So, no, I do not agree. Hi! I can follow your argumentation, but I fail to see that your branches A and B point to the same commit (which is what I was talking about). So my situation would be: o---oA,B I still think I could safely remove either A or B, even when the branch (identified by the commit, not by the name) is unmerged. What did I miss? Regards, Ulrich > > > [Footnotes] > > *1* Since the 'feature' started developing, there were a few commits > added to 'topicB' but because the feature does not depend on > these enhancements to that topic, B is ahead of the commit that > was originally merged with the tip of 'topicA' to form the > 'feature' branch.
Re: [PATCH] strbuf: Remove unused stripspace function alias
On 2017-11-29 at 02:45:59 +0100, Elijah Newren wrote: > In commit 63af4a8446 ("strbuf: make stripspace() part of strbuf", > 2015-10-16), stripspace() was moved to strbuf and renamed to > strbuf_stripspace(). A "temporary" alias was added for the old name until > all topic branches had time to switch over. They have had time, so remove > the old alias. > > Signed-off-by: Elijah Newren Reviewed-by: Tobias Klauser Thanks!
Reply
I have something very confidential and private to discuss with you
Re: Antw: Re: bug deleting "unmerged" branch (2.12.3)
Hi Ulrich, On Wed, 29 Nov 2017, Ulrich Windl wrote: > > On Tue, 28 Nov 2017, Ulrich Windl wrote: > > > >> During a rebase that turned out to be heavier than expected 8-( I > >> decided to keep the old branch by creating a temporary branch name to > >> the commit of the branch to rebase (which was still the old commit ID > >> at that time). > >> > >> When done rebasing, I attached a new name to the new (rebased) > >> branch, deleted the old name (pointing at the same rebase commit), > >> then recreated the old branch from the temporary branch name (created > >> to remember the commit id). > >> > >> When I wanted to delete the temporary branch (which is of no use > >> now), I got a message that the branch is unmerged. > > > > This is actually as designed, at least for performance reasons (it is > > not exactly cheap to figure out whether a given commit is contained in > > any other branch). > > > >> I think if more than one branches are pointing to the same commit, > >> one should be allowed to delete all but the last one without warning. > >> Do you agree? > > > > No, respectfully disagree, because I have found myself with branches > > pointing to the same commit, even if the branches served different > > purposes. I really like the current behavior where you can delete a > > branch with `git branch -d` as long as it is contained in its upstream > > branch. > > I'm not talking about the intention of a branch, but of the state of a > branch: If multiple branches point (not "contain") the same commit, they > are equivalent (besides the name) at that moment. I did a poor job of explaining myself, please let me try again. I'll give you one concrete example: Recently, while working on some topic, I stumbled over a bug and committed a bug fix, then committed that and branched off a new branch to remind myself to rebase the bug fix and contribute it. At that point, those branches were at the same revision, but distinctly not equivalent (except in just one, very narrow sense of the word, which I would argue is the wrong interpretation in this context). Sadly, I was called away at that moment to take care of something completely different. Even if I had not been, the worktree with the first branch would still have been at that revision for a longer time, as I had to try out a couple of changes before I could commit. This is just one example where the idea backfires that you can safely delete one of two branches that happen to point at the same commit at the same time. I am sure that you possess vivid enough of an imagination to come up with plenty more examples where that is the case. > As no program can predict the future or the intentions of the user, it > should be safe to delete the branch, because it can easily be recreated > (from the remaining branches pointing to the same commit). Yes, no program can predict the future (at least *accurately*). No, it is not safe to delete that branch. Especially if you take the current paradigm of "it is safe to delete a branch if it is up-to-date with, or at least fast-forwardable to, its upstream branch" into account. And no, a branch cannot easily be recreated from the remaining branches in the future, as branches can have different reflogs (and they are lost when deleting the branch). > It shouldn't need a lot of computational power to find out when multiple > branches point to the same commit. Sure, that test can even be scripted easily by using the `git for-each-ref --points-at=` command. By the way, if you are still convinced that my argument is flawed and that it should be considered safe to delete a branch if any other branch points to the same revision, I encourage you to work on a patch to make it so. For maximum chance of getting included, you would want to guard this behind a new config setting, say, branch.deleteRedundantIsSafe, parse it here: https://github.com/git/git/blob/v2.15.1/config.c#L1260-L1288 or here: https://github.com/git/git/blob/v2.15.1/builtin/branch.c#L78-L97 document it here: https://github.com/git/git/blob/v2.15.1/Documentation/git-branch.txt and here: https://github.com/git/git/blob/v2.15.1/Documentation/config.txt#L969 and handle it here: https://github.com/git/git/blob/v2.15.1/builtin/branch.c#L185-L288 (look for the places where `force` is used, likely just before the call to `check_branch_commit()`). The way you'd want it to handle is most lilkely by imitating the `--points-at` code here: https://github.com/git/git/blob/v2.15.1/builtin/for-each-ref.c#L42 Ciao, Johannes
[ANNOUNCE] Git for Windows 2.15.1
Dear Git users, It is my pleasure to announce that Git for Windows 2.15.1 is available from: https://git-for-windows.github.io/ Changes since Git for Windows v2.15.0 (October 30th 2017) New Features * Comes with Git v2.15.1. * Operations in massively-sparse worktrees are now much faster if core.fscache = true. * It is now possible to configure nano or Notepad++ as Git's default editor instead of vim. * Comes with OpenSSL v1.0.2m. * Git for Windows' updater now uses non-intrusive toast notifications on Windows 8, 8.1 and 10. * Running git fetch in a repository with lots of refs is now considerably faster. * Comes with cURL v7.57.0. Bug Fixes * The experimental --show-ignored-directory option of git status which was removed in Git for Windows v2.15.0 without warning has been reintroduced as a deprecated option. * The git update command (to auto-update Git for Windows) will now work also work behind proxies. Filename | SHA-256 | --- Git-2.15.1-64-bit.exe | a2ba53197db79b361502836eecf97f09881703148774f9b4e9e6749d41d4ff71 Git-2.15.1-32-bit.exe | 73154bdfd1ad4ced8612d97b95603ff2b2383db9d46b4c308827fb5233d52592 PortableGit-2.15.1-64-bit.7z.exe | 94d485454af33a32d08680950aaf38f0825a189ef8b617054b81b2c48d817699 PortableGit-2.15.1-32-bit.7z.exe | 7d804748a7de735133d78c5420d9b338379123734509415035593e106b03515a MinGit-2.15.1-64-bit.zip | 5e38d13241b0742e6673bc5116ac82e851dd1fad01c660c943017f4549b6afea MinGit-2.15.1-32-bit.zip | 2fc85f67cabe3c13aceb6868b4bb6bda28ddfecd6f32d7e0da9ddce8cee9b940 MinGit-2.15.1-busybox-64-bit.zip | 02611486e3c8c427f09d2c4639484cd604ea812471248ae928960f1e0dc59633 MinGit-2.15.1-busybox-32-bit.zip | a6dfb770f5cfa7b120ba49922d3434577b8601c5d322ad473dd2e2adc91e92b3 Git-2.15.1-64-bit.tar.bz2 | bb630e5f3d7b650db67134b0187cf0cb8f5ed75990838ee65fed85e21777f08a Git-2.15.1-32-bit.tar.bz2 | ec3938e161ac1bbcf2b4f5d41fae1c05ea229fa0276b4db8530ec50b69131832 Ciao, Johannes
Re: [PATCH] git-prompt: fix reading files with windows line endings
Hi Robert, On Tue, 28 Nov 2017, Robert Abel wrote: > If any of the files read by __git_eread have \r\n line endings, read > will only strip \n, leaving \r. This results in an ugly prompt, where > instead of > > user@pc MINGW64 /path/to/repo (BARE:master) > > the last parenthesis is printed over the beginning of the prompt like > > )ser@pc MINGW64 /path/to/repo (BARE:master Thats' unfortunate, and obviously something to fix. > diff --git a/contrib/completion/git-prompt.sh > b/contrib/completion/git-prompt.sh > index c6cbef38c2..71a64e7959 100644 > --- a/contrib/completion/git-prompt.sh > +++ b/contrib/completion/git-prompt.sh > @@ -282,7 +282,7 @@ __git_eread () > { > local f="$1" > shift > - test -r "$f" && read "$@" <"$f" > + test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}" As far as I understand, $'\r' is a Bash-only construct, and this file (git-prompt.sh) is targeting other Unix shells, too. So how about using `tr -d '\r' <"$f" | read "$@"` instead? Or maybe keep with the Bash construct, but guarded behind a test that we area actually running in Bash? Something like test -z "$BASH" || IFS=$' \t\r\n' Ciao, Johannes
[PATCH v4 1/2] refactor "dumb" terminal determination
From: Lars Schneider Move the code to detect "dumb" terminals into a single location. This avoids duplicating the terminal detection code yet again in a subsequent commit. Signed-off-by: Lars Schneider --- cache.h| 1 + color.c| 3 +-- editor.c | 9 +++-- sideband.c | 5 ++--- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/cache.h b/cache.h index 89f5d24579..3842fc097c 100644 --- a/cache.h +++ b/cache.h @@ -1469,6 +1469,7 @@ extern const char *ident_default_name(void); extern const char *ident_default_email(void); extern const char *git_editor(void); extern const char *git_pager(int stdout_is_tty); +extern int is_terminal_dumb(void); extern int git_ident_config(const char *, const char *, void *); extern void reset_ident_date(void); diff --git a/color.c b/color.c index 9a9261ac16..d48dd947c9 100644 --- a/color.c +++ b/color.c @@ -329,8 +329,7 @@ static int check_auto_color(void) if (color_stdout_is_tty < 0) color_stdout_is_tty = isatty(1); if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) { - char *term = getenv("TERM"); - if (term && strcmp(term, "dumb")) + if (!is_terminal_dumb()) return 1; } return 0; diff --git a/editor.c b/editor.c index 7519edecdc..c65ea698eb 100644 --- a/editor.c +++ b/editor.c @@ -7,11 +7,16 @@ #define DEFAULT_EDITOR "vi" #endif +int is_terminal_dumb(void) +{ + const char *terminal = getenv("TERM"); + return !terminal || !strcmp(terminal, "dumb"); +} + const char *git_editor(void) { const char *editor = getenv("GIT_EDITOR"); - const char *terminal = getenv("TERM"); - int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb"); + int terminal_is_dumb = is_terminal_dumb(); if (!editor && editor_program) editor = editor_program; diff --git a/sideband.c b/sideband.c index 1e4d684d6c..6d7f943e43 100644 --- a/sideband.c +++ b/sideband.c @@ -20,13 +20,12 @@ int recv_sideband(const char *me, int in_stream, int out) { - const char *term, *suffix; + const char *suffix; char buf[LARGE_PACKET_MAX + 1]; struct strbuf outbuf = STRBUF_INIT; int retval = 0; - term = getenv("TERM"); - if (isatty(2) && term && strcmp(term, "dumb")) + if (isatty(2) && !is_terminal_dumb()) suffix = ANSI_SUFFIX; else suffix = DUMB_SUFFIX; -- 2.15.0
[PATCH v4 2/2] launch_editor(): indicate that Git waits for user input
From: Lars Schneider When a graphical GIT_EDITOR is spawned by a Git command that opens and waits for user input (e.g. "git rebase -i"), then the editor window might be obscured by other windows. The user may be left staring at the original Git terminal window without even realizing that s/he needs to interact with another window before Git can proceed. To this user Git appears hanging. Print a message that Git is waiting for editor input in the original terminal and get rid of it when the editor returns. No message is printed in a "dumb" terminal as it would not be possible to remove the message after the editor returns. This should not be a problem as this feature is targeted at novice Git users and they are unlikely to work with a "dumb" terminal. Power users might not want to see this message or their editor might already print such a message (e.g. emacsclient). Allow these users to suppress the message by disabling the "advice.waitingForEditor" config. The standard advise() function is not used here as it would always add a newline which would make deleting the message harder. Helped-by: Junio C Hamano Signed-off-by: Lars Schneider --- Documentation/config.txt | 3 +++ advice.c | 2 ++ advice.h | 1 + editor.c | 15 +++ 4 files changed, 21 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 671fcbaa0f..de64201e82 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -354,6 +354,9 @@ advice.*:: ignoredHook:: Advice shown if an hook is ignored because the hook is not set as executable. + waitingForEditor:: + Print a message to the terminal whenever Git is waiting for + editor input from the user. -- core.fileMode:: diff --git a/advice.c b/advice.c index c6169bcb52..406efc183b 100644 --- a/advice.c +++ b/advice.c @@ -18,6 +18,7 @@ int advice_object_name_warning = 1; int advice_rm_hints = 1; int advice_add_embedded_repo = 1; int advice_ignored_hook = 1; +int advice_waiting_for_editor = 1; static struct { const char *name; @@ -40,6 +41,7 @@ static struct { { "rmhints", &advice_rm_hints }, { "addembeddedrepo", &advice_add_embedded_repo }, { "ignoredhook", &advice_ignored_hook }, + { "waitingforeditor", &advice_waiting_for_editor }, /* make this an alias for backward compatibility */ { "pushnonfastforward", &advice_push_update_rejected } diff --git a/advice.h b/advice.h index f525d6f89c..70568fa792 100644 --- a/advice.h +++ b/advice.h @@ -20,6 +20,7 @@ extern int advice_object_name_warning; extern int advice_rm_hints; extern int advice_add_embedded_repo; extern int advice_ignored_hook; +extern int advice_waiting_for_editor; int git_default_advice_config(const char *var, const char *value); __attribute__((format (printf, 1, 2))) diff --git a/editor.c b/editor.c index c65ea698eb..cdad4f74ec 100644 --- a/editor.c +++ b/editor.c @@ -45,6 +45,13 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en const char *args[] = { editor, real_path(path), NULL }; struct child_process p = CHILD_PROCESS_INIT; int ret, sig; + int print_waiting_for_editor = advice_waiting_for_editor && + isatty(2) && !is_terminal_dumb(); + + if (print_waiting_for_editor) { + fprintf(stderr, _("hint: Waiting for your editor input...")); + fflush(stderr); + } p.argv = args; p.env = env; @@ -63,6 +70,14 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en if (ret) return error("There was a problem with the editor '%s'.", editor); + + if (print_waiting_for_editor) + /* +* go back to the beginning and erase the +* entire line to avoid wasting the vertical +* space. +*/ + fputs("\r\033[K", stderr); } if (!buffer) -- 2.15.0
[PATCH v4 0/2] launch_editor(): indicate that Git waits for user input
From: Lars Schneider Hi, I simplified the code and enabled the "Git is waiting for editor input" message only for "intelligent" terminals. The rational is, that today's novice Git users are likely to have an "intelligent" terminal. People working with "dumb" terminals have their reasons and are likely not the target audience for this hint. In addition, I added "advice.waitingForEditor" as an easy way to disable the hint for experienced users. I refactored the detection of dumb terminals in a preparation commit. Thanks, Lars RFC: https://public-inbox.org/git/274b4850-2eb7-4bfa-a42c-25a573254...@gmail.com/ v1: https://public-inbox.org/git/xmqqr2syvjxb@gitster.mtv.corp.google.com/ v2: https://public-inbox.org/git/20171117135109.18071-1-lars.schnei...@autodesk.com/ v3: https://public-inbox.org/git/20171127134716.69471-1-lars.schnei...@autodesk.com/ Base Ref: master Web-Diff: https://github.com/larsxschneider/git/commit/9639e931bf Checkout: git fetch https://github.com/larsxschneider/git editor-v4 && git checkout 9639e931bf ### Interdiff (v3..v4): diff --git a/Documentation/config.txt b/Documentation/config.txt index 671fcbaa0f..de64201e82 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -354,6 +354,9 @@ advice.*:: ignoredHook:: Advice shown if an hook is ignored because the hook is not set as executable. + waitingForEditor:: + Print a message to the terminal whenever Git is waiting for + editor input from the user. -- core.fileMode:: diff --git a/advice.c b/advice.c index c6169bcb52..406efc183b 100644 --- a/advice.c +++ b/advice.c @@ -18,6 +18,7 @@ int advice_object_name_warning = 1; int advice_rm_hints = 1; int advice_add_embedded_repo = 1; int advice_ignored_hook = 1; +int advice_waiting_for_editor = 1; static struct { const char *name; @@ -40,6 +41,7 @@ static struct { { "rmhints", &advice_rm_hints }, { "addembeddedrepo", &advice_add_embedded_repo }, { "ignoredhook", &advice_ignored_hook }, + { "waitingforeditor", &advice_waiting_for_editor }, /* make this an alias for backward compatibility */ { "pushnonfastforward", &advice_push_update_rejected } diff --git a/advice.h b/advice.h index f525d6f89c..70568fa792 100644 --- a/advice.h +++ b/advice.h @@ -20,6 +20,7 @@ extern int advice_object_name_warning; extern int advice_rm_hints; extern int advice_add_embedded_repo; extern int advice_ignored_hook; +extern int advice_waiting_for_editor; int git_default_advice_config(const char *var, const char *value); __attribute__((format (printf, 1, 2))) diff --git a/cache.h b/cache.h index 89f5d24579..3842fc097c 100644 --- a/cache.h +++ b/cache.h @@ -1469,6 +1469,7 @@ extern const char *ident_default_name(void); extern const char *ident_default_email(void); extern const char *git_editor(void); extern const char *git_pager(int stdout_is_tty); +extern int is_terminal_dumb(void); extern int git_ident_config(const char *, const char *, void *); extern void reset_ident_date(void); diff --git a/color.c b/color.c index 9a9261ac16..d48dd947c9 100644 --- a/color.c +++ b/color.c @@ -329,8 +329,7 @@ static int check_auto_color(void) if (color_stdout_is_tty < 0) color_stdout_is_tty = isatty(1); if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) { - char *term = getenv("TERM"); - if (term && strcmp(term, "dumb")) + if (!is_terminal_dumb()) return 1; } return 0; diff --git a/editor.c b/editor.c index 4251ae9d7a..cdad4f74ec 100644 --- a/editor.c +++ b/editor.c @@ -7,11 +7,16 @@ #define DEFAULT_EDITOR "vi" #endif +int is_terminal_dumb(void) +{ + const char *terminal = getenv("TERM"); + return !terminal || !strcmp(terminal, "dumb"); +} + const char *git_editor(void) { const char *editor = getenv("GIT_EDITOR"); - const char *terminal = getenv("TERM"); - int terminal_is_dumb = !terminal || !strcmp(terminal, "dumb"); + int terminal_is_dumb = is_terminal_dumb(); if (!editor && editor_program) editor = editor_program; @@ -40,33 +45,11 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en const char *args[] = { editor, real_path(path), NULL }; struct child_process p = CHILD_PROCESS_INIT; int ret, sig; - static const char *close_notice = NULL; - - if (isatty(2) && !close_notice) { - char *term = getenv("TERM"); - - if (term && strcmp(term, "dumb")) - /* -* go back to the beginning and erase the -* entire line if the terminal is capable -* to do so, to avoid wasting the vertical -
Re: [PATCH v5 5/6] rev-list: add list-objects filtering support
On 11/22/2017 3:08 PM, Jonathan Nieder wrote: Hi, Jeff Hostetler wrote: Teach rev-list to use the filtering provided by the traverse_commit_list_filtered() interface to omit unwanted objects from the result. Object filtering is only allowed when one of the "--objects*" options are used. micronit: the line widths seem to be uneven in these commit messages, which is a bit distracting when reading. ok When the "--filter-print-omitted" option is used, the omitted objects are printed at the end. These are marked with a "~". This option can be combined with "--quiet" to get a list of just the omitted objects. Neat. Can you give a quick example? I can put part of this in the V6 version, but here is a quick example. $ git rev-list --objects HEAD 0629eb0173b5de0fb664e24bd4292988b2af1290 b2eaab8088df7ddad7f134e7b5144892b9646749 21b9d3ce3550213ac7de1391a518ab83f16912a2 3760f5a324c290c4f3518f087e2bdedd9d8f9ce4 large.1000 b3b5c5d37a767d73101e5a27393af2682bda4fd7 large.1 472cd9f4ed2a4804bcc2a2865e1a3ae00742eb01 $ git rev-list --objects HEAD --filter=blob:none --filter-print-omitted 0629eb0173b5de0fb664e24bd4292988b2af1290 b2eaab8088df7ddad7f134e7b5144892b9646749 21b9d3ce3550213ac7de1391a518ab83f16912a2 472cd9f4ed2a4804bcc2a2865e1a3ae00742eb01 ~3760f5a324c290c4f3518f087e2bdedd9d8f9ce4 ~b3b5c5d37a767d73101e5a27393af2682bda4fd7 $ git rev-list --objects HEAD --filter=blob:limit=5000 --filter-print-omitted 0629eb0173b5de0fb664e24bd4292988b2af1290 b2eaab8088df7ddad7f134e7b5144892b9646749 21b9d3ce3550213ac7de1391a518ab83f16912a2 3760f5a324c290c4f3518f087e2bdedd9d8f9ce4 large.1000 472cd9f4ed2a4804bcc2a2865e1a3ae00742eb01 ~b3b5c5d37a767d73101e5a27393af2682bda4fd7 $ git rev-list --objects HEAD --filter=blob:limit=5000 --filter-print-omitted --quiet ~b3b5c5d37a767d73101e5a27393af2682bda4fd7 Using --quiet for this feels a bit odd, since it previously meant to print nothing to stdout. I wonder if there's another way --- e.g. --print-omitted=(yes|no|only) Yeah, now that you say that it does seem a little odd. I could go either way here. My expected usage was to be able to do a commits-and-trees fetch and then do before doing a checkout, do a narrow fetch of just the blobs in the tip that were omitted when the commit and trees were received. If I wanted to list all objects matching a filter, even objects that are not reachable from any ref, is there a way to do that? (Just curious, trying to think about this interface.) I'm building on rev-list, so you can give it one or more refs or revision ranges and it will traverse all of them and print any OIDs it finds during the traversal. I don't know of any way to visit unreachable objects, without using something like GC. Add t6112 test. This part doesn't need to be in the commit message. More generally, anything I could more easily learn from the code or diffstat doesn't need to be in the commit message: the commit message is about the "why" more than the details of what in the code changed. ok In the future, we will introduce a "partial clone" mechanism wherein an object in a repo, obtained from a remote, may reference a missing object that can be dynamically fetched from that remote once needed. This "partial clone" mechanism will have a way, sometimes slow, of determining if a missing link is one of the links expected to be produced by this mechanism. Does this mean the s will be part of the wire protocol? I'll look more carefully at them below with that in mind. yes. a later commit will send the given to the server. the idea is that clone and fetch will accept a filter argument, pass that to the server, and have upload-pack/pack-objects generate an incomplete packfile that omits unwanted objects. so you could do something like: $ git clone --no-checkout --filter=blob:none URL . then either [1] explicitly request the omitted blobs/objects for a tip commit or (in the case of a sparse-checkout) just the blobs for a portion of a tip commit and then do a normal checkout. or [2] rely on the (future) fetch-objects code to dynamically fetch the required omitted blobs during a checkout. Or if you know you (probably) never want gigantic blobs, do something like: $ git clone --filter=blob:limit=1m URL . which would give you a mostly complete view repo, but just exclude files > 1MB. Those blobs could then be demand loaded as needed later. This patch introduces handling of missing objects to help debugging and development of the "partial clone" mechanism, and once the mechanism is implemented, for a power user to perform operations that are missing-object aware without incurring the cost of checking if a missing link is expected. I had trouble understanding what this paragraph is about. Can you give an example? The concept of filtering during a clone or fetch leads to intentionally omitted object
[RFC/PATCH] Makefile: replace the overly complex perl build system with something simple
As the history of perl/Makefile.PL reveals the reason we have it in the first place is because during its initial development the perl bindings would link to libgit, building such a module with any sanity requires ExtUtils::MakeMaker and Devel::PPPort. However, since then this grew into a much simpler problem while keeping all the complexity of the initial implementation. We just need to take the files in perl/**.pm, copy them to a build directory while search/replacing one string (@@LOCALEDIR@@), and have them available for the likes of git-svn once Git is installed. This patch is a WIP effort to do that. Trade-offs this makes & other caveats: * This will not always install into perl's idea of its global "installsitelib". This only potentially matters for packagers that need to expose Git.pm for non-git use, and as explained in the INSTALL file there's a trivial workaround. * The scripts themselves will 'use lib' the target directory, but if INSTLIBDIR is set it overrides it. It doesn't have to be this way, but my reading of 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable that adds to default perl path", 2013-11-15) is that this does the right thing, but I may be wrong. * We don't build the Git(3) Git::I18N(3) etc. man pages from the embedded perldoc. I suspect nobody really cares, these are mostly internal APIs, and if someone's developing against them they likely know enough to issue a "perldoc" against the installed file to get the same result. But this is a change in how Git is installed now on e.g. CentOS & Debian which carry these manpages. They could be added (via pod2man) if anyone really cares. * Currently the Error.pm fallback doesn't work. This is a TODO item, but I didn't have enough time to hack this up. I think that whole thing should be changed, we're checking an ancient Error.pm version that was released in 2006, we should just stop that, then we check *at build time* whether the library is there, instead we should call it/load it via e.g. Git::Error which would be a small wrapper that would either load Error.pm from the system, or use our copy in Git::Error_CPAN. Signed-off-by: Ævar Arnfjörð Bjarmason --- So *long* CC list but a lot of people have touched this stuff over the years. This obviously conflicts with Dan Jacques's patches in a big way, but I think it's worth it to rebase them on top of this if there's interest in this, and depending on what people think about the caveats I've noted about this above. INSTALL | 18 ++- Makefile | 51 + perl/.gitignore | 9 +- perl/Git/I18N.pm | 2 +- perl/Makefile | 90 --- perl/Makefile.PL | 62 --- t/perf/aggregate.perl | 2 +- t/test-lib.sh | 2 +- wrap-for-bin.sh | 2 +- 9 files changed, 45 insertions(+), 193 deletions(-) delete mode 100644 perl/Makefile delete mode 100644 perl/Makefile.PL diff --git a/INSTALL b/INSTALL index ffb071e9f0..e6266a9127 100644 --- a/INSTALL +++ b/INSTALL @@ -84,9 +84,25 @@ Issues of note: GIT_EXEC_PATH=`pwd` PATH=`pwd`:$PATH - GITPERLLIB=`pwd`/perl/blib/lib + GITPERLLIB=`pwd`/perl/build export GIT_EXEC_PATH PATH GITPERLLIB + - By default (unless NO_PERL is provided) Git will ship various perl + scripts & libraries it needs. However, for simplicity it doesn't + use the ExtUtils::MakeMaker toolchain to decide where to place the + perl libraries. Depending on the system this can result in the perl + libraries not being where you'd like them if they're expected to be + used by things other than Git itself. + + Manually supplying a perllibdir prefix should fix this, if this is + a problem you care about, e.g.: + + prefix=/usr perllibdir=/usr/$(/usr/bin/perl -MConfig -wE 'my ($relsite) = $Config{installsitelib} =~ m[^\Q$Config{siteprefixexp}\E/(.+)]s; say $relsite') + + Will result in e.g. perllibdir=/usr/share/perl/5.26.1 on Debian, + perllibdir=/usr/share/perl5 (which we'd use by default) on CentOS + etc. + - Git is reasonably self-sufficient, but does depend on a few external programs and libraries. Git can be used without most of them by adding the approriate "NO_=YesPlease" to the make command line or diff --git a/Makefile b/Makefile index e53750ca01..6d69a7486b 100644 --- a/Makefile +++ b/Makefile @@ -295,9 +295,6 @@ all:: # # Define PERL_PATH to the path of your Perl binary (usually /usr/bin/perl). # -# Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's -# MakeMaker (e.g. using ActiveState under Cygwin). -# # Define NO_PERL if you do not want Perl scripts or libraries at all. # # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python @@ -473,6 +470,7 @@ gitexecdir = libexec/git-core mergetoolsdir = $(gitexecdir)/merg
[PATCH v4 4/4] exec_cmd: RUNTIME_PREFIX on some POSIX systems
Enable Git to resolve its own binary location using a variety of OS-specific and generic methods, including: - procfs via "/proc/self/exe" (Linux) - _NSGetExecutablePath (Darwin) - KERN_PROC_PATHNAME sysctl on BSDs. - argv0, if absolute (all, including Windows). This is used to enable RUNTIME_PREFIX support for non-Windows systems, notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will do a best-effort resolution of its executable path and automatically use this as its "exec_path" for relative helper and data lookups, unless explicitly overridden. Small incidental formatting cleanup of "exec_cmd.c". Signed-off-by: Dan Jacques Thanks-to: Robbie Iannucci Thanks-to: Junio C Hamano --- Makefile | 35 +++- cache.h | 1 + common-main.c| 4 +- config.mak.uname | 7 ++ exec_cmd.c | 239 +++ exec_cmd.h | 4 +- gettext.c| 8 +- git.c| 2 +- 8 files changed, 260 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index 741d1583f..e663fda1f 100644 --- a/Makefile +++ b/Makefile @@ -416,6 +416,16 @@ all:: # # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function. # +# Define HAVE_BSD_KERN_PROC_SYSCTL if your platform supports the KERN_PROC BSD +# sysctl function. +# +# Define PROCFS_EXECUTABLE_PATH if your platform mounts a "procfs" filesystem +# capable of resolving the path of the current executable. If defined, this +# must be the canonical path for the "procfs" current executable path. +# +# Define HAVE_NS_GET_EXECUTABLE_PATH if your platform supports calling +# _NSGetExecutablePath to retrieve the path of the running executable. +# # Define HAVE_GETDELIM if your system has the getdelim() function. # # Define PAGER_ENV to a SP separated VAR=VAL pairs to define @@ -426,6 +436,12 @@ all:: # to say "export LESS=FRX (and LV=-c) if the environment variable # LESS (and LV) is not set, respectively". # +# Define RUNTIME_PREFIX to configure Git to resolve its ancillary tooling and +# support files relative to the location of the runtime binary, rather than +# hard-coding them into the binary. Git installations built with RUNTIME_PREFIX +# can be moved to arbitrary filesystem locations. Users may want to enable +# RUNTIME_PREFIX_PERL as well (see below). +# # Define RUNTIME_PREFIX_PERL to configure Git's PERL commands to locate Git # support libraries relative to their filesystem path instead of hard-coding # it. @@ -466,6 +482,7 @@ ARFLAGS = rcs # mandir # infodir # htmldir +# localedir # perllibdir # This can help installing the suite in a relocatable way. @@ -491,6 +508,7 @@ mandir_relative = $(patsubst $(prefix)/%,%,$(mandir)) infodir_relative = $(patsubst $(prefix)/%,%,$(infodir)) sharedir_relative = $(patsubst $(prefix)/%,%,$(sharedir)) htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir)) +localedir_relative = $(patsubst $(prefix)/%,%,$(localedir)) export prefix bindir sharedir sysconfdir gitwebdir localedir @@ -1637,10 +1655,23 @@ ifdef HAVE_BSD_SYSCTL BASIC_CFLAGS += -DHAVE_BSD_SYSCTL endif +ifdef HAVE_BSD_KERN_PROC_SYSCTL + BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL +endif + ifdef HAVE_GETDELIM BASIC_CFLAGS += -DHAVE_GETDELIM endif +ifneq ($(PROCFS_EXECUTABLE_PATH),) + procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH)) + BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"' +endif + +ifdef HAVE_NS_GET_EXECUTABLE_PATH + BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH +endif + ifeq ($(TCLTK_PATH),) NO_TCLTK = NoThanks endif @@ -1723,6 +1754,7 @@ bindir_relative_SQ = $(subst ','\'',$(bindir_relative)) mandir_relative_SQ = $(subst ','\'',$(mandir_relative)) infodir_relative_SQ = $(subst ','\'',$(infodir_relative)) localedir_SQ = $(subst ','\'',$(localedir)) +localedir_relative_SQ = $(subst ','\'',$(localedir_relative)) gitexecdir_SQ = $(subst ','\'',$(gitexecdir)) template_dir_SQ = $(subst ','\'',$(template_dir)) htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative)) @@ -2185,6 +2217,7 @@ endif exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ + '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ '-DPREFIX="$(prefix_SQ)"' @@ -2202,7 +2235,7 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \ gettext.sp gettext.s gettext.o: GIT-PREFIX gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \ - -DGIT_LOCALE_PATH='"$(localedir_SQ)"' + -DGIT_LOCALE_PATH='"$(localedir_relative_SQ)"' http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS += \ -DCURL_DISABLE_TYPECHECK diff --git a/cache.h b/cache.h index 2e1434505..3d84aa0bf 100644 --- a/cache.h +++ b/cache.h @@ -449,6 +449,7 @@ static inline enum object_type object_type(
[PATCH v4 3/4] Makefile: add Perl runtime prefix support
Add a new Makefile flag, RUNTIME_PREFIX_PERL, which, when enabled, configures Perl scripts to locate the Git installation's Perl support libraries by resolving against the script's path, rather than hard-coding that path at build-time. Enabling RUNTIME_PREFIX_PERL overrides the system-specific Perl script installation path generated by MakeMaker to force installation into a platform-neutral location, "/share/perl5". This change enables Git's Perl scripts to work when their Git installation is relocated or moved to another system. Signed-off-by: Dan Jacques Thanks-to: Ævar Arnfjörð Bjarmason Thanks-to: Johannes Schindelin --- Makefile | 40 +- perl/header_runtime_prefix.pl.template | 24 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 perl/header_runtime_prefix.pl.template diff --git a/Makefile b/Makefile index 80904f8b0..741d1583f 100644 --- a/Makefile +++ b/Makefile @@ -425,6 +425,10 @@ all:: # # to say "export LESS=FRX (and LV=-c) if the environment variable # LESS (and LV) is not set, respectively". +# +# Define RUNTIME_PREFIX_PERL to configure Git's PERL commands to locate Git +# support libraries relative to their filesystem path instead of hard-coding +# it. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -485,6 +489,7 @@ pathsep = : mandir_relative = $(patsubst $(prefix)/%,%,$(mandir)) infodir_relative = $(patsubst $(prefix)/%,%,$(infodir)) +sharedir_relative = $(patsubst $(prefix)/%,%,$(sharedir)) htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir)) export prefix bindir sharedir sysconfdir gitwebdir localedir @@ -1967,7 +1972,38 @@ perl/PM.stamp: GIT-PERL-DEFINES FORCE PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) -PERL_DEFINES += $(NO_PERL_MAKEMAKER) +PERL_DEFINES += $(NO_PERL_MAKEMAKER) $(RUNTIME_PREFIX_PERL) + +# Support Perl runtime prefix. In this mode, a different header is installed +# into Perl scripts. This header expects both the scripts and their support +# library to be installed relative to $(prefix), and resolves the path to +# the Perl libraries (perllibdir) from the executable's current path +# (gitexecdir). +# +# This configuration requires both $(perllibdir) and $(gitexecdir) to be +# relative paths, and will error if this is not the case. +ifdef RUNTIME_PREFIX_PERL + +PERL_HEADER_TEMPLATE = perl/header_runtime_prefix.pl.template +PERL_DEFINES += $(gitexecdir) + +# RUNTIME_PREFIX_PERL requires a $(perllibdir) value. +ifeq ($(perllibdir),) +perllibdir = $(sharedir_relative)/perl5 +endif + +ifneq ($(filter /%,$(firstword $(gitexecdir))),) +$(error RUNTIME_PREFIX_PERL requires a relative gitexecdir, not: $(gitexecdir)) +endif +gitexecdir_relative_SQ = $(gitexecdir_SQ) + +ifneq ($(filter /%,$(firstword $(perllibdir))),) +$(error RUNTIME_PREFIX_PERL requires a relative perllibdir, not: $(perllibdir)) +endif +perllibdir_relative_SQ = $(perllibdir_SQ) + +endif + PERL_DEFINES += $(perllibdir) perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL @@ -2001,6 +2037,8 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES perl/perl.mak Makefile INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \ sed -e 's=@@PATHSEP@@=$(pathsep)=g' \ -e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \ + -e 's=@@GITEXECDIR@@=$(gitexecdir_relative_SQ)=g' \ + -e 's=@@PERLLIBDIR@@=$(perllibdir_relative_SQ)=g' \ $< >$@+ && \ mv $@+ $@ diff --git a/perl/header_runtime_prefix.pl.template b/perl/header_runtime_prefix.pl.template new file mode 100644 index 0..fb9a9924d --- /dev/null +++ b/perl/header_runtime_prefix.pl.template @@ -0,0 +1,24 @@ +# BEGIN RUNTIME_PREFIX_PERL generated code. +# +# This finds our Git::* libraries relative to the script's runtime path. +BEGIN { + use lib split /@@PATHSEP@@/, + ( + $ENV{GITPERLLIB} + || + do { + require FindBin; + require File::Spec; + my $gitexecdir_relative = '@@GITEXECDIR@@'; + my $perllibdir_relative = '@@PERLLIBDIR@@'; + + ($FindBin::Bin =~ m=${gitexecdir_relative}$=) || + die('Unrecognized runtime path.'); + my $prefix = substr($FindBin::Bin, 0, -length($gitexecdir_relative)); + my $perllibdir = File::Spec->catdir($prefix, $perllibdir_relative); + (-e $perllibdir) || die("Invalid library path: $perllibdir"); + $perllibdir; + } + ); +} +# END RUNTIME_PREFIX_PERL generated code. -- 2.15.0.chromium12
[PATCH v4 0/4] RUNTIME_PREFIX relocatable Git
Previous threads: v1: https://public-inbox.org/git/20171116170523.28696-1-...@google.com/ v2: https://public-inbox.org/git/20171119173141.4896-1-...@google.com/ v3: https://public-inbox.org/git/20171127164055.93283-1-...@google.com/ This is a small update to incorporate some Windows fixes from Johannes. At this point, it passes the full test suite on Linux, Mac, and FreeBSD, as well as the Travis.ci test suites, with and without RUNTIME_PREFIX/RUNTIME_PREFIX_PERL flags. I'm happy with the patch set, and feel that it is ready to move forward. However, while it's been looked at by several people, and I have incorporated reviewer feedback, the patch set hasn't received any formal LGTM-style responses yet. I'm not sure what standard of review is required to move forward with a patch on this project, so maybe this is totally fine, but I wanted to make sure to point this out. I also want to note Ævar Arnfjörð Bjarmason's RFC: https://public-inbox.org/git/20171129153436.24471-1-ava...@gmail.com/T/ The proposed patch set conflicts with the Perl installation directory changes in this patch set, as avarab@ notes. The proposed Perl installation process would simplify patch 0002 in this patch set. I don't think the landing order is terribly impactful - if this lands first, the patch in the RFC would delete a few more lines, and if this lands later, patch 0002 would largely not be necessary. Cheers, -Dan Built using this "config.mak" w/ autoconf: === BEGIN config.mak === RUNTIME_PREFIX = YesPlease RUNTIME_PREFIX_PERL = YesPlease gitexecdir = libexec/git-core template_dir = share/git-core/templates sysconfdir = etc === END config.mak === Changes in v4 from v3: - Incorporated some quoting and Makefile dependency fixes, courtesy of . Changes in v3 from v2: - Broken into multiple patches now that Perl is isolated in its own RUNTIME_PREFIX_PERL flag. - Working with avarab@, several changes to Perl script runtime prefix support: - Moved Perl header body content from Makefile into external template file(s). - Added generic "perllibdir" variable to override Perl installation path. - RUNTIME_PREFIX_PERL generated script header is more descriptive and consistent with how the C version operates. - Fixed Generated Perl header Makefile dependency, should rebuild when dependent files and flags change. - Changed some of the new RUNTIME_PREFIX trace strings to use consistent formatting and terminology. Changes in v2 from v1: - Added comments and formatting to improve readability of platform-sepecific executable path resolution sleds in `git_get_exec_path`. - Consolidated "cached_exec_path" and "argv_exec_path" globals into "exec_path_value". - Use `strbuf_realpath` instead of `realpath` for procfs resolution. - Removed new environment variable exports. Git with RUNTIME_PREFIX no longer exports or consumes any additional environment information. - Updated Perl script resolution strategy: rather than having Git export the relative executable path to the Perl scripts, they now resolve it independently when RUNTIME_PREFIX_PERL is enabled. - Updated resolution strategy for "gettext()": use system_path() instead of special environment variable. - Added `sysctl` executable resolution support for BSDs that don't mount "procfs" by default (most of them). Dan Jacques (4): Makefile: generate Perl header from template file Makefile: add support for "perllibdir" Makefile: add Perl runtime prefix support exec_cmd: RUNTIME_PREFIX on some POSIX systems .gitignore | 1 + Makefile | 111 +-- cache.h| 1 + common-main.c | 4 +- config.mak.uname | 7 + exec_cmd.c | 239 - exec_cmd.h | 4 +- gettext.c | 8 +- git.c | 2 +- perl/Makefile | 52 ++- perl/header_fixed_prefix.pl.template | 1 + perl/header_runtime_prefix.pl.template | 24 12 files changed, 396 insertions(+), 58 deletions(-) create mode 100644 perl/header_fixed_prefix.pl.template create mode 100644 perl/header_runtime_prefix.pl.template -- 2.15.0.chromium12
[PATCH v4 1/4] Makefile: generate Perl header from template file
Currently, the generated Perl script headers are emitted by commands in the Makefile. This mechanism restricts options to introduce alternative header content, needed by Perl runtime prefix support, and obscures the origin of the Perl script header. Change the Makefile to generate a header by processing a template file and move the header content into the "perl/" subdirectory. The processed generated will now be stored in the "GIT-PERL-HEADER" file. This allows the content of the Perl header to be controlled by changing the path of the template in the Makefile. Signed-off-by: Dan Jacques Thanks-to: Ævar Arnfjörð Bjarmason Thanks-to: Johannes Schindelin --- .gitignore | 1 + Makefile | 24 +++- perl/header_fixed_prefix.pl.template | 1 + 3 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 perl/header_fixed_prefix.pl.template diff --git a/.gitignore b/.gitignore index 833ef3b0b..89bd7bd8a 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ /GIT-LDFLAGS /GIT-PREFIX /GIT-PERL-DEFINES +/GIT-PERL-HEADER /GIT-PYTHON-VARS /GIT-SCRIPT-DEFINES /GIT-USER-AGENT diff --git a/Makefile b/Makefile index e53750ca0..f7c4ac207 100644 --- a/Makefile +++ b/Makefile @@ -1964,18 +1964,15 @@ perl/PM.stamp: FORCE perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F) +PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ) -$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-VERSION-FILE + +$(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE $(QUIET_GEN)$(RM) $@ $@+ && \ - INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \ - INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \ - INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \ sed -e '1{' \ -e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \ - -e 'h' \ - -e 's=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \ - -e 'H' \ - -e 'x' \ + -e 'rGIT-PERL-HEADER' \ + -e 'G' \ -e '}' \ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \ $< >$@+ && \ @@ -1989,6 +1986,15 @@ GIT-PERL-DEFINES: FORCE echo "$$FLAGS" >$@; \ fi +GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES perl/perl.mak Makefile + $(QUIET_GEN)$(RM) $@ && \ + INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \ + INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \ + INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \ + sed -e 's=@@PATHSEP@@=$(pathsep)=g' \ + -e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \ + $< >$@+ && \ + mv $@+ $@ .PHONY: gitweb gitweb: @@ -2707,7 +2713,7 @@ ifndef NO_TCLTK endif $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS $(RM) GIT-USER-AGENT GIT-PREFIX - $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS + $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS .PHONY: all install profile-clean clean strip .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell diff --git a/perl/header_fixed_prefix.pl.template b/perl/header_fixed_prefix.pl.template new file mode 100644 index 0..9a4bc4d30 --- /dev/null +++ b/perl/header_fixed_prefix.pl.template @@ -0,0 +1 @@ +use lib (split(/@@PATHSEP@@/, $ENV{GITPERLLIB} || "@@INSTLIBDIR@@")); -- 2.15.0.chromium12
[PATCH v4 2/4] Makefile: add support for "perllibdir"
Add the "perllibdir" Makefile variable, which allows the customization of the Perl library installation path. The Perl library installation path is currently left entirely to the Perl Makefile implementation, either MakeMaker (default) or a fixed path when NO_PERL_MAKEMAKER is enabled. This patch introduces "perllibdir", a Makefile variable that can override that Perl module installation path. As with some other Makefile variables, "perllibdir" may be either absolute or relative. In the latter case, it is treated as relative to "$(prefix)". Add some incidental documentation to perl/Makefile. Explicitly specifying an installation path is necessary for Perl runtime prefix support, as runtime prefix resolution code must know in advance where the Perl support modules are installed. Signed-off-by: Dan Jacques --- Makefile | 18 +- perl/Makefile | 52 ++-- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index f7c4ac207..80904f8b0 100644 --- a/Makefile +++ b/Makefile @@ -462,6 +462,7 @@ ARFLAGS = rcs # mandir # infodir # htmldir +# perllibdir # This can help installing the suite in a relocatable way. prefix = $(HOME) @@ -1721,6 +1722,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir)) template_dir_SQ = $(subst ','\'',$(template_dir)) htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative)) prefix_SQ = $(subst ','\'',$(prefix)) +perllibdir_SQ = $(subst ','\'',$(perllibdir)) gitwebdir_SQ = $(subst ','\'',$(gitwebdir)) SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) @@ -1955,17 +1957,22 @@ $(SCRIPT_PERL_GEN): perl/perl.mak perl/perl.mak: perl/PM.stamp -perl/PM.stamp: FORCE +perl/PM.stamp: GIT-PERL-DEFINES FORCE @$(FIND) perl -type f -name '*.pm' | sort >$@+ && \ + cat GIT-PERL-DEFINES >>$@+ && \ $(PERL_PATH) -V >>$@+ && \ { cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \ $(RM) $@+ -perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL - $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' $(@F) - PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ) + +PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) +PERL_DEFINES += $(NO_PERL_MAKEMAKER) +PERL_DEFINES += $(perllibdir) + +perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL + $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' \ + prefix='$(prefix_SQ)' perllibdir='$(perllibdir_SQ)' $(@F) $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE $(QUIET_GEN)$(RM) $@ $@+ && \ @@ -1979,6 +1986,7 @@ $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES GIT-PERL-HEADER GI chmod +x $@+ && \ mv $@+ $@ +PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES)) GIT-PERL-DEFINES: FORCE @FLAGS='$(PERL_DEFINES)'; \ if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \ diff --git a/perl/Makefile b/perl/Makefile index f657de20e..b2aeeb0d8 100644 --- a/perl/Makefile +++ b/perl/Makefile @@ -1,6 +1,22 @@ # # Makefile for perl support modules and routine # +# This Makefile generates "perl.mak", which contains the actual build and +# installation directions. +# +# PERL_PATH must be defined to be the path of the Perl interpreter to use. +# +# prefix must be defined as the Git installation prefix. +# +# localedir must be defined as the path to the locale data. +# +# perllibdir may be optionally defined to override the default Perl module +# installation directory, which is relative to prefix. If perllibdir is not +# absolute, it will be treated as relative to prefix. +# +# NO_PERL_MAKEMAKER may be defined to use a built-in Makefile generation method +# instead of Perl MakeMaker. + makfile:=perl.mak modules = @@ -12,6 +28,16 @@ ifndef V QUIET = @ endif +# If a library directory is provided, and it is not an absolute path, resolve +# it relative to prefix. +ifneq ($(perllibdir),) +ifneq ($(filter /%,$(firstword $(perllibdir))),) +perllib_instdir = $(perllibdir) +else +perllib_instdir = $(prefix)/$(perllibdir) +endif +endif + all install instlibdir: $(makfile) $(QUIET)$(MAKE) -f $(makfile) $@ @@ -25,7 +51,12 @@ clean: $(makfile): PM.stamp ifdef NO_PERL_MAKEMAKER -instdir_SQ = $(subst ','\'',$(prefix)/lib) + +ifeq ($(perllib_instdir),) +perllib_instdir = $(prefix)/lib +endif + +instdir_SQ = $(subst ','\'',$(perllib_instdir)) modules += Git modules += Git/I18N @@ -42,7 +73,7 @@ modules += Git/SVN/Prompt modules += Git/SVN/Ra modules += Git/SVN/Utils -$(makfile): ../GIT-CFLAGS Makefile +$(makfile): ../GIT-CFLAGS ../GIT-PERL-DEFINES Makefile echo all: private-Error.pm Git.pm Git/I18N.pm > $@ set -e; \ for i in $(modules); \ @@ -79,12 +110,21 @@ $(makfile): ../GIT-CFLAGS Makefile echo ' cp pri
imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL
Hi, I'm trying to send a patch with the command `git imap-send`, I used the examples in the manual page as the main reference for my configuration: ``` [imap] folder = "[Gmail]/Drafts" host = imaps://imap.gmail.com user = doron.be...@gmail.com port = 993 sslverify = false ``` This is my `cat patch.out | git imap-send` output: ``` Password for 'imaps://doron.be...@gmail.com@imap.gmail.com': sending 3 messages curl_easy_perform() failed: URL using bad/illegal format or missing URL ``` The URI doesn't seem OK to me, I tried using `imap.user = doron.behar` and the URI was `imaps://doron.be...@imap.gmail.com` but that ended up with the same error as in the previous case. I would love to get some help here, a Google Search didn't help as well. Thanks.
Re: [PATCH] repository: fix a sparse 'using integer as NULL pointer' warning
On 29/11/17 01:35, brian m. carlson wrote: > On Tue, Nov 28, 2017 at 03:01:19AM +, Ramsay Jones wrote: >> >> Commit 78a6766802 ("Integrate hash algorithm support with repo setup", >> 2017-11-12) added a 'const struct git_hash_algo *hash_algo' field to the >> repository structure, without modifying the initializer of the 'the_repo' >> variable. This does not actually introduce a bug, since the '0' initializer >> for the 'ignore_env:1' bit-field is interpreted as a NULL pointer (hence >> the warning), and the final field (now with no initializer) receives a >> default '0'. >> >> Signed-off-by: Ramsay Jones >> --- >> >> Hi Junio, >> >> I don't recall Brian doing a re-roll of the 'bc/hash-algo' branch[1], but >> now that it has been merged into the 'next' branch, sparse is barking on >> that branch too. This patch (slightly different to the last one) applies >> on top of 'next'. > > Thanks for the patch; it's obviously correct. I'll get sparse set up > for future patch series. Heh, at this point I usually remark that you would need to build sparse from source, since all the packaged versions are so old as to be virtually useless (to run over git). That may still be the case, depending on your distro, since there was a new release a couple of months ago. However, you know how long it takes to get into some distros! ;-) [I _think_ it may be in Debian Unstable] Oh, the release (v0.5.1) was tagged on 17 Aug 2017. (I'm currently running v0.5.1-30-gf1e4ba1, built from source). ATB, Ramsay Jones
Git filter clean not working when staging files
Hello, I've got a question or maybe bug about Git clean filter. According to documentation[1] clean should be run when staging files. When I'm staging file I see my script was run (I'm logging execution into /tmp), but file content is unmodified. My script is reading stdin and writing to stdout. Nothing breaks in the script, from logs I see the source and modified content are proper and I'm returning 0 return code. Is this behavior intended? What should I do to modify the content? Smudge works as intended. But I've also noticed that git status runs clean filter but does not modify contents. Is that also intended? Maybe I'm missing something? If this is not going to work, is there other way to modify file content on commit? (so the fixed content goes to local repo) Any other options ? [1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes#filters_b -- Thanks for help, Rafal
[PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l
In the documentation of diff-tree, it is stated that the -l option "prevents rename/copy detection from running if the number of rename/copy targets exceeds the specified number". The documentation does not mention any special handling for the number 0, but the implementation before commit b520abf ("sequencer: warn when internal merge may be suboptimal due to renameLimit", 2017-11-14) treated 0 as a special value indicating that the rename limit is to be a very large number instead. The commit b520abf changed that behavior, treating 0 as 0. Revert this behavior to what it was previously. This allows existing scripts and tools that use "-l0" to continue working. The alternative (to allow "-l0") is probably much less useful, since users can just refrain from specifying -M and/or -C to have the same effect. Signed-off-by: Jonathan Tan --- Note that this patch is built on en/rename-progress. We noticed this through an automated test for an internal tool - the tool uses git diff-tree with -l0, and no longer produces the same results as before. --- diffcore-rename.c | 2 ++ t/t4001-diff-rename.sh | 15 +++ 2 files changed, 17 insertions(+) diff --git a/diffcore-rename.c b/diffcore-rename.c index 9ca0eaec7..245e999fe 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -392,6 +392,8 @@ static int too_many_rename_candidates(int num_create, * *num_create * num_src > rename_limit * rename_limit */ + if (rename_limit <= 0) + rename_limit = 32767; if ((num_create <= rename_limit || num_src <= rename_limit) && ((uint64_t)num_create * (uint64_t)num_src <= (uint64_t)rename_limit * (uint64_t)rename_limit)) diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh index 0d1fa45d2..eadf4f624 100755 --- a/t/t4001-diff-rename.sh +++ b/t/t4001-diff-rename.sh @@ -230,4 +230,19 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' ' test_i18ngrep " d/f/{ => f}/e " output ' +test_expect_success 'diff-tree -l0 defaults to a big rename limit, not zero' ' + test_write_lines line1 line2 line3 >myfile && + git add myfile && + git commit -m x && + + test_write_lines line1 line2 line4 >myotherfile && + git rm myfile && + git add myotherfile && + git commit -m x && + + git diff-tree -M -l0 HEAD HEAD^ >actual && + # Verify that a rename from myotherfile to myfile was detected + grep "myotherfile.*myfile" actual +' + test_done -- 2.15.0.173.g9268cf4a2.dirty
Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input
On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schnei...@autodesk.com wrote: > + if (print_waiting_for_editor) { > + fprintf(stderr, _("hint: Waiting for your editor > input...")); > fflush(stderr); Just FYI, stderr is typically unbuffered on most systems I've used, and although the call to fflush() is harmless, I suspect it's not having any effect. That said, there's plenty of other places in Git which seems to think fflush()ing stderr actually does something. -- Thomas Adam
Re: [PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l
On Wed, Nov 29, 2017 at 10:32 AM, Jonathan Tan wrote: > In the documentation of diff-tree, it is stated that the -l option > "prevents rename/copy detection from running if the number of > rename/copy targets exceeds the specified number". The documentation > does not mention any special handling for the number 0, but the > implementation before commit b520abf ("sequencer: warn when internal > merge may be suboptimal due to renameLimit", 2017-11-14) treated 0 as a > special value indicating that the rename limit is to be a very large > number instead. > > The commit b520abf changed that behavior, treating 0 as 0. Revert this > behavior to what it was previously. This allows existing scripts and > tools that use "-l0" to continue working. The alternative (to allow > "-l0") is probably much less useful, since users can just refrain from > specifying -M and/or -C to have the same effect. > > Signed-off-by: Jonathan Tan > --- > Note that this patch is built on en/rename-progress. > > We noticed this through an automated test for an internal tool - the > tool uses git diff-tree with -l0, and no longer produces the same > results as before. Thanks for testing that version and sending along the fix. I suspect the commit referenced twice in the commit message should have been 9f7e4bfa3b ("diff: remove silent clamp of renameLimit", 2017-11-13) rather than b520abf ("sequencer: warn when internal merge may be suboptimal due to renameLimit", 2017-11-14). Other than that minor issue, patch and test looks good to me.
Re: [PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l
On Wed, 29 Nov 2017 10:51:20 -0800 Elijah Newren wrote: > Thanks for testing that version and sending along the fix. > > I suspect the commit referenced twice in the commit message should > have been 9f7e4bfa3b ("diff: remove silent clamp of renameLimit", > 2017-11-13) rather than b520abf ("sequencer: warn when internal merge > may be suboptimal due to renameLimit", 2017-11-14). Ah...yes, you're right. I'll update the commit message if I need to send out a new version or if Junio requests it (so that it's easier for him to merge it in). > Other than that minor issue, patch and test looks good to me. Thanks.
Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)
Am 28.11.2017 um 02:15 schrieb Igor Djordjevic: On 27/11/2017 22:54, Johannes Sixt wrote: I my opinion, putting the focus on integration merge commits and the desire to automate the re-merge step brings in a LOT of complexity in the implementation for a very specific use-case that does not necessarily help other cases. It might seem more complex than it is, until you examine the guts to see how it really works :) Basic concept is pretty simple, as I actually don`t automate anything, at least not in terms of what would manual user steps look like - for example, there`s no real re-merge step in terms of actually redoing the merge, I just reuse what was already there in a very clean way, I would think (supported by my current, humble knowledge, still). The only merge that could possibly ever happen is upon committing desired subset of changes onto parent, and that shouldn`t be too complex by definition, otherwise that commit doesn`t really belong there in the first place, if it can`t be meaningfully applied where we want it (for now, at least). That said, the whole operation of "posting on parent and re-merging everything", the way it looks like from the outside, could end just with a simple diff-apply-commit-commit internally, no merges at all. Only if simple `git apply` fails, we try some trivial merging - and all that inside separate (parent) index only, not touching original HEAD index nor working tree, staying pristine for the whole process, untouched. Once done, you should be in the very same situation you started from, nothing changed, just having your history tweaked a bit to tell a different story on how you got there (now including a commit you posted on your HEAD`s parent). Ok, then please explain, how this process should work in my workflow and with the `commit --onto-parent` feature that you have in mind. I have an integration branch (which is a throw-away type, so you can mangle it in any way you want); it is a series of merges: ...A...C <- topics A, C \ \ ---o---o---o---o<- integration / / ...B...D <- topics B, D Now I find a bug in topic B. Assume that the merges of C and D have textual conflicts with the integration branch (but not with B) and/or may be evil. What should I do? With git-post, I make a fixup commit commit on the integration branch, then `git post B && git merge B`: ...A...C <- topics A, C \ \ ---o---o---o---o---f---F<- integration / / / ...B...D / <- topic D \ / f'--'<- topic B The merge F does not introduce any changes on the integration branch, so I do not need it, but it helps keep topic B off radar when I ask `git branch --no-merged` later. Don`t let "usual/preferred/recommended" Git workflow distract you too much - one of the reasons I made this is because it also allows _kind of_ "vanilla Git" patch queue, where you can quickly work on top of the merge head, pushing commits onto parents below, being tips of your "queues", putting you up to speed without a need to ever switch a branch (hypothetically), until satisfied with what you have, where you can slow down and polish each branch separately, as usual. Like working on multiple branches at the same time, in the manner similar to what `git add --patch` allows in regards to working on multiple commits at the same time. This just takes it on yet another level... hopefully :) 'kay, I'm not eagerly waiting for this particular next level (I prefer to keep things plain and simple), but I would never say this were a broken workflow. ;) In your scenario above, it would certainly not be too bad if you forgo the automatic merge and have the user issue a merge command manually. The resulting history could look like this: (3) o---o---A---X(topicA) / \ \ / M1--M2 (test, HEAD) / /|| ---o---o---M---' || (master) \ \ / | \ o-B / (topicB) \ / o---o---C(topicC) I.e., commit --onto-parent A produced commit X, but M2 was then a regular manual merge. (Of course, I am assuming that the merge commits are dispensible, and only the resulting tree is of interest.) I see - and what you`re asking for is what I already envisioned and hoped to get some more feedback about, here`s excerpt from [SCRIPT/RFC 3/3] git-commit--onto-parent.sh[1] (I guess you didn`t have time to checked that one yet?): I did have a brief look, but I stopped when I saw # Remove entry from HEAD reflog, not to pollute it with # uninteresting in-between steps we take, leaking implementation # details to end user. It's a clear sign for me that's something wrong. It is not just reflogs that can become stale, but all operations that follow the `git commit` can fa
[PATCH] Makefile: replace perl/Makefile.PL with simple make rules
Replace the perl/Makefile.PL and the fallback perl/Makefile used under NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily inspired by how the i18n infrastructure's build process works[1]. The reason for having the Makefile.PL in the first place is that it was initially[2] building a perl C binding to interface with libgit, this functionality, that was removed[3] before Git.pm ever made it to the master branch. We've since since started maintaining a fallback perl/Makefile, as MakeMaker wouldn't work on some platforms[4]. That's just the tip of the iceberg. We have the PM.stamp hack in the top-level Makefile[5] to detect whether we need to regenerate the perl/perl.mak, which I fixed just recently to deal with issues like the perl version changing from under us[6]. There is absolutely no reason for why this needs to be so complex anymore. All we're getting out of this elaborate Rube Goldberg machine was copying perl/* to perl/blib/* as we do a string-replacement on the *.pm files to hardcode @@LOCALEDIR@@ in the source. So replace the whole thing with something that's pretty much a copy of how we generate po/build/**.mo from po/*.po, just with a small sed(1) command instead of msgfmt. As that's being done rename the files from *.pm to *.pmc just to indicate that they're genreated (see "perldoc -f require"). While I'm at it, change the fallback for Error.pm from being something where we'll ship our own Error.pm if one doesn't exist at build time to one where we just use a Git::Error wrapper that'll always prefer the system-wide Error.pm, only falling back to our own copy if it really doesn't exist at runtime. It's now shipped as Git::FromCPAN::Error, making it easy to add other modules to Git::FromCPAN::* in the future if that's needed. Functional changes: * This will not always install into perl's idea of its global "installsitelib". This only potentially matters for packagers that need to expose Git.pm for non-git use, and as explained in the INSTALL file there's a trivial workaround. * The scripts themselves will 'use lib' the target directory, but if INSTLIBDIR is set it overrides it. It doesn't have to be this way, it could be set in addition to INSTLIBDIR, but my reading of [7] is that this is the desired behavior. * We don't build the Git(3) Git::I18N(3) etc. man pages from the embedded perldoc. I suspect nobody really cares, these are mostly internal APIs, and if someone's developing against them they likely know enough to issue a "perldoc" against the installed file to get the same result. But this is a change in how Git is installed now on e.g. CentOS & Debian which carry these manpages. They could be added (via pod2man) if anyone really cares. I doubt they will. The reason these were built in the first place was as a side-effect of how ExtUtils::MakeMaker works. 1. 5e9637c629 ("i18n: add infrastructure for translating Git with gettext", 2011-11-18) 2. b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24) 3. 18b0fc1ce1 ("Git.pm: Kill Git.xs for now", 2006-09-23) 4. f848718a69 ("Make perl/ build procedure ActiveState friendly.", 2006-12-04) 5. ee9be06770 ("perl: detect new files in MakeMaker builds", 2012-07-27) 6. c59c4939c2 ("perl: regenerate perl.mak if perl -V changes", 2017-03-29) 7. 0386dd37b1 ("Makefile: add PERLLIB_EXTRA variable that adds to default perl path", 2013-11-15) Signed-off-by: Ævar Arnfjörð Bjarmason --- Here's the non-RFC version. I think this is ready to be applied, fixing that issue with Error.pm was easier than I thought. INSTALL | 17 - Makefile | 53 +++--- contrib/examples/git-difftool.perl | 2 +- git-send-email.perl | 2 +- perl/.gitignore | 9 +-- perl/Git.pm | 2 +- perl/Git/Error.pm| 47 + perl/{private-Error.pm => Git/FromCPAN/Error.pm} | 0 perl/Git/I18N.pm | 2 +- perl/Makefile| 90 perl/Makefile.PL | 62 t/perf/aggregate.perl| 2 +- t/test-lib.sh| 2 +- wrap-for-bin.sh | 2 +- 14 files changed, 95 insertions(+), 197 deletions(-) create mode 100644 perl/Git/Error.pm rename perl/{private-Error.pm => Git/FromCPAN/Error.pm} (100%) delete mode 100644 perl/Makefile delete mode 100644 perl/Makefile.PL diff --git a/INSTALL b/INSTALL index ffb071e9f0..822ed16095 100644 --- a/INSTALL +++ b/INSTALL @@ -84,9 +84,24 @@ Issues of note: GIT_EXEC_PATH=`pwd` PATH=`pwd`:$PATH - GITPERLLIB=`pwd`/perl/blib/lib + GITPERLLIB=`pwd`/perl/build export GIT_EXEC_
[PATCH v6 1/6] checkout: factor out functions to new lib file
Factor the functions out, so they can be re-used from other places. In particular these functions will be re-used in builtin/worktree.c to make git worktree add dwim more. While there add some docs to the function. Signed-off-by: Thomas Gummerer --- Makefile | 1 + builtin/checkout.c | 41 + checkout.c | 43 +++ checkout.h | 13 + 4 files changed, 58 insertions(+), 40 deletions(-) create mode 100644 checkout.c create mode 100644 checkout.h diff --git a/Makefile b/Makefile index e53750ca01..a80a8fcca9 100644 --- a/Makefile +++ b/Makefile @@ -759,6 +759,7 @@ LIB_OBJS += branch.o LIB_OBJS += bulk-checkin.o LIB_OBJS += bundle.o LIB_OBJS += cache-tree.o +LIB_OBJS += checkout.o LIB_OBJS += color.o LIB_OBJS += column.o LIB_OBJS += combine-diff.o diff --git a/builtin/checkout.c b/builtin/checkout.c index 7d8bcc3833..ad8f94044c 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1,5 +1,6 @@ #include "builtin.h" #include "config.h" +#include "checkout.h" #include "lockfile.h" #include "parse-options.h" #include "refs.h" @@ -872,46 +873,6 @@ static int git_checkout_config(const char *var, const char *value, void *cb) return git_xmerge_config(var, value, NULL); } -struct tracking_name_data { - /* const */ char *src_ref; - char *dst_ref; - struct object_id *dst_oid; - int unique; -}; - -static int check_tracking_name(struct remote *remote, void *cb_data) -{ - struct tracking_name_data *cb = cb_data; - struct refspec query; - memset(&query, 0, sizeof(struct refspec)); - query.src = cb->src_ref; - if (remote_find_tracking(remote, &query) || - get_oid(query.dst, cb->dst_oid)) { - free(query.dst); - return 0; - } - if (cb->dst_ref) { - free(query.dst); - cb->unique = 0; - return 0; - } - cb->dst_ref = query.dst; - return 0; -} - -static const char *unique_tracking_name(const char *name, struct object_id *oid) -{ - struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 }; - cb_data.src_ref = xstrfmt("refs/heads/%s", name); - cb_data.dst_oid = oid; - for_each_remote(check_tracking_name, &cb_data); - free(cb_data.src_ref); - if (cb_data.unique) - return cb_data.dst_ref; - free(cb_data.dst_ref); - return NULL; -} - static int parse_branchname_arg(int argc, const char **argv, int dwim_new_local_branch_ok, struct branch_info *new, diff --git a/checkout.c b/checkout.c new file mode 100644 index 00..ac42630f74 --- /dev/null +++ b/checkout.c @@ -0,0 +1,43 @@ +#include "cache.h" +#include "remote.h" +#include "checkout.h" + +struct tracking_name_data { + /* const */ char *src_ref; + char *dst_ref; + struct object_id *dst_oid; + int unique; +}; + +static int check_tracking_name(struct remote *remote, void *cb_data) +{ + struct tracking_name_data *cb = cb_data; + struct refspec query; + memset(&query, 0, sizeof(struct refspec)); + query.src = cb->src_ref; + if (remote_find_tracking(remote, &query) || + get_oid(query.dst, cb->dst_oid)) { + free(query.dst); + return 0; + } + if (cb->dst_ref) { + free(query.dst); + cb->unique = 0; + return 0; + } + cb->dst_ref = query.dst; + return 0; +} + +const char *unique_tracking_name(const char *name, struct object_id *oid) +{ + struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 }; + cb_data.src_ref = xstrfmt("refs/heads/%s", name); + cb_data.dst_oid = oid; + for_each_remote(check_tracking_name, &cb_data); + free(cb_data.src_ref); + if (cb_data.unique) + return cb_data.dst_ref; + free(cb_data.dst_ref); + return NULL; +} diff --git a/checkout.h b/checkout.h new file mode 100644 index 00..9980711179 --- /dev/null +++ b/checkout.h @@ -0,0 +1,13 @@ +#ifndef CHECKOUT_H +#define CHECKOUT_H + +#include "cache.h" + +/* + * Check if the branch name uniquely matches a branch name on a remote + * tracking branch. Return the name of the remote if such a branch + * exists, NULL otherwise. + */ +extern const char *unique_tracking_name(const char *name, struct object_id *oid); + +#endif /* CHECKOUT_H */ -- 2.15.0.426.gb06021eeb
[PATCH v6 0/6] make git worktree add dwim more
The previous rounds were at https://public-inbox.org/git/20171112134305.3949-1-t.gumme...@gmail.com/, https://public-inbox.org/git/20171118181103.28354-1-t.gumme...@gmail.com/, https://public-inbox.org/git/20171118224706.13810-1-t.gumme...@gmail.com/, https://public-inbox.org/git/2017113020.2780-1-t.gumme...@gmail.com/ and https://public-inbox.org/git/20171126194356.16187-1-t.gumme...@gmail.com. Thanks Junio for the review of the last round! Changes since the last round: - rephrased documentation and commit messaegs a bit use the - established pattern and call git_config only once, instead of calling it multiple times. Interdiff below: diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index fd841886ef..89ad0faecf 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -116,11 +116,11 @@ OPTIONS in linkgit:git-read-tree[1]. --[no-]guess-remote:: - With `add`, instead of creating a new branch from HEAD when - `` is not given, if there exists a tracking branch - in exactly one remote matching the basename of the path, base - the new branch on the remote-tracking branch, and mark the - remote-tracking branch as "upstream" from the new branch. + With `worktree add `, withouth ``, instead + of creating a new branch from HEAD, if there exists a tracking + branch in exactly one remote matching the basename of `, + base the new branch on the remote-tracking branch, and mark + the remote-tracking branch as "upstream" from the new branch. + This can also be set up as the default behaviour by using the `worktree.guessRemote` config option. diff --git a/builtin/worktree.c b/builtin/worktree.c index 426aea8761..002a569a11 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -43,7 +43,7 @@ static int git_worktree_config(const char *var, const char *value, void *cb) return 0; } - return 0; + return git_default_config(var, value, cb); } static int prune_worktree(const char *id, struct strbuf *reason) @@ -371,8 +371,6 @@ static int add(int ac, const char **av, const char *prefix) OPT_END() }; - git_config(git_worktree_config, NULL); - memset(&opts, 0, sizeof(opts)); opts.checkout = 1; ac = parse_options(ac, av, prefix, options, worktree_usage, 0); @@ -603,7 +601,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix) OPT_END() }; - git_config(git_default_config, NULL); + git_config(git_worktree_config, NULL); if (ac < 2) usage_with_options(worktree_usage, options); Thomas Gummerer (6): checkout: factor out functions to new lib file worktree: add can be created from any commit-ish worktree: add --[no-]track option to the add subcommand worktree: make add dwim worktree: add --guess-remote flag to add subcommand add worktree.guessRemote config option Documentation/config.txt | 10 Documentation/git-worktree.txt | 44 ++ Makefile | 1 + builtin/checkout.c | 41 + builtin/worktree.c | 46 ++- checkout.c | 43 ++ checkout.h | 13 + t/t2025-worktree-add.sh| 130 + 8 files changed, 277 insertions(+), 51 deletions(-) create mode 100644 checkout.c create mode 100644 checkout.h -- 2.15.0.426.gb06021eeb
[PATCH v6 5/6] worktree: add --guess-remote flag to add subcommand
Currently 'git worktree add ' creates a new branch named after the basename of the , that matches the HEAD of whichever worktree we were on when calling "git worktree add ". It's sometimes useful to have 'git worktree add behave more like the dwim machinery in 'git checkout ', i.e. check if the new branch name, derived from the basename of the , uniquely matches the branch name of a remote-tracking branch, and if so check out that branch and set the upstream to the remote-tracking branch. Add a new --guess-remote option that enables exactly that behaviour. Signed-off-by: Thomas Gummerer --- Documentation/git-worktree.txt | 7 +++ builtin/worktree.c | 10 ++ t/t2025-worktree-add.sh| 29 + 3 files changed, 46 insertions(+) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 3044d305a6..a4ffee5e08 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -115,6 +115,13 @@ OPTIONS such as configuring sparse-checkout. See "Sparse checkout" in linkgit:git-read-tree[1]. +--[no-]guess-remote:: + With `worktree add `, withouth ``, instead + of creating a new branch from HEAD, if there exists a tracking + branch in exactly one remote matching the basename of `, + base the new branch on the remote-tracking branch, and mark + the remote-tracking branch as "upstream" from the new branch. + --[no-]track:: When creating a new branch, if `` is a branch, mark it as "upstream" from the new branch. This is the diff --git a/builtin/worktree.c b/builtin/worktree.c index 7021d02585..15cb1600ee 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -343,6 +343,7 @@ static int add(int ac, const char **av, const char *prefix) char *path; const char *branch; const char *opt_track = NULL; + int guess_remote = 0; struct option options[] = { OPT__FORCE(&opts.force, N_("checkout even if already checked out in other worktree")), OPT_STRING('b', NULL, &opts.new_branch, N_("branch"), @@ -355,6 +356,8 @@ static int add(int ac, const char **av, const char *prefix) OPT_PASSTHRU(0, "track", &opt_track, NULL, N_("set up tracking mode (see git-branch(1))"), PARSE_OPT_NOARG | PARSE_OPT_OPTARG), + OPT_BOOL(0, "guess-remote", &guess_remote, +N_("try to match the new branch name with a remote-tracking branch")), OPT_END() }; @@ -389,6 +392,13 @@ static int add(int ac, const char **av, const char *prefix) int n; const char *s = worktree_basename(path, &n); opts.new_branch = xstrndup(s, n); + if (guess_remote) { + struct object_id oid; + const char *remote = + unique_tracking_name(opts.new_branch, &oid); + if (remote) + branch = remote; + } } if (ac == 2 && !opts.new_branch && !opts.detach) { diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 96ebc63d04..d25c774cb7 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -384,4 +384,33 @@ test_expect_success '"add" dwims' ' ) ' +test_expect_success 'git worktree add does not match remote' ' + test_when_finished rm -rf repo_a repo_b foo && + setup_remote_repo repo_a repo_b && + ( + cd repo_b && + git worktree add ../foo + ) && + ( + cd foo && + test_must_fail git config "branch.foo.remote" && + test_must_fail git config "branch.foo.merge" && + ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo + ) +' + +test_expect_success 'git worktree add --guess-remote sets up tracking' ' + test_when_finished rm -rf repo_a repo_b foo && + setup_remote_repo repo_a repo_b && + ( + cd repo_b && + git worktree add --guess-remote ../foo + ) && + ( + cd foo && + test_branch_upstream foo repo_a foo && + test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo + ) +' + test_done -- 2.15.0.426.gb06021eeb
[PATCH v6 6/6] add worktree.guessRemote config option
Some users might want to have the --guess-remote option introduced in the previous commit on by default, so they don't have to type it out every time they create a new worktree. Add a config option worktree.guessRemote that allows users to configure the default behaviour for themselves. Signed-off-by: Thomas Gummerer --- Documentation/config.txt | 10 ++ Documentation/git-worktree.txt | 3 +++ builtin/worktree.c | 14 -- t/t2025-worktree-add.sh| 31 +++ 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 5f65fa9234..4966d90ebb 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -3425,3 +3425,13 @@ web.browser:: Specify a web browser that may be used by some commands. Currently only linkgit:git-instaweb[1] and linkgit:git-help[1] may use it. + +worktree.guessRemote:: + With `add`, if no branch argument, and neither of `-b` nor + `-B` nor `--detach` are given, the command defaults to + creating a new branch from HEAD. If `worktree.guessRemote` is + set to true, `worktree add` tries to find a remote-tracking + branch whose name uniquely matches the new branch name. If + such a branch exists, it is checked out and set as "upstream" + for the new branch. If no such match can be found, it falls + back to creating a new branch from the current HEAD. diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index a4ffee5e08..89ad0faecf 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -121,6 +121,9 @@ OPTIONS branch in exactly one remote matching the basename of `, base the new branch on the remote-tracking branch, and mark the remote-tracking branch as "upstream" from the new branch. ++ +This can also be set up as the default behaviour by using the +`worktree.guessRemote` config option. --[no-]track:: When creating a new branch, if `` is a branch, diff --git a/builtin/worktree.c b/builtin/worktree.c index 15cb1600ee..002a569a11 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -33,8 +33,19 @@ struct add_opts { static int show_only; static int verbose; +static int guess_remote; static timestamp_t expire; +static int git_worktree_config(const char *var, const char *value, void *cb) +{ + if (!strcmp(var, "worktree.guessremote")) { + guess_remote = git_config_bool(var, value); + return 0; + } + + return git_default_config(var, value, cb); +} + static int prune_worktree(const char *id, struct strbuf *reason) { struct stat st; @@ -343,7 +354,6 @@ static int add(int ac, const char **av, const char *prefix) char *path; const char *branch; const char *opt_track = NULL; - int guess_remote = 0; struct option options[] = { OPT__FORCE(&opts.force, N_("checkout even if already checked out in other worktree")), OPT_STRING('b', NULL, &opts.new_branch, N_("branch"), @@ -591,7 +601,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix) OPT_END() }; - git_config(git_default_config, NULL); + git_config(git_worktree_config, NULL); if (ac < 2) usage_with_options(worktree_usage, options); diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index d25c774cb7..6ce9b9c070 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -413,4 +413,35 @@ test_expect_success 'git worktree add --guess-remote sets up tracking' ' ) ' +test_expect_success 'git worktree add with worktree.guessRemote sets up tracking' ' + test_when_finished rm -rf repo_a repo_b foo && + setup_remote_repo repo_a repo_b && + ( + cd repo_b && + git config worktree.guessRemote true && + git worktree add ../foo + ) && + ( + cd foo && + test_branch_upstream foo repo_a foo && + test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo + ) +' + +test_expect_success 'git worktree --no-guess-remote option overrides config' ' + test_when_finished rm -rf repo_a repo_b foo && + setup_remote_repo repo_a repo_b && + ( + cd repo_b && + git config worktree.guessRemote true && + git worktree add --no-guess-remote ../foo + ) && + ( + cd foo && + test_must_fail git config "branch.foo.remote" && + test_must_fail git config "branch.foo.merge" && + ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo + ) +' + test_done -- 2.15.0.426.gb06021eeb
[PATCH v6 4/6] worktree: make add dwim
Currently 'git worktree add ', errors out when 'branch' is not a local branch. It has no additional dwim'ing features that one might expect. Make it behave more like 'git checkout ' when the branch doesn't exist locally, but a remote tracking branch uniquely matches the desired branch name, i.e. create a new branch from the remote tracking branch and set the upstream to the remote tracking branch. As 'git worktree add' currently just dies in this situation, there are no backwards compatibility worries when introducing this feature. Signed-off-by: Thomas Gummerer --- Documentation/git-worktree.txt | 8 builtin/worktree.c | 16 t/t2025-worktree-add.sh| 19 +++ 3 files changed, 43 insertions(+) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 15e58b18f7..3044d305a6 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -52,6 +52,14 @@ is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. `-` may also be specified as ``; it is synonymous with `@{-1}`. + +If is a branch name (call it `` and is not found, +and neither `-b` nor `-B` nor `--detach` are used, but there does +exist a tracking branch in exactly one remote (call it ``) +with a matching name, treat as equivalent to + +$ git worktree add --track -b / + ++ If `` is omitted and neither `-b` nor `-B` nor `--detach` used, then, as a convenience, a new branch based at HEAD is created automatically, as if `-b $(basename )` was specified. diff --git a/builtin/worktree.c b/builtin/worktree.c index ea9678cac8..7021d02585 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "checkout.h" #include "config.h" #include "builtin.h" #include "dir.h" @@ -390,6 +391,21 @@ static int add(int ac, const char **av, const char *prefix) opts.new_branch = xstrndup(s, n); } + if (ac == 2 && !opts.new_branch && !opts.detach) { + struct object_id oid; + struct commit *commit; + const char *remote; + + commit = lookup_commit_reference_by_name(branch); + if (!commit) { + remote = unique_tracking_name(branch, &oid); + if (remote) { + opts.new_branch = branch; + branch = remote; + } + } + } + if (opts.new_branch) { struct child_process cp = CHILD_PROCESS_INIT; cp.git_cmd = 1; diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 72e8b62927..96ebc63d04 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -365,4 +365,23 @@ test_expect_success '--no-track avoids setting up tracking' ' ) ' +test_expect_success '"add" fails' ' + test_must_fail git worktree add foo non-existent +' + +test_expect_success '"add" dwims' ' + test_when_finished rm -rf repo_upstream repo_dwim foo && + setup_remote_repo repo_upstream repo_dwim && + git init repo_dwim && + ( + cd repo_dwim && + git worktree add ../foo foo + ) && + ( + cd foo && + test_branch_upstream foo repo_upstream foo && + test_cmp_rev refs/remotes/repo_upstream/foo refs/heads/foo + ) +' + test_done -- 2.15.0.426.gb06021eeb
[PATCH v6 3/6] worktree: add --[no-]track option to the add subcommand
Currently 'git worktree add' sets up tracking branches if '' is a remote tracking branch, and doesn't set them up otherwise, as is the default for 'git branch'. This may or may not be what the user wants. Allow overriding this behaviour with a --[no-]track flag that gets passed through to 'git branch'. We already respect branch.autoSetupMerge, as 'git worktree' just calls 'git branch' internally. Signed-off-by: Thomas Gummerer --- Documentation/git-worktree.txt | 6 + builtin/worktree.c | 8 +++ t/t2025-worktree-add.sh| 51 ++ 3 files changed, 65 insertions(+) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 121895209d..15e58b18f7 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -107,6 +107,12 @@ OPTIONS such as configuring sparse-checkout. See "Sparse checkout" in linkgit:git-read-tree[1]. +--[no-]track:: + When creating a new branch, if `` is a branch, + mark it as "upstream" from the new branch. This is the + default if `` is a remote-tracking branch. See + "--track" in linkgit:git-branch[1] for details. + --lock:: Keep the working tree locked after creation. This is the equivalent of `git worktree lock` after `git worktree add`, diff --git a/builtin/worktree.c b/builtin/worktree.c index ed043d5f1c..ea9678cac8 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -341,6 +341,7 @@ static int add(int ac, const char **av, const char *prefix) const char *new_branch_force = NULL; char *path; const char *branch; + const char *opt_track = NULL; struct option options[] = { OPT__FORCE(&opts.force, N_("checkout even if already checked out in other worktree")), OPT_STRING('b', NULL, &opts.new_branch, N_("branch"), @@ -350,6 +351,9 @@ static int add(int ac, const char **av, const char *prefix) OPT_BOOL(0, "detach", &opts.detach, N_("detach HEAD at named commit")), OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")), OPT_BOOL(0, "lock", &opts.keep_locked, N_("keep the new working tree locked")), + OPT_PASSTHRU(0, "track", &opt_track, NULL, +N_("set up tracking mode (see git-branch(1))"), +PARSE_OPT_NOARG | PARSE_OPT_OPTARG), OPT_END() }; @@ -394,9 +398,13 @@ static int add(int ac, const char **av, const char *prefix) argv_array_push(&cp.args, "--force"); argv_array_push(&cp.args, opts.new_branch); argv_array_push(&cp.args, branch); + if (opt_track) + argv_array_push(&cp.args, opt_track); if (run_command(&cp)) return -1; branch = opts.new_branch; + } else if (opt_track) { + die(_("--[no-]track can only be used if a new branch is created")); } UNLEAK(path); diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index b5c47ac602..72e8b62927 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -313,5 +313,56 @@ test_expect_success 'checkout a branch under bisect' ' test_expect_success 'rename a branch under bisect not allowed' ' test_must_fail git branch -M under-bisect bisect-with-new-name ' +# Is branch "refs/heads/$1" set to pull from "$2/$3"? +test_branch_upstream () { + printf "%s\n" "$2" "refs/heads/$3" >expect.upstream && + { + git config "branch.$1.remote" && + git config "branch.$1.merge" + } >actual.upstream && + test_cmp expect.upstream actual.upstream +} + +test_expect_success '--track sets up tracking' ' + test_when_finished rm -rf track && + git worktree add --track -b track track master && + test_branch_upstream track . master +' + +# setup remote repository $1 and repository $2 with $1 set up as +# remote. The remote has two branches, master and foo. +setup_remote_repo () { + git init $1 && + ( + cd $1 && + test_commit $1_master && + git checkout -b foo && + test_commit upstream_foo + ) && + git init $2 && + ( + cd $2 && + test_commit $2_master && + git remote add $1 ../$1 && + git config remote.$1.fetch \ + "refs/heads/*:refs/remotes/$1/*" && + git fetch --all + ) +} + +test_expect_success '--no-track avoids setting up tracking' ' + test_when_finished rm -rf repo_upstream repo_local foo && + setup_remote_repo repo_upstream repo_local && + ( + cd repo_local && + git worktree add --no-track -b foo ../foo repo_upstream/foo + ) && + (
[PATCH v6 2/6] worktree: add can be created from any commit-ish
Currently 'git worktree add' is documented to take an optional argument, which is checked out in the new worktree. However it is more generally possible to use a commit-ish as the optional argument, and check that out into the new worktree. Document that this is a possibility, as new users of git worktree add might find it helpful. Reported-by: Randall S. Becker Signed-off-by: Thomas Gummerer --- Documentation/git-worktree.txt | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index b472acc356..121895209d 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple working trees SYNOPSIS [verse] -'git worktree add' [-f] [--detach] [--checkout] [--lock] [-b ] [] +'git worktree add' [-f] [--detach] [--checkout] [--lock] [-b ] [] 'git worktree list' [--porcelain] 'git worktree lock' [--reason ] 'git worktree prune' [-n] [-v] [--expire ] @@ -45,14 +45,14 @@ specifying `--reason` to explain why the working tree is locked. COMMANDS -add []:: +add []:: -Create `` and checkout `` into it. The new working directory +Create `` and checkout `` into it. The new working directory is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. `-` may also be -specified as ``; it is synonymous with `@{-1}`. +specified as ``; it is synonymous with `@{-1}`. + -If `` is omitted and neither `-b` nor `-B` nor `--detach` used, +If `` is omitted and neither `-b` nor `-B` nor `--detach` used, then, as a convenience, a new branch based at HEAD is created automatically, as if `-b $(basename )` was specified. @@ -84,25 +84,25 @@ OPTIONS -f:: --force:: - By default, `add` refuses to create a new working tree when `` + By default, `add` refuses to create a new working tree when `` is a branch name and is already checked out by another working tree. This option overrides that safeguard. -b :: -B :: With `add`, create a new branch named `` starting at - ``, and check out `` into the new working tree. - If `` is omitted, it defaults to HEAD. + ``, and check out `` into the new working tree. + If `` is omitted, it defaults to HEAD. By default, `-b` refuses to create a new branch if it already exists. `-B` overrides this safeguard, resetting `` to - ``. + ``. --detach:: With `add`, detach HEAD in the new working tree. See "DETACHED HEAD" in linkgit:git-checkout[1]. --[no-]checkout:: - By default, `add` checks out ``, however, `--no-checkout` can + By default, `add` checks out ``, however, `--no-checkout` can be used to suppress checkout in order to make customizations, such as configuring sparse-checkout. See "Sparse checkout" in linkgit:git-read-tree[1]. -- 2.15.0.426.gb06021eeb
Re: [PATCH on en/rename-progress] diffcore-rename: make diff-tree -l0 mean -l
Elijah Newren wrote: > On Wed, Nov 29, 2017 at 10:32 AM, Jonathan Tan > wrote: >> In the documentation of diff-tree, it is stated that the -l option >> "prevents rename/copy detection from running if the number of >> rename/copy targets exceeds the specified number". The documentation >> does not mention any special handling for the number 0, but the >> implementation before commit b520abf ("sequencer: warn when internal >> merge may be suboptimal due to renameLimit", 2017-11-14) treated 0 as a >> special value indicating that the rename limit is to be a very large >> number instead. >> >> The commit b520abf changed that behavior, treating 0 as 0. Revert this >> behavior to what it was previously. This allows existing scripts and >> tools that use "-l0" to continue working. The alternative (to allow >> "-l0") is probably much less useful, since users can just refrain from I think in the parenthesis you mean 'to allow "-l0" to suppress rename detection', since this patch is all about allowing '-l0' already. >> specifying -M and/or -C to have the same effect. >> >> Signed-off-by: Jonathan Tan >> --- >> Note that this patch is built on en/rename-progress. >> >> We noticed this through an automated test for an internal tool - the >> tool uses git diff-tree with -l0, and no longer produces the same >> results as before. > > Thanks for testing that version and sending along the fix. > > I suspect the commit referenced twice in the commit message should > have been 9f7e4bfa3b ("diff: remove silent clamp of renameLimit", > 2017-11-13) rather than b520abf ("sequencer: warn when internal merge > may be suboptimal due to renameLimit", 2017-11-14). > > Other than that minor issue, patch and test looks good to me. Thanks, both. Looking at that patch, the fix is obviously correct. With Elijah's commit message tweak, Reviewed-by: Jonathan Nieder
Re: [PATCH v4 2/4] Makefile: add support for "perllibdir"
On Wed, Nov 29 2017, Dan Jacques jotted: > Add the "perllibdir" Makefile variable, which allows the customization > of the Perl library installation path. > > The Perl library installation path is currently left entirely to the > Perl Makefile implementation, either MakeMaker (default) or a fixed path > when NO_PERL_MAKEMAKER is enabled. This patch introduces "perllibdir", a > Makefile variable that can override that Perl module installation path. > > As with some other Makefile variables, "perllibdir" may be either > absolute or relative. In the latter case, it is treated as relative to > "$(prefix)". > > Add some incidental documentation to perl/Makefile. > > Explicitly specifying an installation path is necessary for Perl runtime > prefix support, as runtime prefix resolution code must know in advance > where the Perl support modules are installed. > > Signed-off-by: Dan Jacques > --- > Makefile | 18 +- > perl/Makefile | 52 ++-- > 2 files changed, 59 insertions(+), 11 deletions(-) > > diff --git a/Makefile b/Makefile > index f7c4ac207..80904f8b0 100644 > --- a/Makefile > +++ b/Makefile > @@ -462,6 +462,7 @@ ARFLAGS = rcs > # mandir > # infodir > # htmldir > +# perllibdir > # This can help installing the suite in a relocatable way. > > prefix = $(HOME) > @@ -1721,6 +1722,7 @@ gitexecdir_SQ = $(subst ','\'',$(gitexecdir)) > template_dir_SQ = $(subst ','\'',$(template_dir)) > htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative)) > prefix_SQ = $(subst ','\'',$(prefix)) > +perllibdir_SQ = $(subst ','\'',$(perllibdir)) > gitwebdir_SQ = $(subst ','\'',$(gitwebdir)) > > SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) > @@ -1955,17 +1957,22 @@ $(SCRIPT_PERL_GEN): perl/perl.mak > > perl/perl.mak: perl/PM.stamp > > -perl/PM.stamp: FORCE > +perl/PM.stamp: GIT-PERL-DEFINES FORCE > @$(FIND) perl -type f -name '*.pm' | sort >$@+ && \ > + cat GIT-PERL-DEFINES >>$@+ && \ > $(PERL_PATH) -V >>$@+ && \ > { cmp $@+ $@ >/dev/null 2>/dev/null || mv $@+ $@; } && \ > $(RM) $@+ > > -perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL > - $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' > prefix='$(prefix_SQ)' $(@F) > - > PERL_HEADER_TEMPLATE = perl/header_fixed_prefix.pl.template > -PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ) > + > +PERL_DEFINES := $(PERL_PATH_SQ) $(PERLLIB_EXTRA_SQ) > +PERL_DEFINES += $(NO_PERL_MAKEMAKER) > +PERL_DEFINES += $(perllibdir) > + > +perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL > + $(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' \ > + prefix='$(prefix_SQ)' perllibdir='$(perllibdir_SQ)' $(@F) > > $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak GIT-PERL-DEFINES > GIT-PERL-HEADER GIT-VERSION-FILE > $(QUIET_GEN)$(RM) $@ $@+ && \ > @@ -1979,6 +1986,7 @@ $(SCRIPT_PERL_GEN): % : %.perl perl/perl.mak > GIT-PERL-DEFINES GIT-PERL-HEADER GI > chmod +x $@+ && \ > mv $@+ $@ > > +PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES)) > GIT-PERL-DEFINES: FORCE > @FLAGS='$(PERL_DEFINES)'; \ > if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \ > diff --git a/perl/Makefile b/perl/Makefile > index f657de20e..b2aeeb0d8 100644 > --- a/perl/Makefile > +++ b/perl/Makefile > @@ -1,6 +1,22 @@ > # > # Makefile for perl support modules and routine > # > +# This Makefile generates "perl.mak", which contains the actual build and > +# installation directions. > +# > +# PERL_PATH must be defined to be the path of the Perl interpreter to use. > +# > +# prefix must be defined as the Git installation prefix. > +# > +# localedir must be defined as the path to the locale data. > +# > +# perllibdir may be optionally defined to override the default Perl module > +# installation directory, which is relative to prefix. If perllibdir is not > +# absolute, it will be treated as relative to prefix. > +# > +# NO_PERL_MAKEMAKER may be defined to use a built-in Makefile generation > method > +# instead of Perl MakeMaker. > + > makfile:=perl.mak > modules = > > @@ -12,6 +28,16 @@ ifndef V > QUIET = @ > endif > > +# If a library directory is provided, and it is not an absolute path, resolve > +# it relative to prefix. > +ifneq ($(perllibdir),) > +ifneq ($(filter /%,$(firstword $(perllibdir))),) > +perllib_instdir = $(perllibdir) > +else > +perllib_instdir = $(prefix)/$(perllibdir) > +endif > +endif Maybe I'm missing something, but isn't this "perllibdir can be relative" aim orthagonal to what this patch series is about? I.e. we could just avoid this work by saying you must say "prefix=/usr perllibdir=/usr/perl" instead of "prefix=/usr perllibdir=perl" as this allows. I started going down this route with my own patch and just threw it away. > all install instlibdir: $(makfile) > $(QUIET)$(MAKE) -f $(makfile) $@ > > @@ -25,7 +51,12 @@ clean: > $(makfile): PM.stamp > > ifde
[PATCH on en/rename-progress v2] diffcore-rename: make diff-tree -l0 mean -l
In the documentation of diff-tree, it is stated that the -l option "prevents rename/copy detection from running if the number of rename/copy targets exceeds the specified number". The documentation does not mention any special handling for the number 0, but the implementation before commit 9f7e4bfa3b ("diff: remove silent clamp of renameLimit", 2017-11-13) treated 0 as a special value indicating that the rename limit is to be a very large number instead. The commit 9f7e4bfa3b changed that behavior, treating 0 as 0. Revert this behavior to what it was previously. This allows existing scripts and tools that use "-l0" to continue working. The alternative (to have "-l0" suppress rename detection) is probably much less useful, since users can just refrain from specifying -M and/or -C to have the same effect. Signed-off-by: Jonathan Tan --- There are multiple issues with the commit message, so I am sending this new version out. v2 is exactly the same as previously, except that the commit message is changed following Elijah Newren's and Jonathan Nieder's comments. --- diffcore-rename.c | 2 ++ t/t4001-diff-rename.sh | 15 +++ 2 files changed, 17 insertions(+) diff --git a/diffcore-rename.c b/diffcore-rename.c index 9ca0eaec7..245e999fe 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -392,6 +392,8 @@ static int too_many_rename_candidates(int num_create, * *num_create * num_src > rename_limit * rename_limit */ + if (rename_limit <= 0) + rename_limit = 32767; if ((num_create <= rename_limit || num_src <= rename_limit) && ((uint64_t)num_create * (uint64_t)num_src <= (uint64_t)rename_limit * (uint64_t)rename_limit)) diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh index 0d1fa45d2..eadf4f624 100755 --- a/t/t4001-diff-rename.sh +++ b/t/t4001-diff-rename.sh @@ -230,4 +230,19 @@ test_expect_success 'rename pretty print common prefix and suffix overlap' ' test_i18ngrep " d/f/{ => f}/e " output ' +test_expect_success 'diff-tree -l0 defaults to a big rename limit, not zero' ' + test_write_lines line1 line2 line3 >myfile && + git add myfile && + git commit -m x && + + test_write_lines line1 line2 line4 >myotherfile && + git rm myfile && + git add myotherfile && + git commit -m x && + + git diff-tree -M -l0 HEAD HEAD^ >actual && + # Verify that a rename from myotherfile to myfile was detected + grep "myotherfile.*myfile" actual +' + test_done -- 2.15.0.173.g9268cf4a2.dirty
Re: [PATCH on en/rename-progress v2] diffcore-rename: make diff-tree -l0 mean -l
Jonathan Tan wrote: > In the documentation of diff-tree, it is stated that the -l option > "prevents rename/copy detection from running if the number of > rename/copy targets exceeds the specified number". The documentation > does not mention any special handling for the number 0, but the > implementation before commit 9f7e4bfa3b ("diff: remove silent clamp of > renameLimit", 2017-11-13) treated 0 as a special value indicating that > the rename limit is to be a very large number instead. > > The commit 9f7e4bfa3b changed that behavior, treating 0 as 0. Revert > this behavior to what it was previously. This allows existing scripts > and tools that use "-l0" to continue working. The alternative (to have > "-l0" suppress rename detection) is probably much less useful, since > users can just refrain from specifying -M and/or -C to have the same > effect. > > Signed-off-by: Jonathan Tan > --- > v2 is exactly the same as previously, except that the commit message is > changed following Elijah Newren's and Jonathan Nieder's comments. > > diffcore-rename.c | 2 ++ > t/t4001-diff-rename.sh | 15 +++ > 2 files changed, 17 insertions(+) Reviewed-by: Jonathan Nieder Thanks again. > diff --git a/diffcore-rename.c b/diffcore-rename.c > index 9ca0eaec7..245e999fe 100644 > --- a/diffcore-rename.c > +++ b/diffcore-rename.c > @@ -392,6 +392,8 @@ static int too_many_rename_candidates(int num_create, >* >*num_create * num_src > rename_limit * rename_limit >*/ > + if (rename_limit <= 0) > + rename_limit = 32767; > if ((num_create <= rename_limit || num_src <= rename_limit) && > ((uint64_t)num_create * (uint64_t)num_src ><= (uint64_t)rename_limit * (uint64_t)rename_limit)) > diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh > index 0d1fa45d2..eadf4f624 100755 > --- a/t/t4001-diff-rename.sh > +++ b/t/t4001-diff-rename.sh > @@ -230,4 +230,19 @@ test_expect_success 'rename pretty print common prefix > and suffix overlap' ' > test_i18ngrep " d/f/{ => f}/e " output > ' > > +test_expect_success 'diff-tree -l0 defaults to a big rename limit, not zero' > ' > + test_write_lines line1 line2 line3 >myfile && > + git add myfile && > + git commit -m x && > + > + test_write_lines line1 line2 line4 >myotherfile && > + git rm myfile && > + git add myotherfile && > + git commit -m x && > + > + git diff-tree -M -l0 HEAD HEAD^ >actual && > + # Verify that a rename from myotherfile to myfile was detected > + grep "myotherfile.*myfile" actual > +' > + > test_done
Re: [PATCH v4 3/4] Makefile: add Perl runtime prefix support
On Wed, Nov 29 2017, Dan Jacques jotted: > Add a new Makefile flag, RUNTIME_PREFIX_PERL, which, when enabled, > configures Perl scripts to locate the Git installation's Perl support > libraries by resolving against the script's path, rather than > hard-coding that path at build-time. > [...] > diff --git a/perl/header_runtime_prefix.pl.template > b/perl/header_runtime_prefix.pl.template > new file mode 100644 > index 0..fb9a9924d > --- /dev/null > +++ b/perl/header_runtime_prefix.pl.template > @@ -0,0 +1,24 @@ > +# BEGIN RUNTIME_PREFIX_PERL generated code. > +# > +# This finds our Git::* libraries relative to the script's runtime path. > +BEGIN { > + use lib split /@@PATHSEP@@/, > + ( > + $ENV{GITPERLLIB} > + || > + do { > + require FindBin; > + require File::Spec; > + my $gitexecdir_relative = '@@GITEXECDIR@@'; > + my $perllibdir_relative = '@@PERLLIBDIR@@'; > + > + ($FindBin::Bin =~ m=${gitexecdir_relative}$=) || > + die('Unrecognized runtime path.'); > + my $prefix = substr($FindBin::Bin, 0, > -length($gitexecdir_relative)); > + my $perllibdir = File::Spec->catdir($prefix, > $perllibdir_relative); > + (-e $perllibdir) || die("Invalid library path: > $perllibdir"); > + $perllibdir; > + } > + ); > +} > +# END RUNTIME_PREFIX_PERL generated code. Ah, I see. To answer my own question in <87lgiovokg@evledraar.booking.com> you're making this stuff a relative path so you can use it here later on. I.e. we $FindBin::Bin, and then go from there. Makes sense. We could use $ENV{GIT_EXEC_PATH} instead of FindBin here though, I missed that the first time. But that's just a nano-optimization. I just wondered whether git wasn't already passing us this info. There is one remaining bug here. Git::I18N isn't doing the right thing, I installed in /tmp/git and moved to /tmp/git2, and it has: our $TEXTDOMAINDIR = $ENV{GIT_TEXTDOMAINDIR} || '/tmp/git/share/locale'; And GIT_TEXTDOMAINDIR is not passed by git (it's only used for the tests IIRC). Would need a similar treatment as this. Easiest to just set the path we find here in $Git::Whatever and pick it up in $Git::I18N later, it's not like anyone uses it outside of git.git. But that does raise a more general concern for me. Isn't there some way we can run the test suite against an installed git (don't remember), then build, install, move the dir, and run the tests from the moved dir. That would have caught this bug, and anything else that may be lurking still.
Re: [PATCH v4 3/4] Makefile: add Perl runtime prefix support
On Wed, Nov 29 2017, Dan Jacques jotted: > + use lib split /@@PATHSEP@@/, > + ( > + $ENV{GITPERLLIB} > + || > + do { > + require FindBin; > + require File::Spec; > + my $gitexecdir_relative = '@@GITEXECDIR@@'; > + my $perllibdir_relative = '@@PERLLIBDIR@@'; > + > + ($FindBin::Bin =~ m=${gitexecdir_relative}$=) || > + die('Unrecognized runtime path.'); > + my $prefix = substr($FindBin::Bin, 0, > -length($gitexecdir_relative)); > + my $perllibdir = File::Spec->catdir($prefix, > $perllibdir_relative); > + (-e $perllibdir) || die("Invalid library path: > $perllibdir"); > + $perllibdir; > + } > + ); > +} > +# END RUNTIME_PREFIX_PERL generated code. Noticed after I sent the last E-Mail, this is missing @@INSTLIBDIR@@ which per my reading of it being initially introduced should be here in addition to this relative path. My reading of the intial patch that added it, as indicated in my patch, is that it's the dir we're going to be installing our libs to, so I didn't fiddle with it, but I think with your patches we need that dir *and* or own $perllibdir.
Re: [PATCH v4 0/4] RUNTIME_PREFIX relocatable Git
On Wed, Nov 29 2017, Dan Jacques jotted: > This is a small update to incorporate some Windows fixes from Johannes. > At this point, it passes the full test suite on Linux, Mac, and FreeBSD, > as well as the Travis.ci test suites, with and without > RUNTIME_PREFIX/RUNTIME_PREFIX_PERL flags. > > I'm happy with the patch set, and feel that it is ready to move forward. > However, while it's been looked at by several people, and I have > incorporated reviewer feedback, the patch set hasn't received any formal > LGTM-style responses yet. I'm not sure what standard of review is required > to move forward with a patch on this project, so maybe this is totally > fine, but I wanted to make sure to point this out. > > I also want to note Ævar Arnfjörð Bjarmason's RFC: > https://public-inbox.org/git/20171129153436.24471-1-ava...@gmail.com/T/ > > The proposed patch set conflicts with the Perl installation directory > changes in this patch set, as avarab@ notes. The proposed Perl installation > process would simplify patch 0002 in this patch set. I don't think the > landing order is terribly impactful - if this lands first, the patch in the > RFC would delete a few more lines, and if this lands later, patch 0002 > would largely not be necessary. In general this whole thing structurally looks good to me with the caveats noted in other review E-Mails. I haven't done anything but skim the details of the "where's my executable" C code though, just looked at what it's doing structurally. I think it makes sense for this to land first ahead of my patch. This is an actual feature you need, whereas I just hate our use of MakeMaker, but that can wait, unless you're keen to rebase this on my patch. Would probably make your whole diff a bit shorter. The whole converting our absolute to relative paths in the make code is unavoidably ugly, but after having an initial knee-jerk reaction to it I don't see how it can be avoided. I was hoping most of these paths could/would just be a fixed path away from our libexec path, but alas due to having had these configurable all along that simplicity seems out of reach. Maybe I asked this before, but I don't see any obvious reason for why RUNTIME_PREFIX_PERL is a different thing than RUNTIME_PREFIX as opposed to us just doing the right thing for the perl scripts.
Re: [PATCH] pathspec: only match across submodule boundaries when requested
Hi Brandon, On Tue, 28 Nov 2017, Brandon Williams wrote: > Commit 74ed43711fd (grep: enable recurse-submodules to work on > objects, 2016-12-16) taught 'tree_entry_interesting()' to be able to > match across submodule boundaries in the presence of wildcards. This is > done by performing literal matching up to the first wildcard and then > punting to the submodule itself to perform more accurate pattern > matching. Instead of introducing a new flag to request this behavior, > commit 74ed43711fd overloaded the already existing 'recursive' flag in > 'struct pathspec' to request this behavior. > > This leads to a bug where whenever any other caller has the 'recursive' > flag set as well as a pathspec with wildcards that all submodules will > be indicated as matches. One simple example of this is: > > git init repo > cd repo > > git init submodule > git -C submodule commit -m initial --allow-empty > > touch "[bracket]" > git add "[bracket]" > git commit -m bracket > git add submodule > git commit -m submodule > > git rev-list HEAD -- "[bracket]" > > Fix this by introducing the new flag 'recurse_submodules' in 'struct > pathspec' and using this flag to determine if matches should be allowed > to cross submodule boundaries. > > Signed-off-by: Brandon Williams Could you also add something like This fixes https://github.com/git-for-windows/git/issues/1371 at the end of the commit message, to keep a reference to the original bug report? > 4 files changed, 22 insertions(+), 2 deletions(-) Phew. That was much smaller than I expected. > +test_expect_success 'tree_entry_interesting does not match past submodule > boundaries' ' > + test_when_finished "rm -rf repo submodule" && > + git init submodule && > + test_commit -C submodule initial && > + git init repo && > + >"repo/[bracket]" && > + git -C repo add "[bracket]" && > + git -C repo commit -m bracket && > + git -C repo rev-list HEAD -- "[bracket]" >expect && > + > + git -C repo submodule add ../submodule && > + git -C repo commit -m submodule && > + > + git -C repo rev-list HEAD -- "[bracket]" >actual && > + test_cmp expect actual > +' Nicely prepared for a new hash function, too (no explicit SHA-1). I wonder, however, why we can't `git checkout -b bracket` and `test_when_finished "git checkout master"` and void those many `-C repo` options. But then, it is actually one of the shorter test cases, and pretty easy to understand. However, I would still like to see `test_tick`s before those `git commit` calls, to make the commit names reproducible. Thanks, Dscho
From Mrs. Serita Miner
My name is Mrs. Serita Miner is 53years old, I wish to make a donation of 3.5million dollars to you. As my doctor just confirmed to me that I have limited days to live due to the cancer problems I am suffering from. I have asked the lord to forgive me all my sins and I believe he has, because He is merciful. I will be going in for 3rd operation, and I pray that I survive the operation this third time I have decided to WILL/Donate this fund to charity through you for the good work of the lord, and to help the motherless, less privileged and also for the assistance of the widows genuinely. At the moment I cannot take any telephone calls, I have been restricted by my doctor, from taking telephone calls because I deserve all the rest I can get. And I need is from you please Full name: ___ Full Address: _ Nationality: Mobile Number: E-mail address: ___ Occupation: __ Country : __ Any form of Identification either Drivers license or International Passport: __ If you can assure me of being able to handle my offer please get back to me for more details by e-mailing me. ( seritaminer...@gmail.com ) miner...@yahoo.com Thank you, Expecting to hear from you soon, Mrs. Serita Miner.
Re: [PATCH 3/5] rebase -i: add exec commands via the rebase--helper
Hi Liam, On Tue, 28 Nov 2017, liam Beguin wrote: > I just realized we could maybe add exec instructions only after pick > commands if we do add-exec-commands before rearrange-squash. That won't work, because the squash/fixup commands are pick commands before rearrange-squash. So you'd add one unwanted exec per squash/fixup... Ciao, Dscho
Re: [PATCH 4/5] rebase -i: learn to abbreviate command names
Hi Liam, On Tue, 28 Nov 2017, liam Beguin wrote: > On 27/11/17 06:04 PM, Johannes Schindelin wrote: > > > > On Sun, 26 Nov 2017, Liam Beguin wrote: > > > >> @@ -2483,7 +2491,9 @@ int sequencer_make_script(int keep_empty, FILE *out, > >>strbuf_reset(&buf); > >>if (!keep_empty && is_original_commit_empty(commit)) > >>strbuf_addf(&buf, "%c ", comment_line_char); > >> - strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid)); > >> + strbuf_addf(&buf, "%s %s ", > >> + abbreviate_commands ? "p" : "pick", > >> + oid_to_hex(&commit->object.oid)); > > > > I guess the compiler will optimize this code so that the conditional > > is evaluated only once. Not that this is performance critical ;-) > > Is your guess enough? :-) If not, how could I make sure this is > optimized? Should I do that check before the while() loop? I am a fan of not relying too heavily on compiler optimization and e.g. extract code from loops when it does not need to be evaluated every single iteration. In this case: const char *pick = abbreviate_commands ? "p" : "pick"; ... strbuf_addf(&buf, "%s %s ", pick, oid_to_hex(&commit->object.oid)); But given Junio's comment that the assignment of `first` was too far away from the line where it is used for his taste, I guess he will argue (once again) the exact opposite of me. Ciao, Dscho
Re: [PATCH] git-prompt: fix reading files with windows line endings
Hi Johannes, On 29 Nov 2017 15:27, Johannes Schindelin wrote: >> diff --git a/contrib/completion/git-prompt.sh >> b/contrib/completion/git-prompt.sh >> index c6cbef38c2..71a64e7959 100644 >> --- a/contrib/completion/git-prompt.sh >> +++ b/contrib/completion/git-prompt.sh >> @@ -282,7 +282,7 @@ __git_eread () >> { >> local f="$1" >> shift >> -test -r "$f" && read "$@" <"$f" >> +test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}" > > As far as I understand, $'\r' is a Bash-only construct, and this file > (git-prompt.sh) is targeting other Unix shells, too. Sorry, I wasn't really aware about this bash-ism. I agree that a generic solution would be best. > So how about using `tr -d '\r' <"$f" | read "$@"` instead? That doesn't work for me. Apparently, the variable is always reset to "" and hence the prompt will always display the shortened sha1. Maybe it has something to do with variable scoping inside the backtick evaluation? > Or maybe keep with the Bash construct, but guarded behind a test that we > area actually running in Bash? Something like > > test -z "$BASH" || IFS=$' \t\r\n' Actually, this got me thinking and reading the POSIX.1-2008, specifically http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html. It seems POSIX states that IFS should be supported by read. This means that it should be okay to just do > test -r "$f" && IFS=" \t\r\n" read "$@" < "$f" This would also get rid of the export and avoid introducing backtick evaluation. Regards, Robert
Re: [PATCH v4 0/4] RUNTIME_PREFIX relocatable Git
> In general this whole thing structurally looks good to me with the > caveats noted in other review E-Mails. > > I haven't done anything but skim the details of the "where's my > executable" C code though, just looked at what it's doing structurally. > > I think it makes sense for this to land first ahead of my patch. This is > an actual feature you need, whereas I just hate our use of MakeMaker, > but that can wait, unless you're keen to rebase this on my patch. Would > probably make your whole diff a bit shorter. I'm not strictly pressed for time here, so we can put this off if that's strategically appropriate. Chromiuum, right now, just manually patches upstream Git with a similar patch set, so it's working and function. This is just an effort to upstream those changes for everyone else to enjoy! I think that landing in either order is probably okay, so if your RFC goes through pretty quickly I don't mind rebasing, but if this is otherwise good to go in v5, I wouldn't mind landing it and then removing the obsoleted parts during the Makefile update. > The whole converting our absolute to relative paths in the make code is > unavoidably ugly, but after having an initial knee-jerk reaction to it I > don't see how it can be avoided. I was hoping most of these paths > could/would just be a fixed path away from our libexec path, but alas > due to having had these configurable all along that simplicity seems out > of reach. Yeah I spent no small amount of time massaging that code hoping some better formulation would shake out, and this is what I ended up with. UNTIME_PREFIX_PERL is a specialty build with a stronger set of assumptions than the standard installation (RE prefix-relative paths). > Maybe I asked this before, but I don't see any obvious reason for why > RUNTIME_PREFIX_PERL is a different thing than RUNTIME_PREFIX as opposed > to us just doing the right thing for the perl scripts. I did this to isolate Windows builds from the Perl script changes. Git-for- Windows uses (invented) RUNTIME_PREFIX, but runs its Perl scripts in a MinGW sub-environment which, internally, has the Perl libraries installed at a fixed path, so it doesn't need any special resolution logic. I support that if Git-for-Windows wants to, a trivial future patch can merge the two, so I opted to play it safe and keep these changes isolated. === > We could use $ENV{GIT_EXEC_PATH} instead of FindBin here though, I > missed that the first time. But that's just a nano-optimization. I just > wondered whether git wasn't already passing us this info. Oh good idea - I'll go ahead and do this. > There is one remaining bug here. Git::I18N isn't doing the right thing, > I installed in /tmp/git and moved to /tmp/git2, and it has: > > our $TEXTDOMAINDIR = $ENV{GIT_TEXTDOMAINDIR} || '/tmp/git/share/locale'; > > And GIT_TEXTDOMAINDIR is not passed by git (it's only used for the tests > IIRC). Would need a similar treatment as this. Easiest to just set the > path we find here in $Git::Whatever and pick it up in $Git::I18N later, > it's not like anyone uses it outside of git.git. Good catch! I'm going to see what I can do here. > But that does raise a more general concern for me. Isn't there some way > we can run the test suite against an installed git (don't remember), > then build, install, move the dir, and run the tests from the moved dir. > > That would have caught this bug, and anything else that may be lurking > still. I am not aware of such an option, but I'll take a look again. This sort of thing might just require a reformulation of the test suite in general to make it run against a Git installation instead of the intermediate artifacts. A positive outcome would be the ability to remove the Perl path environment variable hacks ($ENV{...} || ...) and just use production resolution logic, increasing the test surface! But I think that's a bit more work than I have time for at the moment. === > Noticed after I sent the last E-Mail, this is missing @@INSTLIBDIR@@ > which per my reading of it being initially introduced should be here in > addition to this relative path. > > My reading of the intial patch that added it, as indicated in my patch, > is that it's the dir we're going to be installing our libs to, so I > didn't fiddle with it, but I think with your patches we need that dir > *and* or own $perllibdir. INSTLIBDIR is used for the standard fixed-prefix header, but not in this one. This one uses a combination of GITEXECDIR and PERLLIBDIR. PERLLIBDIR is effectively the prefix-relative part of INSTLIBDIR, so it's here in spirit! Thanks for taking the time to review! I'll go ahead and see what I can do RE your comments. Cheers, -Dan
Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)
Hi Hannes, On 29/11/2017 20:11, Johannes Sixt wrote: > > Ok, then please explain, how this process should work in my workflow > and with the `commit --onto-parent` feature that you have in mind. I > have an integration branch (which is a throw-away type, so you can > mangle it in any way you want); it is a series of merges: > > ...A...C<- topics A, C > \ \ E >---o---o---o---o I<- integration > / / > ...B...D<- topics B, D > > Now I find a bug in topic B. Assume that the merges of C and D have > textual conflicts with the integration branch (but not with B) and/or > may be evil. What should I do? > > With git-post, I make a fixup commit commit on the integration > branch, then `git post B && git merge B`: > > ...A...C <- topics A, C > \ \ >---o---o---o---o---f---F<- integration > / / / > ...B...D / <- topic D > \ / > f'--'<- topic B > > The merge F does not introduce any changes on the integration branch, > so I do not need it, but it helps keep topic B off radar when I ask > `git branch --no-merged` later. But you`re not committing (posting) on your HEAD`s (direct) parent in the first place (topic B), so `commit --onto-parent` isn`t right tool for the job... yet :) To make it easier to explain, I marked your integration branch initial head with "I" in the quote above (commit merging-in branch D), and marked commit merging-in branch C with "E". HEAD being currently on commit "I", you can only use `--onto-parent` option to commit onto "E" or "D", being parents of "I". To work with `--onto-parent` and be able to commit on top of any of the topic branches, you would need a situation like this instead: (1) ...C <- topic C | ...A | <- topic A \| ...o I<- integration /| ...B | <- topic B | ...D <- topic D With `commit --onto-parent` you would skip `git post B && git merge B` steps, where "fixup commit" would be done with `--onto-parent B`, So you end up in situation like this: (2) ...C <- topic C | ...A | <- topic A \| ...o I' <- integration /| ...B---f | <- topic B | ...D <- topic D State of index and working tree files in your F and my I' commit is exactly the same (I' = I + f), where in my case (2) history looks like "f" was part of topic B from the start, before integration merge happened. BUT, all this said, I understand that your starting position, where not all topic branches are merged at the same time (possibly to keep conflict resolution sane), is probably more often to be encountered in real-life work... and as existing `--onto-parent` machinery should be able to support it already, I`m looking forward to make it happen :) Once there, starting from your initial position: >...A...C<- topics A, C >\ \ E > ---o---o---o---o I<- integration <- HEAD >/ / >...B...D<- topics B, D ... and doing something like `git commit --onto B --merge` would yield: (3) ...A...C<- topics A, C \ \ E ---o---o---o---o I' <- integration / /| ...B...D | <- topic D \| f---' <- topic B ... where (I' = I + f) is still true. If that`s preferred in some cases, it could even look like this instead: (4) ...A...C <- topics A, C \ \ E I ---o---o---o---o---F <- integration / / / ...B...D / <- topic D \ / f---' <- topic B ... where F(4) = I'(3), so similar situation, just that we don`t discard I but post F on top of it. Good thing is all necessary logic should already be in place, I just need to think a bit about the most sane user interface, and get back to you. Thanks for invaluable input so far :) Of course, do feel free to drop any ideas you come up with as well, on how `git commit` user interface/options leading to (3) or (4) should look like (they could both be supported). > > Like working on multiple branches at the same time, in the manner > > similar to what `git add --patch` allows in regards to working on > > multiple commits at the same time. This just takes it on yet another > > level... hopefully :) > > 'kay, I'm not eagerly waiting for this particular next level (I > prefer to keep things plain and simple), but I would never say this > were a broken workflow. ;) Hehe, thanks, I guess :) Simplicity, but user-oriented, is the major point here, where you can work on unrelated patch series at the same time (patch queues?), without a need to switch branches, while you still ma
Re: [PATCH on en/rename-progress v2] diffcore-rename: make diff-tree -l0 mean -l
On Wed, Nov 29, 2017 at 12:11 PM, Jonathan Tan wrote: > In the documentation of diff-tree, it is stated that the -l option > "prevents rename/copy detection from running if the number of > rename/copy targets exceeds the specified number". The documentation > does not mention any special handling for the number 0, but the > implementation before commit 9f7e4bfa3b ("diff: remove silent clamp of > renameLimit", 2017-11-13) treated 0 as a special value indicating that > the rename limit is to be a very large number instead. > > The commit 9f7e4bfa3b changed that behavior, treating 0 as 0. Revert > this behavior to what it was previously. This allows existing scripts > and tools that use "-l0" to continue working. The alternative (to have > "-l0" suppress rename detection) is probably much less useful, since > users can just refrain from specifying -M and/or -C to have the same > effect. > > Signed-off-by: Jonathan Tan > --- > There are multiple issues with the commit message, so I am sending this > new version out. > > v2 is exactly the same as previously, except that the commit message is > changed following Elijah Newren's and Jonathan Nieder's comments. > --- > diffcore-rename.c | 2 ++ > t/t4001-diff-rename.sh | 15 +++ > 2 files changed, 17 insertions(+) Reviewed-by: Elijah Newren Thanks.
[PATCH] hashmap: adjust documentation to reflect reality
The hashmap API is just complicated enough that even at least one long-time Git contributor has to look up how to use it every time he finds a new use case. When that happens, it is really useful if the provided example code is correct... While at it, "fix a memory leak", avoid statements before variable declarations, fix a const -> no-const cast, several %l specifiers (which want to be %ld), avoid using an undefined constant, call scanf() correctly, use FLEX_ALLOC_STR() where appropriate, and adjust the style here and there. Signed-off-by: Johannes Schindelin --- hashmap.h | 60 +--- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/hashmap.h b/hashmap.h index 7cb29a6aede..7ce79f3f72c 100644 --- a/hashmap.h +++ b/hashmap.h @@ -18,75 +18,71 @@ * * #define COMPARE_VALUE 1 * - * static int long2string_cmp(const struct long2string *e1, + * static int long2string_cmp(const void *hashmap_cmp_fn_data, + *const struct long2string *e1, *const struct long2string *e2, - *const void *keydata, const void *userdata) + *const void *keydata) * { - * char *string = keydata; - * unsigned *flags = (unsigned*)userdata; + * const char *string = keydata; + * unsigned flags = *(unsigned *)hashmap_cmp_fn_data; * * if (flags & COMPARE_VALUE) - * return !(e1->key == e2->key) || (keydata ? - * strcmp(e1->value, keydata) : strcmp(e1->value, e2->value)); + * return e1->key != e2->key || + * strcmp(e1->value, string ? string : e2->value); * else - * return !(e1->key == e2->key); + * return e1->key != e2->key; * } * * int main(int argc, char **argv) * { * long key; - * char *value, *action; - * - * unsigned flags = ALLOW_DUPLICATE_KEYS; + * char value[255], action[32]; + * unsigned flags = 0; * * hashmap_init(&map, (hashmap_cmp_fn) long2string_cmp, &flags, 0); * - * while (scanf("%s %l %s", action, key, value)) { + * while (scanf("%s %ld %s", action, &key, value)) { * * if (!strcmp("add", action)) { * struct long2string *e; - * e = malloc(sizeof(struct long2string) + strlen(value)); + * FLEX_ALLOC_STR(e, value, value); * hashmap_entry_init(e, memhash(&key, sizeof(long))); * e->key = key; - * memcpy(e->value, value, strlen(value)); * hashmap_add(&map, e); * } * * if (!strcmp("print_all_by_key", action)) { - * flags &= ~COMPARE_VALUE; - * - * struct long2string k; + * struct long2string k, *e; * hashmap_entry_init(&k, memhash(&key, sizeof(long))); * k.key = key; * - * struct long2string *e = hashmap_get(&map, &k, NULL); + * flags &= ~COMPARE_VALUE; + * e = hashmap_get(&map, &k, NULL); * if (e) { - * printf("first: %l %s\n", e->key, e->value); - * while (e = hashmap_get_next(&map, e)) - * printf("found more: %l %s\n", e->key, e->value); + * printf("first: %ld %s\n", e->key, e->value); + * while ((e = hashmap_get_next(&map, e))) + * printf("found more: %ld %s\n", e->key, e->value); * } * } * * if (!strcmp("has_exact_match", action)) { - * flags |= COMPARE_VALUE; - * * struct long2string *e; - * e = malloc(sizeof(struct long2string) + strlen(value)); + * FLEX_ALLOC_STR(e, value, value); * hashmap_entry_init(e, memhash(&key, sizeof(long))); * e->key = key; - * memcpy(e->value, value, strlen(value)); * - * printf("%s found\n", hashmap_get(&map, e, NULL) ? "" : "not"); + * flags |= COMPARE_VALUE; + * printf("%sfound\n", hashmap_get(&map, e, NULL) ? "" : "not "); + * free(e); * } * * if (!strcmp("has_exact_match_no_heap_alloc", action)) { - * flags |= COMPARE_VALUE; - * - * struct long2string e; - * hashmap_entry_init(e, memhash(&key, sizeof(long))); - * e.key = key; + * struct long2string k; + * hashmap_entry_init(&k, memhash(&key, sizeof(long))); + * k.key = key; * - * printf("%s found\n", hashmap_get(&map, e, value) ? "" : "not"); + * flags |= COMPARE_VALUE; + * printf("%sfound\n", hashmap_get(&map, &k, value) ? "" : "not "); * } * * if (!strcmp("end", action)) { @@ -94,6 +90,8 @@ * break; * } * } + * + * return 0; * } */ base-commit: 1a4e40aa5dc16564af879142ba9dfbbb88d1e5ff -- 2.15.0.windows
Re: [ANNOUNCE] Git for Windows 2.15.1
Hi Johannes, On 29/11/2017 14:57, Johannes Schindelin wrote: > > * It is now possible to configure nano or Notepad++ as Git's > default editor instead of vim. This seems as a really nice option, as it could\should greatly help Windows people in lowering friction in first encounter with Git (for Windows). Being pretty unfamiliar with Linux and its tools at the time, I remember the initial frustration in trying to do what otherwise felt as a no-brain, simple and trivial task - write the damn commit message after `git commit`, lol. Even had to kill the bash window a few times, not knowing what to do, where it was clear it was expecting something from me :$ I later learned about vim, like getting started with Git wasn`t hard enough... :) As soon as I found it being a possibility, I`ve set Notepad++ as my default editor. That said, what is the Notepad++ as default editor option doing, just setting: [core] editor = 'F:/Install/Notepad++/notepad++.exe' -multiInst -notabbar -nosession ... inside users` .gitconfig (`git config --global`)? Asking because I already had it there, and seems the option made no difference, so I`m not sure if it actually worked. Otherwise, I guess I can dig the answer up from the installer code as well... :) Thanks for yet another Git for Windows release. Regards, Buga p.s. Ok, It seems it actually went to `git config --system` instead, like: [core] editor = 'F:\\Install\\Notepad++\\notepad++.exe' -multiInst -notabbar -nosession -noPlugin I removed my original (user) configuration, and it still works (minus plugins, due to that last parameter), so all good :) Plugins do come in handy for me, though, like spell-check when writing commit messages, so I`ll just stick with my option for now.
Re: [PATCH] hashmap: adjust documentation to reflect reality
Johannes Schindelin wrote: > The hashmap API is just complicated enough that even at least one > long-time Git contributor has to look up how to use it every time he > finds a new use case. When that happens, it is really useful if the > provided example code is correct... > > While at it, "fix a memory leak", avoid statements before variable > declarations, fix a const -> no-const cast, several %l specifiers (which > want to be %ld), avoid using an undefined constant, call scanf() > correctly, use FLEX_ALLOC_STR() where appropriate, and adjust the style > here and there. > > Signed-off-by: Johannes Schindelin > --- > hashmap.h | 60 +--- > 1 file changed, 29 insertions(+), 31 deletions(-) Yay, thanks for this. Reviewed-by: Jonathan Nieder Follow-on idea for the interested: would making a test that extracts this sample code from hashmap.h and confirms it still works be a crazy idea?
Re: [PATCH] git-prompt: fix reading files with windows line endings
Hi Robert, On Wed, 29 Nov 2017, Robert Abel wrote: > On 29 Nov 2017 15:27, Johannes Schindelin wrote: > > > Or maybe keep with the Bash construct, but guarded behind a test that we > > area actually running in Bash? Something like > > > > test -z "$BASH" || IFS=$' \t\r\n' > > Actually, this got me thinking and reading the POSIX.1-2008, specifically > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html. > > It seems POSIX states that IFS should be supported by read. Yes, that's what I meant: you could use IFS. > This means that it should be okay to just do > > > test -r "$f" && IFS=" \t\r\n" read "$@" < "$f" I am afraid that this won't work: when I call printf '123\r\n' | while IFS=" \t\r\n" read line do printf '%s' "$line" | hexdump -C done it prints 31 32 33 0d |123.| 0004 If I replace the double-quoted IFS by the dollar-single-quoted one, it works again. I think the reason is that \t, \r and \n are used literally when double-quoted, not as , and . Ciao, Johannes
Re: [PATCH v3] diff: support anchoring line(s)
Hi Jonathan, On Tue, 28 Nov 2017, Jonathan Tan wrote: > @@ -4607,7 +4627,14 @@ int diff_opt_parse(struct diff_options *options, > DIFF_XDL_CLR(options, NEED_MINIMAL); > options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK; > options->xdl_opts |= value; > + if (value == XDF_PATIENCE_DIFF) > + clear_patience_anchors(options); > return argcount; > + } else if (skip_prefix(arg, "--anchored=", &arg)) { > + options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF); > + ALLOC_GROW(options->anchors, options->anchors_nr + 1, > +options->anchors_alloc); > + options->anchors[options->anchors_nr++] = xstrdup(arg); I looked and failed to find the code that releases this array after the diff is done... did I miss anything? Ciao, Dscho
Re: [PATCH] git-prompt: fix reading files with windows line endings
> > diff --git a/contrib/completion/git-prompt.sh > > b/contrib/completion/git-prompt.sh > > index c6cbef38c2..71a64e7959 100644 > > --- a/contrib/completion/git-prompt.sh > > +++ b/contrib/completion/git-prompt.sh > > @@ -282,7 +282,7 @@ __git_eread () > > { > > local f="$1" > > shift > > - test -r "$f" && read "$@" <"$f" > > + test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}" I don't think that export is necessary here. > As far as I understand, $'\r' is a Bash-only construct, and this file > (git-prompt.sh) is targeting other Unix shells, too. The only other shell the prompt (and completion) script is targeting is ZSH, and ZSH understands this construct. We already use this construct to set IFS in several places in both scripts for a long time, so it should be fine here, too. Gábor
Re: [ANNOUNCE] Git for Windows 2.15.1
Hi Buga, On Thu, 30 Nov 2017, Igor Djordjevic wrote: > On 29/11/2017 14:57, Johannes Schindelin wrote: > > > > * It is now possible to configure nano or Notepad++ as Git's > > default editor instead of vim. > > This seems as a really nice option, as it could\should greatly help > Windows people in lowering friction in first encounter with Git (for > Windows). > > Being pretty unfamiliar with Linux and its tools at the time, I > remember the initial frustration in trying to do what otherwise felt > as a no-brain, simple and trivial task - write the damn commit > message after `git commit`, lol. Even had to kill the bash window a > few times, not knowing what to do, where it was clear it was > expecting something from me :$ > > I later learned about vim, like getting started with Git wasn`t hard > enough... :) As soon as I found it being a possibility, I`ve set > Notepad++ as my default editor. Thanks for this entertaining personal account! And yes, you guessed it, I wanted this option for a long time, but never got around to it (always hoping that somebody would beat me to it...). BTW this installer page is probably far from done, there is a lot of room for improvement, e.g. this up-for-grabs ticket: https://github.com/git-for-windows/git/issues/1356 (hint, hint ;-)) > That said, what is the Notepad++ as default editor option doing, just > setting: > > [core] > editor = 'F:/Install/Notepad++/notepad++.exe' -multiInst -notabbar > -nosession > > ... inside users` .gitconfig (`git config --global`)? As you found out, it is set in the system config. There are two reasons for that: - the installer runs as administrator, so it cannot know for which user you want to configure Notepad++ - in case the user does not like the setting (as in your case), they can still override it in their $HOME forever. Ciao, Johannes
[ANNOUNCE] Git for Windows 2.15.1(2), was Re:Git for Windows 2.15.1
Hi all, unfortunately, a last-minute bug slipped in: MSYS2 updated vim (including its defaults.vim, in a way that interacted badly with Git for Windows' configuration). The result was that an ugly warning is shown every time a user opens vim (which is still the default editor of Git for Windows, for historical reasons). Git for Windows v2.15.1(2) fixes just this one bug, and can be downloaded here: https://github.com/git-for-windows/git/releases/tag/v2.15.1.windows.2 Ciao, Johannes P.S.: Oh, yes, you're right, I forgot the SHA-256s: Git-2.15.1.2-64-bit.exe 5eaffe3f364c89d2811f572678b3d4bbd092b887f2dda1f2d12b7b81eea4e6a7 Git-2.15.1.2-32-bit.exe d227c0694fe1eef9a7299570167549aaf454bf28a5ac6d34ad3d288d9185f7e2 PortableGit-2.15.1.2-64-bit.7z.exe 36847f62418a5c62a7f50650f3662283f8d324602f4a611d592095ee296cd912 PortableGit-2.15.1.2-32-bit.7z.exe 8b1973bde82718684945c7373266976c70be55022ac554847a8a201c941952af MinGit-2.15.1.2-64-bit.zip 59060c93425709d66db1756f884d8f235b012d58f6de0963087afbac46134c57 MinGit-2.15.1.2-32-bit.zip 6d2e782c9fd714feab86a6a6358353da32adb4975eaca8a6532bc51aa0ad4ed9 MinGit-2.15.1.2-busybox-64-bit.zip 2d2f3ac49caf7296dd4299236ca9425dc11954b3486f5a3301bf7b7a63286dbc MinGit-2.15.1.2-busybox-32-bit.zip 2b25fcc2b27e8f1e75eae79912c59a35b066397195597600fd48e32140df0f24 Git-2.15.1.2-64-bit.tar.bz2 174f1a37f313a4ac6cb617693d10d28304d6b952121b41a934e5c484eb68444c Git-2.15.1.2-32-bit.tar.bz2 709c15a33c0c25f4b293438eef5cf8c501e84032be36a5f4de8815df1e927eaf On Wed, 29 Nov 2017, Johannes Schindelin wrote: > Dear Git users, > > It is my pleasure to announce that Git for Windows 2.15.1 is available from: > > https://git-for-windows.github.io/ > > Changes since Git for Windows v2.15.0 (October 30th 2017) > > New Features > > * Comes with Git v2.15.1. > * Operations in massively-sparse worktrees are now much faster if > core.fscache = true. > * It is now possible to configure nano or Notepad++ as Git's default > editor instead of vim. > * Comes with OpenSSL v1.0.2m. > * Git for Windows' updater now uses non-intrusive toast notifications > on Windows 8, 8.1 and 10. > * Running git fetch in a repository with lots of refs is now > considerably faster. > * Comes with cURL v7.57.0. > > Bug Fixes > > * The experimental --show-ignored-directory option of git status > which was removed in Git for Windows v2.15.0 without warning has > been reintroduced as a deprecated option. > * The git update command (to auto-update Git for Windows) will now > work also work behind proxies. > > Filename | SHA-256 > | --- > Git-2.15.1-64-bit.exe | > a2ba53197db79b361502836eecf97f09881703148774f9b4e9e6749d41d4ff71 > Git-2.15.1-32-bit.exe | > 73154bdfd1ad4ced8612d97b95603ff2b2383db9d46b4c308827fb5233d52592 > PortableGit-2.15.1-64-bit.7z.exe | > 94d485454af33a32d08680950aaf38f0825a189ef8b617054b81b2c48d817699 > PortableGit-2.15.1-32-bit.7z.exe | > 7d804748a7de735133d78c5420d9b338379123734509415035593e106b03515a > MinGit-2.15.1-64-bit.zip | > 5e38d13241b0742e6673bc5116ac82e851dd1fad01c660c943017f4549b6afea > MinGit-2.15.1-32-bit.zip | > 2fc85f67cabe3c13aceb6868b4bb6bda28ddfecd6f32d7e0da9ddce8cee9b940 > MinGit-2.15.1-busybox-64-bit.zip | > 02611486e3c8c427f09d2c4639484cd604ea812471248ae928960f1e0dc59633 > MinGit-2.15.1-busybox-32-bit.zip | > a6dfb770f5cfa7b120ba49922d3434577b8601c5d322ad473dd2e2adc91e92b3 > Git-2.15.1-64-bit.tar.bz2 | > bb630e5f3d7b650db67134b0187cf0cb8f5ed75990838ee65fed85e21777f08a > Git-2.15.1-32-bit.tar.bz2 | > ec3938e161ac1bbcf2b4f5d41fae1c05ea229fa0276b4db8530ec50b69131832 > > Ciao, > Johannes >
Re: [PATCH] git-prompt: fix reading files with windows line endings
Hi Gábor, On Thu, 30 Nov 2017, SZEDER Gábor wrote: > > > diff --git a/contrib/completion/git-prompt.sh > > > b/contrib/completion/git-prompt.sh > > > index c6cbef38c2..71a64e7959 100644 > > > --- a/contrib/completion/git-prompt.sh > > > +++ b/contrib/completion/git-prompt.sh > > > @@ -282,7 +282,7 @@ __git_eread () > > > { > > > local f="$1" > > > shift > > > - test -r "$f" && read "$@" <"$f" > > > + test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}" > > I don't think that export is necessary here. > > > As far as I understand, $'\r' is a Bash-only construct, and this file > > (git-prompt.sh) is targeting other Unix shells, too. > > The only other shell the prompt (and completion) script is targeting > is ZSH, and ZSH understands this construct. We already use this > construct to set IFS in several places in both scripts for a long > time, so it should be fine here, too. That's good to know! I should have `git grep`ped... Sorry for the noise, Johannes
Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL
(+cc: Nicolas) Hi, Doron Behar wrote: > I'm trying to send a patch with the command `git imap-send`, I used the > examples in the manual page as the main reference for my configuration: > > ``` > [imap] > folder = "[Gmail]/Drafts" > host = imaps://imap.gmail.com > user = doron.be...@gmail.com > port = 993 > sslverify = false > ``` > > This is my `cat patch.out | git imap-send` output: > > ``` > Password for 'imaps://doron.be...@gmail.com@imap.gmail.com': > sending 3 messages > curl_easy_perform() failed: URL using bad/illegal format or missing URL > ``` Thanks for reporting this. I suspect this is related to v2.15.0-rc0~63^2 (imap-send: use curl by default when possible, 2017-09-14) --- e.g. perhaps our custom IMAP code was doing some escaping on the username that libcurl does not do. "man git imap-send" says this is a recommended configuration, so I don't think it's a configuration error. What platform are you on? What version of libcurl are you using? In libcurl::lib/easy.c I am also seeing if(mcode) return CURLE_URL_MALFORMAT; /* TODO: return a proper error! */ which looks suspicious. Nicolas, am I on the right track? Thanks, Jonathan > The URI doesn't seem OK to me, I tried using `imap.user = doron.behar` and the > URI was `imaps://doron.be...@imap.gmail.com` but that ended up with the same > error as in the previous case. > > I would love to get some help here, a Google Search didn't help as well. > > Thanks.
Re: [PATCH] Makefile: replace perl/Makefile.PL with simple make rules
Hi, Ævar Arnfjörð Bjarmason wrote: > Replace the perl/Makefile.PL and the fallback perl/Makefile used under > NO_PERL_MAKEMAKER=NoThanks with a much simpler implementation heavily > inspired by how the i18n infrastructure's build process works[1]. Yay! This looks exciting. One quick comment: [...] > * We don't build the Git(3) Git::I18N(3) etc. man pages from the >embedded perldoc. I suspect nobody really cares, these are mostly >internal APIs, and if someone's developing against them they likely >know enough to issue a "perldoc" against the installed file to get >the same result. > >But this is a change in how Git is installed now on e.g. CentOS & >Debian which carry these manpages. They could be added (via >pod2man) if anyone really cares. > >I doubt they will. The reason these were built in the first place >was as a side-effect of how ExtUtils::MakeMaker works. Debian cares (see https://www.debian.org/doc/packaging-manuals/perl-policy/ch-module_packages.html for details). I'll try applying this patch and seeing what happens some time this week. Thanks, Jonathan
Att! Att!! Att!!! Att!!!!
Good Day, I'm Wong Shiu a staff of Wing Hang Bank here in Hong Kong. Can i TRUST you in transferring- $13,991,674 USD? If yes do get back to me with my private email: wong.shiu...@accountant.com Best Regards
Re: [PATCH] hashmap: adjust documentation to reflect reality
On Thu, Nov 30, 2017 at 12:51:41AM +0100, Johannes Schindelin wrote: > The hashmap API is just complicated enough that even at least one > long-time Git contributor has to look up how to use it every time he > finds a new use case. When that happens, it is really useful if the > provided example code is correct... > > While at it, "fix a memory leak", avoid statements before variable > declarations, fix a const -> no-const cast, several %l specifiers (which > want to be %ld), avoid using an undefined constant, call scanf() > correctly, use FLEX_ALLOC_STR() where appropriate, and adjust the style > here and there. Heh, that's quite a list of faults for what's supposed to be simple example code. ;) Your improvements all look good to me, and I'd be happy to see this applied as-is. But here are two possible suggestions: > diff --git a/hashmap.h b/hashmap.h > index 7cb29a6aede..7ce79f3f72c 100644 > --- a/hashmap.h > +++ b/hashmap.h > @@ -18,75 +18,71 @@ > * > * #define COMPARE_VALUE 1 > * > - * static int long2string_cmp(const struct long2string *e1, > + * static int long2string_cmp(const void *hashmap_cmp_fn_data, > + *const struct long2string *e1, > *const struct long2string *e2, > - *const void *keydata, const void *userdata) > + *const void *keydata) If these struct pointers became "const void *", then we would not need to cast the function pointer here: > * hashmap_init(&map, (hashmap_cmp_fn) long2string_cmp, &flags, 0); which would mean that the original problem you are fixing would have been caught by the compiler, rather than probably segfaulting at runtime. My second suggestion (which I'm on the fence about) is: would it better to just say "see t/helper/test-hashmap.c for a representative example?" I'm all for code examples in documentation, but this one is quite complex. The code in test-hashmap.c is not much more complex, and is at least guaranteed to compile and run. It doesn't show off how to combine a flex-array with a hashmap as well, but I'm not sure how important that is. So I could go either way. -Peff
How hard would it be to implement sparse fetching/pulling?
Hi guys, I'm looking for ways to improve fetch/pull/clone time for large git (mono)repositories with unrelated source trees (that span across multiple services). I've found sparse checkout approach appealing and helpful for most of client-side operations (e.g. status, reset, commit, etc.) The problem is that there is no feature like sparse fetch/pull in git, this means that ALL objects in unrelated trees are always fetched. It may take a lot of time for large repositories and results in some practical scalability limits for git. This forced some large companies like Facebook and Google to move to Mercurial as they were unable to improve client-side experience with git while Microsoft has developed GVFS, which seems to be a step back to CVCS world. I want to get a feedback (from more experienced git users than I am) on what it would take to implement sparse fetching/pulling. (Downloading only objects related to the sparse-checkout list) Are there any issues with missing hashes? Are there any fundamental problems why it can't be done? Can we get away with only client-side changes or would it require special features on the server side? If we had such a feature then all we would need on top is a separate tool that builds the right "sparse" scope for the workspace based on paths that developer wants to work on. In the world where more and more companies are moving towards large monorepos this improvement would provide a good way of scaling git to meet this demand. PS. Please don't advice to split things up, as there are some good reasons why many companies decide to keep their code in the monorepo, which you can easily find online. So let's keep that part out the scope. -Vitaly
Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL
On Wed, Nov 29, 2017 at 06:04:45PM -0800, Jonathan Nieder wrote: > > Password for 'imaps://doron.be...@gmail.com@imap.gmail.com': > > sending 3 messages > > curl_easy_perform() failed: URL using bad/illegal format or missing URL > > ``` > > Thanks for reporting this. I suspect this is related to > v2.15.0-rc0~63^2 (imap-send: use curl by default when possible, > 2017-09-14) --- e.g. perhaps our custom IMAP code was doing some > escaping on the username that libcurl does not do. > > "man git imap-send" says this is a recommended configuration, so I > don't think it's a configuration error. > > What platform are you on? What version of libcurl are you using? All good thoughts/questions. I have two suggestions to add: 1. As an immediate work-around, running "imap-send --no-curl" may work. That will at least get this case working while we debug. 2. Setting GIT_TRACE_CURL=1 may dump more verbose information. But one caveat: if you get as far as authenticating, then the trace will contain your password. We redact HTTP auth from the trace output, but not imap ones. -Peff
Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL
On Wed, Nov 29, 2017 at 10:28:32PM -0500, Jeff King wrote: > 2. Setting GIT_TRACE_CURL=1 may dump more verbose information. But one > caveat: if you get as far as authenticating, then the trace will > contain your password. We redact HTTP auth from the trace output, > but not imap ones. I tried my hand at a patch, but it ended up a lot more complicated than I would have liked. Thoughts? diff --git a/http.c b/http.c index 713525f38e..fcc9001af6 100644 --- a/http.c +++ b/http.c @@ -560,7 +560,7 @@ static void set_curl_keepalive(CURL *c) } #endif -static void redact_sensitive_header(struct strbuf *header) +static void redact_http_header(struct strbuf *header) { const char *sensitive_header; @@ -577,7 +577,60 @@ static void redact_sensitive_header(struct strbuf *header) } } -static void curl_dump_header(const char *text, unsigned char *ptr, size_t size, int hide_sensitive_header) +static void redact_imap_header(struct strbuf *header) +{ + const char *p; + + /* skip past the command tag */ + p = strchr(header->buf, ' '); + if (!p) + return; /* no tag */ + p++; + + if (skip_prefix(p, "AUTHENTICATE ", &p)) { + /* the first token is the auth type, which is OK to log */ + while (*p && !isspace(*p)) + p++; + /* the rest is an opaque blob; fall through to redact */ + } else if (skip_prefix(p, "LOGIN ", &p)) { + /* fall through to redact both login and password */ + } else { + /* not a sensitive header */ + return; + } + + strbuf_setlen(header, p - header->buf); + strbuf_addstr(header, " "); +} + +static void redact_sensitive_header(CURL *handle, struct strbuf *header) +{ + const char *url; + int ret; + + ret = curl_easy_getinfo(handle, CURLINFO_EFFECTIVE_URL, &url); + if (!ret && url) { + if (starts_with(url, "http")) { + redact_http_header(header); + return; + } + if (starts_with(url, "imap")) { + redact_imap_header(header); + return; + } + } + + /* +* We weren't able to figure out the protocol. Err on the side of +* redacting too much. +*/ + redact_http_header(header); + redact_imap_header(header); +} + +static void curl_dump_header(CURL *handle, const char *text, +unsigned char *ptr, size_t size, +int hide_sensitive_header) { struct strbuf out = STRBUF_INIT; struct strbuf **headers, **header; @@ -591,7 +644,7 @@ static void curl_dump_header(const char *text, unsigned char *ptr, size_t size, for (header = headers; *header; header++) { if (hide_sensitive_header) - redact_sensitive_header(*header); + redact_sensitive_header(handle, *header); strbuf_insert((*header), 0, text, strlen(text)); strbuf_insert((*header), strlen(text), ": ", 2); strbuf_rtrim((*header)); @@ -641,7 +694,7 @@ static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, break; case CURLINFO_HEADER_OUT: text = "=> Send header"; - curl_dump_header(text, (unsigned char *)data, size, DO_FILTER); + curl_dump_header(handle, text, (unsigned char *)data, size, DO_FILTER); break; case CURLINFO_DATA_OUT: text = "=> Send data"; @@ -653,7 +706,7 @@ static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, break; case CURLINFO_HEADER_IN: text = "<= Recv header"; - curl_dump_header(text, (unsigned char *)data, size, NO_FILTER); + curl_dump_header(handle, text, (unsigned char *)data, size, NO_FILTER); break; case CURLINFO_DATA_IN: text = "<= Recv data";
Re: [PATCH] git-prompt: fix reading files with windows line endings
Hi Johannes, On 30 Nov 2017 01:21, Johannes Schindelin wrote: > On Wed, 29 Nov 2017, Robert Abel wrote: >> This means that it should be okay to just do >> >>> test -r "$f" && IFS=" \t\r\n" read "$@" < "$f" > > I am afraid that this won't work: when I call I managed to trick myself with that one, yes... Apparently I had already converted my HEAD back to Unix line endings. However, I noticed that the behavior of read is apparently ambiguous for the last (or a single) variable: >From POSIX.1-2008: > If there are fewer vars than fields, the last var shall be set to a > value comprising the following elements: > - The field that corresponds to the last var in the normal assignment > sequence described above > - The delimiter(s) that follow the field corresponding to the last var > - The remaining fields and their delimiters, with trailing IFS white > space ignored I read that last "ignored" as "trailing IFS white space shall not be appended". Apparently, people implementing read read it as "trailing IFS while space shall not be processed further" Thus, the behavior for trailing IFS white space is different in case of one or two variables: printf '123 456\r\n' | while IFS=$' \t\r\n' read foo bar do printf 'foo: %s' "$foo" | hexdump -C printf 'bar: %s' "$bar" | hexdump -C done This works as expected trimming the trailing \r: 66 6f 6f 3a 20 31 32 33 |foo: 123| 0008 62 61 72 3a 20 34 35 36 |bar: 456| 0008 While doing the same just reading a single variable printf '123 456\r\n' | while IFS=$' \t\r\n' read foo do printf 'foo: %s' "$foo" | hexdump -C printf 'bar: %s' "$bar" | hexdump -C done prints 66 6f 6f 3a 20 31 32 33 20 34 35 36 0d |foo: 123 456.| 000d 62 61 72 3a 20|bar: | 0005 Notice the 0d at the end of foo, which didn't get trimmed. So reading a dummy variable along with the actual content variable works for git-prompt: __git_eread () { local f="$1" local dummy shift test -r "$f" && IFS=$'\r\n' read "$@" dummy < "$f" } I feel like this would be the most readable solution thus far. Regards, Robert
Git stash (or git rev-parse) doesn't inherit the '--work-tree' parameter
Hello guys, I don't know if this is a normal behavior, so I'll just explain it here. * Behavior: from a directory OUTSIDE my git repo, I can run this command successfully: $ git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path pull origin master But this one: $ git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path stash Will fails with : 'fatal: $program_name cannot be used without a working tree.' (Yes, the error message has been corrected since, but I use debian, so I'm at least 6 month late). * A bit of investigation: I don't know many about git internal functioning, but with a bit of googleing, I would point out git-sh-setup.sh:204, in function require_work_tree, called by git-stash. I could test (from outside my git repo, again): git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path rev-parse --is-inside-work-tree return: false What surprise me is that 'git-dir' seems to be passed ok to rev-parse: git --git-dir=/my/repo/path/.git --work-tree=/my/repo/path rev-parse --git-dir returns: /my/repo/path/.git * Question Is this a normal behavior? Stash can only be applied in current working directory? If yes, I'll have to change my script to (cd && git command) If not, should I push a bug report somewhere? Marc