Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
On Thu, Dec 13, 2018 at 3:48 PM Stefan Beller wrote: > > > > The current situation is definitely a problem. If I am in a worktree, > > > using "head" should be the same as "HEAD". > > By any chance, is your file system case insensitive? > That is usually the source of confusion for these discussions. This behavior is the same for MacOS (High Sierra) and Windows 7. I assume other derivatives of those act the same. On CentOS "head" is an ambiguous ref. If Windows and Mac resulted in an ambiguous ref, that would also be OK, but as it is now, they return the result of "HEAD" on the primary worktree. > > Maybe in worktree code we have a spillover between path > resolution and ref namespace?
Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
On Thu, Dec 13, 2018 at 3:43 PM Duy Nguyen wrote:
>
> On Thu, Dec 13, 2018 at 9:34 PM Mike Rappazzo wrote:
> >
> > On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen wrote:
> > >
> > > On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> > > wrote:
> > > >
> > > > From: Michael Rappazzo
> > > >
> > > > On a worktree which is not the primary, using the symbolic-ref 'head'
> > > > was
> > > > incorrectly pointing to the main worktree's HEAD. The same was true for
> > > > any other case of the word 'Head'.
> > > >
> > > > Signed-off-by: Michael Rappazzo
> > > > ---
> > > > refs.c | 8
> > > > t/t1415-worktree-refs.sh | 9 +
> > > > 2 files changed, 13 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/refs.c b/refs.c
> > > > index f9936355cd..963e786458 100644
> > > > --- a/refs.c
> > > > +++ b/refs.c
> > > > @@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct
> > > > object_id *oid, char **ref)
> > > > *ref = xstrdup(r);
> > > > if (!warn_ambiguous_refs)
> > > > break;
> > > > - } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf,
> > > > "HEAD")) {
> > > > + } else if ((flag & REF_ISSYMREF) &&
> > > > strcasecmp(fullref.buf, "HEAD")) {
> > >
> > > This is not going to work. How about ~40 other "strcmp.*HEAD"
> > > instances? All refs are case-sensitive and this probably will not
> > > change even when we introduce new ref backends.
> >
> > The current situation is definitely a problem. If I am in a worktree,
> > using "head" should be the same as "HEAD".
>
> No "head" is not the same as "HEAD". It does not matter if you're in a
> worktree or not.
I was not aware of a difference. Is that spelled out in the docs
somewhere? It seems like a bad idea to have a magical symbolic ref
that _sometimes_ gives you a different answer depending on casing.
What should "head" do in a worktree? Is it supposed to mean the HEAD
of the primary worktree?
>
> > I am not sure if you mean that the fix is too narrow or too wide.
> > Maybe it is only necessary in 'is_per_worktree_ref'. On the other
> > side of the coin, I could change every strcmp to strcasecmp where the
> > comparison is against "HEAD".
>
> If you make "head" work like "HEAD", then it should work for _all_
> commands, not just worktree, and "MASTER" should match
> "refs/heads/master" and so on. I don't think it's as simple as
> changing strcmp to strcasecmp. You would need to make ref management
> case-insensitive (and make sure if still is case-sensitive if
> configured so). I don't think anybody has managed that.
I am all for making "head" work in all cases, not just worktree. I
don't think that this situation applies to non-magical refs
(branches/tags).
> --
> Duy
Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen wrote:
>
> On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> wrote:
> >
> > From: Michael Rappazzo
> >
> > On a worktree which is not the primary, using the symbolic-ref 'head' was
> > incorrectly pointing to the main worktree's HEAD. The same was true for
> > any other case of the word 'Head'.
> >
> > Signed-off-by: Michael Rappazzo
> > ---
> > refs.c | 8
> > t/t1415-worktree-refs.sh | 9 +
> > 2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/refs.c b/refs.c
> > index f9936355cd..963e786458 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct
> > object_id *oid, char **ref)
> > *ref = xstrdup(r);
> > if (!warn_ambiguous_refs)
> > break;
> > - } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf,
> > "HEAD")) {
> > + } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf,
> > "HEAD")) {
>
> This is not going to work. How about ~40 other "strcmp.*HEAD"
> instances? All refs are case-sensitive and this probably will not
> change even when we introduce new ref backends.
The current situation is definitely a problem. If I am in a worktree,
using "head" should be the same as "HEAD".
I am not sure if you mean that the fix is too narrow or too wide.
Maybe it is only necessary in 'is_per_worktree_ref'. On the other
side of the coin, I could change every strcmp to strcasecmp where the
comparison is against "HEAD".
>
> > warning(_("ignoring dangling symref %s"),
> > fullref.buf);
> > } else if ((flag & REF_ISBROKEN) && strchr(fullref.buf,
> > '/')) {
> > warning(_("ignoring broken ref %s"), fullref.buf);
> --
> Duy
Re: commit -> public-inbox link helper
On Wed, Apr 4, 2018 at 12:35 PM, Johannes Schindelin
wrote:
> Hi team,
>
> I found myself in dear need to quickly look up mails in the public-inbox
> mail archive corresponding to any given commit in git.git. Some time ago,
> I wrote a shell script to help me with that, and I found myself using it a
> couple of times, so I think it might be useful for others, too.
>
> This script (I call it lookup-commit.sh) needs to be dropped into a *bare*
> clone of http://public-inbox.org/git, and called with its absolute or
> relative path from a git.git worktree, e.g.
>
> ~/public-inbox-git.git/lookup-commit.sh \
> fea16b47b603e7e4fa7fca198bd49229c0e5da3d
>
> This will take a while initially, or when the `master` branch of the
> public-inbox mirror was updated, as it will generate two files with
> plain-text mappings.
>
> In its default mode, it will print the Message-ID of the mail (if found).
>
> With --open, it opens the mail in a browser (macOS support is missing,
> mainly because I cannot test: just add an `open` alternative to
> `xdg-open`).
>
> With --reply, it puts the mail into the `from-public-inbox` folder of a
> maildir-formatted ~/Mail/, so it is pretty specific to my setup here.
>
> Note: the way mails are matched is by timestamp. In practice, this works
> amazingly often (although not always, I reported findings short after
> GitMerge 2017). My plan was to work on this when/as needed.
>
> Note also: the script is very much in a 'works-for-me' state, and I could
> imagine that others might want to modify it to their needs. I would be
> delighted if somebody would take custody of it (as in: start a repository
> on GitHub, adding a README.md, adding a config setting to point to the
> location of the public-inbox mirror without having to copy the script,
> adding an option to install an alias to run the script, etc).
>
> And now, without further ado, here it is, the script, in its current glory:
>
> -- snipsnap --
> #!/bin/sh
>
> # This is a very simple helper to assist with finding the mail (if any)
> # corresponding to a given commit in git.git.
>
> die () {
> echo "$*" >&2
> exit 1
> }
>
> mode=print
> while case "$1" in
> --open) mode=open;;
> --reply) mode=reply;;
> -*) die "Unknown option: $1";;
> *) break;;
> esac; do shift; done
>
> test $# = 1 ||
> die "Usage: $0 ( [--open] | [--reply] ) "
>
> test reply != $mode ||
> test -d "$HOME/Mail" ||
> die "Need $HOME/Mail to reply"
>
> commit="$1"
> tae="$(git show -s --format='%at %an <%ae>' "$1")" ||
> die "Could not get Timestamp/Author/Email triplet from $1"
>
> # We try to match the timestamp first; the author name and author email are
> # not as reliable: they might have been overridden via a "From:" line in the
> # mail's body
> timestamp=${tae%% *}
>
> cd "$(dirname "$0")" ||
> die "Could not cd to the public-inbox directory"
>
> git rev-parse --quiet --verify \
> b60d038730d2c2bb8ab2b48c117db917ad529cf7 >/dev/null 2>&1 ||
> die "Not a public-inbox directory: $(pwd)"
>
> head="$(git rev-parse --verify master)" ||
> die "Could not determine tip of master"
>
> prevhead=
> test ! -f map.latest-rev ||
> prevhead="$(cat map.latest-rev)"
>
> if test $head != "$prevhead"
> then
> range=${prevhead:+$prevhead..}$head
> echo "Inserting records for $range" >&2
> git log --format="%at %h %an <%ae>" $range >map.txt.add ||
> die "Could not enumerate $range"
>
> cat map.txt map.txt.add 2>/dev/null | sort -n >map.txt.new &&
> mv -f map.txt.new map.txt ||
> die "Could not insert new records"
>
> echo $head >map.latest-rev
> fi
>
> lines="$(grep "^$timestamp " if test 1 != $(echo "$lines" | wc -l)
> then
> test -n "$lines" ||
> die "No records found for timestamp $timestamp"
>
> echo "Multiple records found:"
>
> for h in $(echo "$lines" | cut -d ' ' -f 2)
> do
> git show -s --format="%nOn %ad, %an <%ae> sent" $h
> git show $h |
> sed -n -e 's/^+Message-Id: <\(.*\)>/\1/ip' \
> -e 's/^+Subject: //ip'
> done
>
> exit 1
> fi
>
> # We found exactly one record: print the message ID
> h=${lines#$timestamp }
> h=${h%% *}
> messageid="$(git show $h | sed -n 's/^+Message-Id: <\(.*\)>/\1/ip')" ||
> die "Could not determine Message-Id from $h"
>
> case $mode in
> print) echo $messageid;;
> open)
> url=https://public-inbox.org/git/$messageid
> case "$(uname -s)" in
> Linux) xdg-open "$url";;
> MINGW*|MSYS*) start "$url";;
Darwin*) open "$url";;
> *) die "Need to learn how to open URLs on $(uname -s)";;
> esac
> ;;
> reply)
> mkdir -p "$HOME/Mail/from-public-inbox/new" &&
> mkdir -p "$HOME/Mail/from-public-inbox/cur" &&
> mkdir -p "$HOME/Mail/from-public-inbox/tmp" ||
> die "Could not set up mail folder 'from-public-inbox'"
>
> path=$(
Re: [PATCH] push: add config option to --force-with-lease by default.
On Wed, Jul 5, 2017 at 12:43 PM, Francesco Mazzoli wrote: >> On 5 Jul 2017, at 17:17, Junio C Hamano wrote: >> >> The take-away lesson that the earlier thread gave me was that the >> order in which the three options are ranked by their desirebility >> in the UI (and the order we would like to encourage users to use) >> is, from the most to the least preferrable: >> >> - "--force-with-lease=:" that is safer than "--force"; >> >> - "--force" that is known to be dangerous, and does not pretend to >> be anything but; >> >> - "--force-with-lease" that pretends to be safer but is not. >> >> The last form should eventually be eliminated, as there is no way to >> correctly intuit what the expected object should be. > > What's not clear to me is what the intended workflow using > `--force-with-lease=:` is. Intuitively it seems extremely > cumbersome to manually pluck a revision each time, especially when > dealing with commits that all have the same description. > > On the other hand for my workflow `--force-with-lease` works quite well > because I tend to use it in cases where me and a colleague are working > on the same PR, and thus I'm not doing anything else (including fetching). > > Moreover, it seems to me that the problem `--force-with-lease` is > just one of marketing. `--force-with-lease` is strictly more "safe" > than `--force` in the sense that it'll reject some pushes that `--force` > will let through. I think that if we advertise it better including its > drawbacks it can still be better than no checks at all. > > Francesco I am in your camp on this, and I will also only ever explicitly fetch. I would hate for --force-with-lease to disappear. However, I believe that the problem is that there are many third party tools which do a fetch behind the scenes (for example Atlassian SourceTree). This can update the local refs without a user necessarily thinking about it. This can lead to a force-with-lease being used unsafely (without the stated lease). _Mike
Re: [PATCH v2] rebase -i: add config to abbreviate command-names
On Tue, Apr 25, 2017 at 5:57 AM, Andreas Schwab wrote: > On Apr 25 2017, Liam Beguin wrote: > >> Add the 'rebase.abbrevCmd' boolean config option to allow `git rebase -i` >> to abbreviate the command-names in the instruction list. >> >> This means that `git rebase -i` would print: >> p deadbee The oneline of this commit >> ... >> >> instead of: >> pick deadbee The oneline of this commit >> ... >> >> Using a single character command-name allows the lines to remain >> aligned, making the whole set more readable. > > Perhaps there should rather be an option to tell rebase to align the > columns? > You _can_ set a custom instruction format using the config variable: `rebase.instructionFormat`. With this, you can align columns using the normal git log format. For example, I personally use this as my instruction format: [%an%<|(64)%x5d %s While, this won't always align perfectly, it may help scratch your itch.
Re: [ANNOUNCE] Git for Windows 2.12.2
Forwarding to the lists, as my original message was rejected for html.
On Thu, Mar 30, 2017 at 3:44 PM, Andrew Witte wrote:
> Just updated back to git 2.12.2 and git-lfs 2.0.2 and everything worked
> fine. Wish I could have gotten more info when it happened as its happened on
> a different computer as well. Will keep an eye out.
>
> Also another note that I really don't like with Windows for Git since 2.12
> is that It packages git-lfs with it. When I use the cmd it overrides the
> other git-lfs install I have. I have to manually go and remove the old
> git-lfs file in "program files" for things to work correctly.
>
> On top of this git-lfs needs to be registered in the environment vars
> because this is what the main git-lfs install does and apps Iv'e made like
> Git-It-GUI (https://github.com/reignstudios/Git-It-GUI) invoke git-lfs
> directly for some stuff. Because of this issue, the app will think a newer
> version is installed thats different from what the normal git cmd reports.
> Also doing git clone outside of the windows cmd with only git for windows
> installed doesn't invoke git-lfs correctly as its not registered in the
> system environment vars. In short I don't think it should be shipped with
> the installer as it just creates confusion.
>
> On Thu, Mar 30, 2017 at 8:41 AM, Michael Rappazzo
> wrote:
>>
>> I suspect that this is a problem in the windows credential manager. I
>> tried this on:
>> - git 2.12.2.windows.1 => failure
>> - git 2.12.1.windows.1 => success
>>
>> More Details:
>> I have a perl script which uses (a copy of Git.pm) to invoke the
>> credential manager. While debugging that script, I dumped the hash that I
>> read from the credential manager:
>>
>> $git->credential($cred, 'fill');
>> print Data::Dumper->Dump( [ $cred ] , [ "cred" ] );
>>
>> In 2.12.2, this produces output like this:
>>
>> $cred = {
>> 'path' => '',
>> 'protocol' => 'https',
>> 'username' => '',
>> 'host' => 'some.host.com',
>> 'password' => ''
>> };
>>
>> In 2.12.1, this produces output like this:
>>
>> $cred = {
>> 'path' => '',
>> 'host' => 'some.host.com',
>> 'protocol' => 'https',
>> 'password' => 'my.password',
>> 'username' => 'mrappazzo'
>> };
>>
>> While debugging this, I did something to get it to work on 2.12.2. After
>> downgrading to 2.12.1, I manually removed the credentials from Credential
>> Manager (in Control Panel). After successful authentication, they were back
>> in the credential manager. I then upgraded to 2.12.2, and I was able to
>> successfully authenticate.
>>
>> To try to recreate the problem scenario again (in 2.12.2), I cleared the
>> credentials in Credential Manager. Reattempting to authenticate gave the
>> credentials prompt. The output of the perl hash was missing the password
>> again (thus, reproducing the error condition).
>>
>> I hope this helps.
>> _Mike
>>
>>
>> On Wednesday, March 29, 2017 at 10:06:03 PM UTC-4, Andrew Witte wrote:
>>>
>>> I'll try to get more info tomorrow.
>>>
>>>
>>> On Wednesday, March 29, 2017 at 2:59:10 PM UTC-7, Johannes Schindelin
>>> wrote:
Hi Andrew,
On Wed, 29 Mar 2017, Andrew Witte wrote:
> The git 2.12 GCM for Windows is broken. I tried doing a git clone and
> got "*remote: HTTP Basic: Access denied*".
> I downgraded to git 2.11.0 and everything worked fine.
Could you test v2.12.1, too, and open a bug report at:
https://github.com/git-for-windows/git/issues/new ?
I am particularly interested in any details you can share that would
help
other developers like me to reproduce the issue.
Thank you,
Johannes
>
>
Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
On Thu, Feb 9, 2017 at 5:54 PM, Junio C Hamano wrote: > > That leaves what the right single-step behaviour change should be. > As I recall Duy said something about --common-dir and other things > Mike's earlier change also covered, I'd prefer to leave it to three > of you to figure out what the final patch should be. > The tests which I covered in my previous patch [1] addressed the places where we identified similar problems. We should try to include some form of those tests. As far as implementation goes in rev-parse, the version in this thread is probably better that what I had, but it would need to also be applied to --git-common-dir and --shared-index-path. [1] http://public-inbox.org/git/1464261556-89722-2-git-send-email-rappa...@gmail.com/
Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
On Thu, Feb 9, 2017 at 4:48 AM, Duy Nguyen wrote:
> On Wed, Feb 8, 2017 at 7:17 PM, Johannes Schindelin
> wrote:
>> In addition to making git_path() aware of certain file names that need
>> to be handled differently e.g. when running in worktrees, the commit
>> 557bd833bb (git_path(): be aware of file relocation in $GIT_DIR,
>> 2014-11-30) also snuck in a new option for `git rev-parse`:
>> `--git-path`.
>>
>> On the face of it, there is no obvious bug in that commit's diff: it
>> faithfully calls git_path() on the argument and prints it out, i.e. `git
>> rev-parse --git-path ` has the same precise behavior as
>> calling `git_path("")` in C.
>>
>> The problem lies deeper, much deeper. In hindsight (which is always
>> unfair), implementing the .git/ directory discovery in
>> `setup_git_directory()` by changing the working directory may have
>> allowed us to avoid passing around a struct that contains information
>> about the current repository, but it bought us many, many problems.
>
> Relevant thread in the past [1] which fixes both --git-path and
> --git-common-dir. I think the author dropped it somehow (or forgot
> about it, I know I did). Sorry can't comment on that thread, or this
> patch, yet.
I didn't exactly forget it (I have it sitting in a branch), I wasn't
sure what else was needed (from a v5 I guess), so it has stagnated.
There was also another patch [1] at the time done by SZEDER Gábor
trying to speed up the completion scripts by adding `git rev-parse
--absolute-git-dir` option to deal with this case as well.
>
> [1]
> http://public-inbox.org/git/1464261556-89722-1-git-send-email-rappa...@gmail.com/
> --
> Duy
[1] http://public-inbox.org/git/20170203024829.8071-16-szeder@gmail.com/
Re: merge --no-ff is NOT mentioned in help
(Please reply inline)
On Wed, Nov 16, 2016 at 10:48 AM, Vanderhoof, Tzadik
wrote:
> I am running:git version 2.10.1.windows.1
>
> I typed: git merge -h
>
> and got:
>
> usage: git merge [] [...]
>or: git merge [] HEAD
>or: git merge --abort
>
> -ndo not show a diffstat at the end of the merge
> --statshow a diffstat at the end of the merge
> --summary (synonym to --stat)
> --log[=] add (at most ) entries from shortlog to merge
> commit message
> --squash create a single commit instead of doing a merge
> --commit perform a commit if the merge succeeds (default)
> -e, --editedit message before committing
> --ff allow fast-forward (default)
> --ff-only abort if fast-forward is not possible
> --rerere-autoupdate update the index with reused conflict resolution if
> possible
> --verify-signatures verify that the named commit has a valid GPG
> signature
> -s, --strategy
> merge strategy to use
> -X, --strategy-option
> option for selected merge strategy
> -m, --message
> merge commit message (for a non-fast-forward merge)
> -v, --verbose be more verbose
> -q, --quiet be more quiet
> --abort abort the current in-progress merge
> --allow-unrelated-histories
> allow merging unrelated histories
> --progressforce progress reporting
> -S, --gpg-sign[=]
> GPG sign commit
> --overwrite-ignoreupdate ignored files (default)
>
> Notice there is NO mention of the "--no-ff" option
I understand. On my system I can reproduce this by providing a bad
argument to `git merge`. This is the output from the arg setup. For
"boolean" arguments (like '--ff'), there is an automatic counter
argument with "no-" in there ('--no-ff') to disable the option. Maybe
it would make sense to word the output to include both.
>
> -Original Message-
> From: Mike Rappazzo [mailto:rappa...@gmail.com]
> Sent: Wednesday, November 16, 2016 7:37 AM
> To: Vanderhoof, Tzadik
> Cc: git@vger.kernel.org
> Subject: Re: merge --no-ff is NOT mentioned in help
>
> On Wed, Nov 16, 2016 at 10:16 AM, Vanderhoof, Tzadik
> wrote:
>> When I do: "git merge -h" to get help, the option "--no-ff" is left out of
>> the list of options.
>
> I am running git version 2.10.0, and running git merge --help contains these
> lines:
>
>--ff
>When the merge resolves as a fast-forward, only update the branch
> pointer, without creating a merge commit. This is the default behavior.
>
>--no-ff
>Create a merge commit even when the merge resolves as a
> fast-forward. This is the default behaviour when merging an annotated (and
> possibly signed) tag.
>
>--ff-only
>Refuse to merge and exit with a non-zero status unless the current
> HEAD is already up-to-date or the merge can be resolved as a fast-forward.
>
>
>
> This e-mail, including attachments, may include confidential and/or
> proprietary information, and may be used only by the person or entity
> to which it is addressed. If the reader of this e-mail is not the intended
> recipient or his or her authorized agent, the reader is hereby notified
> that any dissemination, distribution or copying of this e-mail is
> prohibited. If you have received this e-mail in error, please notify the
> sender by replying to this message and delete this e-mail immediately.
Re: merge --no-ff is NOT mentioned in help
On Wed, Nov 16, 2016 at 10:16 AM, Vanderhoof, Tzadik wrote: > When I do: "git merge -h" to get help, the option "--no-ff" is left out of > the list of options. I am running git version 2.10.0, and running git merge --help contains these lines: --ff When the merge resolves as a fast-forward, only update the branch pointer, without creating a merge commit. This is the default behavior. --no-ff Create a merge commit even when the merge resolves as a fast-forward. This is the default behaviour when merging an annotated (and possibly signed) tag. --ff-only Refuse to merge and exit with a non-zero status unless the current HEAD is already up-to-date or the merge can be resolved as a fast-forward. > > This e-mail, including attachments, may include confidential and/or > proprietary information, and may be used only by the person or entity > to which it is addressed. If the reader of this e-mail is not the intended > recipient or his or her authorized agent, the reader is hereby notified > that any dissemination, distribution or copying of this e-mail is > prohibited. If you have received this e-mail in error, please notify the > sender by replying to this message and delete this e-mail immediately. >
Re: git diff
On Wed, Oct 12, 2016 at 6:50 AM, wrote: > Hi. > > I created a new branch named hotfix from master. > I switched to the branch, changed 1 file. > > Now I want to see the diff from the both using > > git diff hotfix master > > I do not see any output (difference). > When I do a git status I see my file with status mofified, not staged for > commit. Since you just created the branch, and did not add any content, there is no difference to see. A branch is just a pointer to a commit. You now have two pointers pointing at the same commit. If you want to see the difference between your changes and the master branch, you can omit the first reference: git diff master When you start adding commits to your hotfix branch, you will be able to see the diff between that and master with the command that you gave. However, your arguments may be in the reverse order than what you expect. You want to specify master first because that is the mainline branch (I presume). When you have several commits on your hotfix branch, you can refer to older commits to diff against. There are several ways to refer back, but the simplest is to use a tilde '~' followed by a number to count back. For example 'hotfix~1' refers to the parent commit on the hotfix branch. There is a lot in the documentation[1], so take a look there for more info. Good luck. _Mike [1] https://git-scm.com/doc > Also, I can see that I am working with the correct branch, hotfix > > What am I doing wrong? > > -fuz On Wed, Oct 12, 2016 at 6:50 AM, wrote: > Hi. > > I created a new branch named hotfix from master. > I switched to the branch, changed 1 file. > > Now I want to see the diff from the both using > > git diff hotfix master > > I do not see any output (difference). > When I do a git status I see my file with status mofified, not staged for > commit. > Also, I can see that I am working with the correct branch, hotfix > > What am I doing wrong? > > -fuz
Re: [PATCH] make rebase respect core.hooksPath if set
On Sun, Aug 14, 2016 at 12:29 PM, ryenus wrote: > Patch attached. > > It seems gmail ruined the white spaces. > Not sure how to stop gmail from doing that. > Sorry for me, and for Gmail. Did you use git-send-email? I don't think that the gmail ui works. If you have 2-factor authentication, there are instructions on how to set that up in the docs in Documentation/git-format-patch.txt -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] gitk
On Wed, Jun 8, 2016 at 8:31 AM, Eric Frederich wrote:
> Thanks for confirming. I do a similar workaround too.
> The issue is when new Git users don't even have a ~/.config/git/gitk to
> modify.
> They first have to run it natively where lime exists, then they can
> edit it and use it over VNC.
>
> I cannot find any instructions for submitting patches to the gitk subproject.
> Does anyone know the process for this?
>
> After looking at the original commit which caused this
> (66db14c94c95f911f55575c7fdf74c026443d482)...
> ... reverting may not be the right thing to do.
> Instead, lime should be replaced with "#32cd32", a trivial fix.
>
> Again, I'd do this myself if I had instructions for submitting patches
> for to gitk.
You should be able to submit patches for gitk right here. There is
information right in
Documentation/SubmittingPatches [1]. The primary difference is that
you should clone
gitk and build your patch from that. You would git format-patch and
git send-email in
the same way.
[1] https://github.com/git/git/blob/master/Documentation/SubmittingPatches#L319
> Thanks,
> ~Eric
>
> On Wed, Jun 8, 2016 at 5:58 AM, wrote:
>> Am 08.06.2016 um 11:40 schrieb stefan.na...@atlas-elektronik.com:
>>> Am 07.06.2016 um 21:20 schrieb Eric Frederich:
Hello,
I couldn’t find any documentation on submitting patches for gitk.
I saw in Documentation/SubmittingPatches that gitk is maintained in
its own repo.
I can’t clone repo’s unless they’re http while on my corporate proxy.
I’m hoping someone can help me out or just do it for me ;-)
I’d like to revert 66db14c94c95f911f55575c7fdf74c026443d482.
That commit just renamed “green” to “lime”
It causes gitk to not start up on when ran through VNC.
It works fine on that same system natively or over X11 forwarding but not
VNC.
>>>
>>> FWIW, I can confirm that.
>>>
>>> git version 2.8.3
>>>
>>> My $HOME/.config/git/gitk contains:
>>>
>>> set mergecolors {red blue green purple brown "#009090" magenta "#808000"
>>> "#009000" "#ff0080" cyan "#b07070" "#70b0f0" "#70f0b0" "#f0b070" "#ff70b0"}
>>>
>>> With that file gitk runs without problems.
>>> If I move that file away, gitk stops working over VNC and also forwarded
>>> X11 for me.
>>
>> More Info:
>>
>> The forwarded X11 (which is from a Windows machine running Exceed to a Linux
>> machine) works
>> without the gitk file mentioned above, if I edit the 'rgb.txt' used by
>> Exceed to contain something like:
>>
>> 50 205 50 lime
>>
>> Before the editing the file only contained the following:
>>
>> 50 205 50 lime green
>> 50 205 50 LimeGreen
>>
>> I couldn't do the same for the VNC connection though (Xvnc seems to use a
>> hardcoded 'rgb.txt' file).
>>
>> It seems that using 'lime' was not the best choice...
>>
>>
>> HTH
>>
>> Stefan
>> --
>>
>> /dev/random says: I used to be schizophrenic, but we're all right now.
>> python -c "print
>> '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
>> GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF
>>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: add instructions to help setup gmail 2FA
On Fri, May 27, 2016 at 5:06 PM, Junio C Hamano wrote: > Michael Rappazzo writes: > >> For those who use two-factor authentication with gmail, git-send-email >> will not work unless it is setup with an app-specific password. The >> example for setting up git-send-email for use with gmail will now >> include information on generating and storing the app-specific password. >> --- > > Sounds good. We'd need your sign-off in order to be able to use > this patch, though. > Signed-off-by: Michael Rappazzo >> Documentation/git-send-email.txt | 13 + >> 1 file changed, 13 insertions(+) >> >> diff --git a/Documentation/git-send-email.txt >> b/Documentation/git-send-email.txt >> index 771a7b5..edbba3a 100644 >> --- a/Documentation/git-send-email.txt >> +++ b/Documentation/git-send-email.txt >> @@ -450,6 +450,19 @@ edit ~/.gitconfig to specify your account settings: >> smtpUser = yourn...@gmail.com >> smtpServerPort = 587 >> >> +If you have multifactor authentication setup on your gmail acocunt, you will >> +need to generate an app-specific password for use with 'git send-email'. >> Visit >> +https://security.google.com/settings/security/apppasswords to setup an >> +app-specific password. Once setup, you can store it with the credentials >> +helper: >> + >> + $ git credential fill >> + protocol=smtp >> + host=smtp.gmail.com >> + username=youn...@gmail.com >> + password=app-password >> + >> + >> Once your commits are ready to be sent to the mailing list, run the >> following commands: -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree
On Thu, May 19, 2016 at 3:49 AM, Mike Hommey wrote: > On Fri, Apr 08, 2016 at 08:35:51AM -0400, Mike Rappazzo wrote: >> On Fri, Apr 8, 2016 at 7:47 AM, Duy Nguyen wrote: >> > On Mon, Apr 4, 2016 at 8:42 AM, Michael Rappazzo >> > wrote: >> >> Executing `git-rev-parse --git-common-dir` from the root of the main >> >> worktree results in '.git', which is the relative path to the git dir. >> >> When executed from a subpath of the main tree it returned somthing like: >> >> 'sub/path/.git'. Change this to return the proper relative path to the >> >> git directory (similar to `--show-cdup`). >> > >> > I faced a similar problem just a couple days ago, I expected "git >> > rev-parse --git-path" to return a path relative to cwd too, but it >> > returned relative to git dir. The same solution (or Eric's, which is >> > cleaner in my opinion) applies. --shared-index-path also does >> > puts(git_path(... and has the same problem. Do you want to fix them >> > too? >> >> Sure, I can do that. I will try to get it up soon. > > If I'm not mistaken, it looks like this fell off your radar. (I haven't > seen an updated patch, and it doesn't look like the fix made it to any > git branch). Would you mind updating? > > Cheers, > > Mike There is a newer version submitted on May 6th[1]. Eric Sunshine has submitted a patch [2] which fixes up t1500. It looks like that is in a stable form, so I will rebase my v2 onto those changes, and resubmit in the near future. [1] http://thread.gmane.org/gmane.comp.version-control.git/293778 [2] http://thread.gmane.org/gmane.comp.version-control.git/294999 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] rev-parse tests: add tests executed from a subdirectory
On Fri, May 6, 2016 at 6:10 PM, Junio C Hamano wrote:
> Michael Rappazzo writes:
>
>> t1500-rev-parse contains envrionment leaks (changing dir without
>> changing back, setting config variables, etc). Add a test to clean this
>> up up so that future tests can be added without worry of any setting
>> from a previous test.
>
> This is a wonderful thing to do, but...
>
>> test_rev_parse toplevel false false true '' .git
>>
>> @@ -84,4 +85,41 @@ test_rev_parse 'GIT_DIR=../repo.git, core.bare = true'
>> true false false ''
>> git config --unset core.bare
>> test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true
>> ''
>>
>> +test_expect_success 'cleanup from the previous tests' '
>> + cd .. &&
>> + rm -r work &&
>
> Instead of cleaning things up like this, could you please please
> please fix these existing tests that chdir around without being in a
> subshell? If the "previous tests" failed before going down as this
> step expects, the "cd .. && rm -r" can make things worse.
I still have fixing this test up on my do-to list. My previous
attempt[1] had some flaws
in addition to some objection to the approach I took to expand each test. Eric
Sunshine suggested using a table approach, but I am not sure if that can be done
cleanly.
I figured that a fix to the rev-parse code would supersede test
cleanup, so I separated
my efforts.
I originally copied the pattern from above this code:
> +#cleanup from the above
> +cd ..
> +rm -r work
> +mv repo.git .git || exit 1
but Gábor had an objection to it [2]. So I went with this simple cleanup test.
I could move it back to outside of a test, and do some checks around
it. Something
like:
dir=$(pwd)
target=${dir##*/}
if [ "$target" == "work" ]
then
cd ..
rm -r "work"
fi
>
>> + mv repo.git .git &&
>> + unset GIT_DIR &&
>> + unset GIT_CONFIG &&
>
> The spirit of this change is to make the test more independent from
> the effects of what happened previously. Use sane_unset so that
> we do not have to worry about previous step that may have failed
> before it has a chance to set GIT_DIR and GIT_CONFIG (which would
> cause these unset to fail).
>
>> + git config core.bare $original_core_bare
>
> Is this (rather, the capturing of $original_core_bare up above)
> necessary? We are in the default 'trash' repository when the
> capturing happens, and we know it is not a bare repository, right?
My goal was to have the test be in the state exactly as it was at the
beginning of
the test. Right above my cleanup test this line is executed:
> git config --unset core.bare
I just wanted to be absolutely sure that the value was the same. I
could certainly
simplify it to assume core.bare is "true" though.
Thanks,
_Mike
[1] http://thread.gmane.org/gmane.comp.version-control.git/291729
[2] http://article.gmane.org/gmane.comp.version-control.git/293003
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] rev-parse: fix some options when executed from subpath of main tree
On Fri, Apr 29, 2016 at 9:50 AM, SZEDER Gábor wrote: >> Executing `git-rev-parse` with `--git-common-dir`, `--git-path `, >> or `--shared-index-path` from the root of the main worktree results in >> a relative path to the git dir. >> >> When executed from a subdirectory of the main tree, however, it incorrectly >> returns a path which starts 'sub/path/.git'. > > This is not completely true, because '--git-path ...' returns a > relative path starting with '.git': > > $ git -C t/ rev-parse --git-dir --git-path objects --git-common-dir > /home/szeder/src/git/.git > .git/objects > t/.git > > It's still wrong, of course. I'll try to reword this to make it indicate that the value isn't always incorrect. > >> Change this to return the >> proper relative path to the git directory. > > I think returning absolute paths would be better. It is consistent > with the already properly working '--git-dir' option, which returns an > absolute path in this case. Furthermore, both '--git-path ...' and > '--git-common-dir' already return absolute paths when run from a > subdirectory of the .git directory: > > $ git -C .git/refs rev-parse --git-dir --git-path objects --git-common-dir > /home/szeder/src/git/.git > /home/szeder/src/git/.git/objects > /home/szeder/src/git/.git > I agree with this, but at one point Junio suggested that it should return the relative path[1], so I went with that. Maybe I could RFC a separate patch that changes all of these to absolute. >> Signed-off-by: Michael Rappazzo >> --- >> builtin/rev-parse.c | 19 ++- >> 1 file changed, 14 insertions(+), 5 deletions(-) > > This patch doesn't add any new tests, while subsequent patches of the > series do nothing but add more tests. Splitting up your changes this > way doesn't add any value, it only increases the number of commits. I > think either: > > - all those new tests could be added with this patch, or > > - if you want to add the test separately, then add them before > this patch and mark them with 'test_expect_failure' to clearly > demonstrate what the series is about to fix, and flip them to > 'test_expect_success' in this patch. > > - An alternative way to split this series, following the "Make > separate commits for logically separate changes" guideline, would > be to fix and test these options in separate patches, i.e. fix and > test '--git-path ...' in one patch, then fix and test > '--git-common-dir' in the next, ... > > Thanks, have a re-roll ready to go based on you input. I went with option 2 - add tests with expect failure and fix them with the next. [1] http://article.gmane.org/gmane.comp.version-control.git/290061 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] gitk: changes for the "Tags and heads" view
On Sun, Mar 27, 2016 at 11:06 AM, Michael Rappazzo wrote: > Changes since v2[1]: > - Instead of getting the remote info for each local branch individually, >grab it all at once and store the result > - Instead of a command line option to enable the new sorting option, >enable it with a preference which is stored in the config. > > v1 can be found here[2]. > > [1] http://thread.gmane.org/gmane.comp.version-control.git/289244 > [2] http://thread.gmane.org/gmane.comp.version-control.git/288544 > > Michael Rappazzo (2): > gitk: alter the ordering for the "Tags and heads" view > gitk: add an option to enable sorting the "Tags and heads" view by ref > type > > gitk | 79 > > 1 file changed, 66 insertions(+), 13 deletions(-) > > -- > 2.7.4 > I am still looking for comments on this patch. Also, is there a 'pu' repo for gitk? Currently, I am only tracking git://ozlabs.org/~paulus/gitk -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "gitk --author=foo" shows also parent
On Tue, Apr 26, 2016 at 9:08 AM, Nikolai Kosjar wrote: > Hi! > > $ gitk --author=foo > > ...seems to show also the parent of each author-matched commit, whereas > > $ git log --author=foo > > does not. Is this intended or a bug? I've stumbled over this while > configuring a gitk view with the author field. I believe that this is intentional. Notice that the parent commit's circle is just outlined compared to the selected authored commits are filled. I consider this the context of the commits you are looking at. > > Nikolai > > > > > > # Setup > ~/work/gitkBug % git init . > ~/work/gitkBug % touch file1 file2 > ~/work/gitkBug % git add file1 > ~/work/gitkBug % git commit "--author=MrFoo " file1 -m "add > file1" > ~/work/gitkBug % git add file2 > ~/work/gitkBug % git commit "--author=MrBar " file2 -m "add > file2" > > # TEST: git log --author - OK > ~/work/gitkBug % git log --author=MrBar # OK, as expected > commit 8aa4a4f651162bcb2275a1e9ee23fc1bb7226097 > Author: MrBar > Date: Tue Apr 26 14:22:58 2016 +0200 > > add file2 > > # TEST: gitk --author - OPS > ~/work/gitkBug % gitk --author=MrBar # Ops, gitk shows also the parent > commit > > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/21] rev-parse: add '--absolute-git-dir' option
On Thu, Feb 25, 2016 at 5:54 PM SZEDER Gábor wrote:
>
> Some scripts can benefit from not having to deal with the possibility
> of relative paths to the repository, but the output of 'git rev-parse
> --git-dir' can be a relative path. Case in point: supporting 'git -C
> ' in our Bash completion script turned out to be considerably
> more difficult, error prone and required more subshells and git
> processes when we had to cope with a relative path to the .git
> directory.
>
> Help these use cases and teach 'git rev-parse' a new
> '--absolute-git-dir' option which always outputs a canonicalized
> absolute path to the .git directory, regardless of whether the path is
> discovered automatically or is specified via $GIT_DIR or 'git
> --git-dir='.
>
> Signed-off-by: SZEDER Gábor
> ---
> Documentation/git-rev-parse.txt | 4
> builtin/rev-parse.c | 29 +
> t/t1500-rev-parse.sh| 17 ++---
> 3 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index b6c6326cdc7b..fb06e3118570 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -216,6 +216,10 @@ If `$GIT_DIR` is not defined and the current directory
> is not detected to lie in a Git repository or work tree
> print a message to stderr and exit with nonzero status.
>
> +--absolute-git-dir::
> + Like `--git-dir`, but its output is always the canonicalized
> + absolute path.
> +
> --git-common-dir::
> Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
>
After working a little bit with rev-parse [1], I feel that this might
be better served as a
stand-alone option. Consider that in addition to --git-dir, the
options --git-common-dir,
--git-path, and --git-shared-index produce relative paths.
I propose that it might make more sense to use something like
`--abs-path` to indicate
that the result should include an absolute path (or we could also just spell out
`--absolute-path`). That way we don't have to add additional options
for any other type
that might want an absolute path.
git rev-parse --git-dir --abs-path
git rev-parse --git-common-dir --absolute-path
I do understand that this might be more work than is necessary for the
completion series
here. Would it be unreasonable to suggest a partial implementation
that, for now, only
works with `--git-dir`?
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index cf8487b3b95f..90a4dd6032c0 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -744,17 +744,30 @@ int cmd_rev_parse(int argc, const char **argv, const
> char *prefix)
> putchar('\n');
> continue;
> }
> - if (!strcmp(arg, "--git-dir")) {
> + if (!strcmp(arg, "--git-dir") ||
> + !strcmp(arg, "--absolute-git-dir")) {
> const char *gitdir =
> getenv(GIT_DIR_ENVIRONMENT);
> char *cwd;
> int len;
> - if (gitdir) {
> - puts(gitdir);
> - continue;
> - }
> - if (!prefix) {
> - puts(".git");
> - continue;
> + if (arg[2] == 'g') {/* --git-dir */
> + if (gitdir) {
> + puts(gitdir);
> + continue;
> + }
> + if (!prefix) {
> + puts(".git");
> + continue;
> + }
> + } else {/* --absolute-git-dir
> */
> + if (!gitdir && !prefix)
> + gitdir = ".git";
> + if (gitdir) {
> + char absolute_path[PATH_MAX];
> + if (!realpath(gitdir,
> absolute_path))
> + die_errno(_("unable
> to get absolute path"));
> + puts(absolute_path);
> + continue;
> + }
> }
> cwd = xgetcwd();
> len = strlen(cwd);
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index
Re: Cannot checkout a branch / worktree shows multiple branches for the same directory
On Thu, Apr 14, 2016 at 3:51 PM, Krzysztof Voss wrote: > Hi, > > I stumbled upon an interesting problem when checking out a branch. > I had to switch to a testing branch to merge in some changes from yet > another branch, but when I tried to check out the testing branch I got > a message saying that I'm already on the target branch. > > I used worktree a few times, but the checkouts were always in their > own directories. > It crossed my mind that this behaviour may be related, so I took a > look at the worktree list and noticed that according to that list > there are three branches at the same time in one directory. > > It may be a conicidence and I have no confidence in saying that these > issues are related. > Can someone shed some light on this issue for me? > > > $ git --version > git version 2.7.0.235.g07c314d > > $ git status -uno -sb > ## ticket-22444 > M src/core/parsers/ParserBase.py > M src/core/parsers/test/ParserBase_test.py > > $ git stash > Saved working directory and index state WIP on ticket-22444: > 7c5edaa #22444 refactoring > HEAD is now at 7c5edaa #22444 refactoring > > $ git co testing > fatal: 'testing' is already checked out at '/home/k/workspace/moyo' > > $ pwd > /home/k/workspace/moyo > > $ git branch | grep '*' > * ticket-22444 > > $ git worktree list > /home/k/workspace/moyo 7c5edaa [ticket-22444] > /var/home/k/moyo-lsf 349613d (detached HEAD) > /home/k/workspace/moyo 265b7f9 (detached HEAD) > /home/k/workspace/moyo c852282 [testing] This looks a lot like the `update_linked_gitdir()` bug that (I thought) was fixed[1]. Is it possible that you had this problem since before the bug was fixed and are just noticing it now? If you look in '/home/k/workspace/moyo/.git/worktrees/` I suspect that there are 3 dirs in there, two of which have a file 'gitdir' which have the contents '.git'. These _should_ instead point to the '.git' file in your other work trees. It would be nice to know the last time that those bad worktrees were updated. If you know where the other worktrees are located, then you should be able to manually update this file in each of the worktree dirs. Alternatively, you can manually remove the bad linked worktrees (`rm -r .git/worktrees/bad_wt`). [1] http://thread.gmane.org/gmane.comp.version-control.git/284284 > > $ uname -a > Linux k 3.13.0-83-generic #127-Ubuntu SMP Fri Mar 11 00:25:37 UTC > 2016 x86_64 x86_64 x86_64 GNU/Linux > > $ cat /etc/lsb-release > DISTRIB_ID=Ubuntu > DISTRIB_RELEASE=14.04 > DISTRIB_CODENAME=trusty > DISTRIB_DESCRIPTION="Ubuntu 14.04.4 LTS" > > > Thanks, > Krzysztof > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: Fix how remote branch names with / are drawn
On Wed, Apr 13, 2016 at 2:19 PM, David Holmer wrote:
> I agree that this switches the issue around and that a remote with a
> '/' in the name would be miss colored in the same way a branch with a
> '/' in the name is miss colored now. However, I would guess that
> branches with '/' are MUCH MUCH more common than remotes with '/', so
> like you say "this is a better state than the present". A "complete"
> solution would take iterating through the list of remotes and matching
> the explicit whole pattern (e.g. match
> "remotes/my/remote/with/slashes/" for remote "my/remote/with/slashes")
> but I doubt that is worth it for 99.9% of people.
>
> The alternative regex that you are asking about is either using some
> syntax I am not familiar with or isn't quite correct. I'm most
> familiar with grep command line format, so perhaps tcl regex is
> different.
>
> The original code does the equivalent of this:
>
> ~$ echo "remotes/origin/dev/test1" | grep -o "remotes/.*/"
> remotes/origin/dev/
>
> The issue is that the '.*/' part is greedy in that it will match all
> the way up to and including the last /
>
> My solution was to change the . to [^/] which means "any character but
> /". This stops the match at the first / after the remote name starts:
>
> ~$ echo "remotes/origin/dev/test1" | grep -o "remotes/[^/]*/"
> remotes/origin/
>
> The alternative you suggested with '.*?/' doesn't seem to work with grep:
>
> ~$ echo "remotes/origin/dev/test1" | grep -o "remotes/.*?/"
> (no output, i.e. does not match)
`.*?` is a lazy match. I think it is an extended-regex, and your
version is probably more efficient anyway.
echo "remotes/origin/dev/test1" | grep -Eo "remotes/.*?/"
>
>
> Thank you.
>
(Most people on this list don't like "top posting"), please try to
reply inline instead.
> On Wed, Apr 13, 2016 at 7:35 AM, Mike Rappazzo wrote:
>> On Tue, Apr 12, 2016 at 9:59 PM, David Holmer wrote:
>>> Consider this example branch:
>>>
>>> remotes/origin/master
>>>
>>> gitk displays this branch with different background colors for each part:
>>> "remotes/origin" in orange and "master" in green. The idea is to make it
>>> visually easy to read the branch name separately from the remote name.
>>>
>>> However this fails when given this example branch:
>>>
>>> remotes/origin/foo/bar
>>>
>>> gitk displays this branch with "remotes/origin/foo" in orange and "bar" in
>>> green. This makes it hard to read the branch name "foo/bar". This is due
>>> to an inappropriately greedy regexp. This patch provides a fix so the same
>>> branch will now be displayed with "remotes/origin" in orange and "foo/bar"
>>> in green.
>>>
>>> Signed-off-by: David Holmer
>>> ---
>>> gitk | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gitk b/gitk
>>> index 805a1c7..ca2392b 100755
>>> --- a/gitk
>>> +++ b/gitk
>>> @@ -6640,7 +6640,7 @@ proc drawtags {id x xt y1} {
>>> set xl [expr {$xl - $delta/2}]
>>> $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
>>> -width 1 -outline black -fill $col -tags tag.$id
>>> - if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]}
>>> {
>>> + if {[regexp {^(remotes/[^/]*/|remotes/)} $tag match
>>> remoteprefix]} {
>>> set rwid [font measure mainfont $remoteprefix]
>>> set xi [expr {$x + 1}]
>>> set yti [expr {$yt + 1}]
>>> --
>>
>> This likely fixes the problem for most situations, but doesn't for a
>> remote with a '/' in the name. Yet, I think this is a better state
>> than the present.
>>
>> Is the regex `[^/]*/` more efficient than '.*?/`? Or do you find the
>> former more readable?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t1500-rev-parse: rewrite each test to run in isolation
On Wed, Apr 13, 2016 at 12:54 AM, Eric Sunshine wrote: > On Sat, Apr 9, 2016 at 7:19 AM, Michael Rappazzo wrote: >> t1500-rev-parse has many tests which change directories and leak >> environment variables. This makes it difficult to add new tests without >> minding the environment variables and current directory. >> >> Each test is now setup, executed, and cleaned up without leaving anything >> behind. Tests which have textual expectations have been converted to use >> test_cmp (which will show a diff when the test is run with --verbose). > > It might be easier to review this if broken into several cleanup and > modernization patches, however, some comments below... I felt that the changes are repetitive enough that it did not necessitate separate patches. Perhaps after simplifying based on your suggestions, it will be even smaller. > >> Signed-off-by: Michael Rappazzo >> --- >> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh >> @@ -3,88 +3,571 @@ >> +test_expect_success '.git/: is-bare-repository' ' >> + (cd .git && test false = "$(git rev-parse --is-bare-repository)") >> +' >> >> -# label is-bare is-inside-git is-inside-work prefix git-dir absolute-git-dir >> +test_expect_success '.git/: is-inside-git-dir' ' >> + (cd .git && test true = "$(git rev-parse --is-inside-git-dir)") > > Simpler: > > test true = "$(git -C .git rev-parse --is-inside-git-dir)" > >> +' >> >> -ROOT=$(pwd) >> +test_expect_success '.git/: is-inside-work-tree' ' >> + (cd .git && test false = "$(git rev-parse --is-inside-work-tree)") > > Ditto. > >> +' >> >> -test_rev_parse toplevel false false true '' .git "$ROOT/.git" >> +test_expect_success '.git/: prefix' ' >> + ( >> + cd .git && >> + echo >expected && >> + git rev-parse --show-prefix >actual && >> + test_cmp expected actual >> + ) > > Likewise, you could drop the entire subshell: > > echo >expected && > git -C .git rev-parse --show-prefix >actual && > test_cmp expected actual > >> +' >> >> +test_expect_success '.git/: git-dir' ' >> + ( >> + cd .git && >> + echo . >expected && >> + git rev-parse --git-dir >actual && >> + test_cmp expected actual >> + ) > > Same here and for many subsequent tests (which I won't quote). > >> +' >> +test_expect_success 'core.bare = true: is-bare-repository' ' >> + git config core.bare true && >> + test_when_finished "git config core.bare false" && >> + test true = "$(git rev-parse --is-bare-repository)" > > Simpler: > > test_config core.bare true > > and then you can drop 'test_when_finished' altogether. However, even simpler: > > test true = "$(git -c core.bare=false rev-parse --is-bare-repository)" > > which allows you to drop 'test_config', as well. > > Ditto for subsequent tests (which I won't quote). > >> +' >> +test_expect_success 'core.bare undefined: is-bare-repository' ' >> + git config --unset core.bare && > > test_unconfig core.bare > >> + test_when_finished "git config core.bare false" && > > Why does this need to re-instate core.bare? > > Same comments for subsequent tests. > >> + test false = "$(git rev-parse --is-bare-repository)" >> +' >> +test_expect_success 'GIT_DIR=../.git, core.bare = false: >> is-bare-repository' ' >> + mkdir work && >> + test_when_finished "rm -rf work && git config core.bare false" && >> + ( >> + cd work && >> + export GIT_DIR=../.git && >> + export GIT_CONFIG="$(pwd)"/../.git/config >> + git config core.bare false && >> + test false = "$(git rev-parse --is-bare-repository)" >> + ) >> +' > > Same comments about -C to avoid the subshell and -c for configuration. > > Also, you can do one-shot environment variable setting for the command > invocation, so the subshell goes away, and everything inside the > subshell collapses to: > > test false = "$(GIT_DIR=... GIT_CONFIG=... > git -C work -c core.bare=false rev-parse ...)" > > Additionally, I'm confused about why this test "reverts" the > core.bare=false by setting core.bare=false in 'test_when_finished'. > > Ditto for subsequent tests. > >> +test_expect_success 'GIT_DIR=../repo.git, core.bare = false: >> is-bare-repository' ' >> + mkdir work && >> + cp -r .git repo.git && >> + test_when_finished "rm -r repo.git && rm -rf work && git config >> core.bare false" && > > If 'cp' fails, then 'test_when_finished' will never be invoked, which > means that the cleanup will never happen; thus 'test_when_finished' > needs to be called earlier. Ditto for subsequent tests. > >> + ( >> + cd work && >> + export GIT_DIR=../repo.git && >> + export GIT_CONFIG="$(pwd)"/../repo.git/config >> + git config core.bare false && >> + test false = "$(git rev-par
Re: [PATCH] gitk: Fix how remote branch names with / are drawn
On Tue, Apr 12, 2016 at 9:59 PM, David Holmer wrote:
> Consider this example branch:
>
> remotes/origin/master
>
> gitk displays this branch with different background colors for each part:
> "remotes/origin" in orange and "master" in green. The idea is to make it
> visually easy to read the branch name separately from the remote name.
>
> However this fails when given this example branch:
>
> remotes/origin/foo/bar
>
> gitk displays this branch with "remotes/origin/foo" in orange and "bar" in
> green. This makes it hard to read the branch name "foo/bar". This is due
> to an inappropriately greedy regexp. This patch provides a fix so the same
> branch will now be displayed with "remotes/origin" in orange and "foo/bar"
> in green.
>
> Signed-off-by: David Holmer
> ---
> gitk | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitk b/gitk
> index 805a1c7..ca2392b 100755
> --- a/gitk
> +++ b/gitk
> @@ -6640,7 +6640,7 @@ proc drawtags {id x xt y1} {
> set xl [expr {$xl - $delta/2}]
> $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
> -width 1 -outline black -fill $col -tags tag.$id
> - if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]} {
> + if {[regexp {^(remotes/[^/]*/|remotes/)} $tag match
> remoteprefix]} {
> set rwid [font measure mainfont $remoteprefix]
> set xi [expr {$x + 1}]
> set yti [expr {$yt + 1}]
> --
This likely fixes the problem for most situations, but doesn't for a
remote with a '/' in the name. Yet, I think this is a better state
than the present.
Is the regex `[^/]*/` more efficient than '.*?/`? Or do you find the
former more readable?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse: fix --git-common-dir when executed from subpath of main tree
On Fri, Apr 8, 2016 at 7:47 AM, Duy Nguyen wrote: > On Mon, Apr 4, 2016 at 8:42 AM, Michael Rappazzo wrote: >> Executing `git-rev-parse --git-common-dir` from the root of the main >> worktree results in '.git', which is the relative path to the git dir. >> When executed from a subpath of the main tree it returned somthing like: >> 'sub/path/.git'. Change this to return the proper relative path to the >> git directory (similar to `--show-cdup`). > > I faced a similar problem just a couple days ago, I expected "git > rev-parse --git-path" to return a path relative to cwd too, but it > returned relative to git dir. The same solution (or Eric's, which is > cleaner in my opinion) applies. --shared-index-path also does > puts(git_path(... and has the same problem. Do you want to fix them > too? Sure, I can do that. I will try to get it up soon. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug] git rev-parse --git-common-dir executed from a sub dir of the main worktree is wrong
I found a case where it seems that the result of `git rev-parse --git-common-dir` is incorrect. If you execute the command from within a subdirectory in the main worktree, it returns the path from the root of the worktree to the current dir + "/.git". (As a refresher, running this command from the root of the worktree returns ".git"). I wrote a quick test to demonstrate the problem: +test_expect_success 'git-common-dir inside sub-dir' ' + ( + mkdir -p path/to/child && + cd path/to/child && + echo "$(git rev-parse --show-toplevel)/.git" >expected && + git rev-parse --git-common-dir >actual && + test_cmp expected actual + ) +' + I suggest that we change the result of this call to _always_ return an absolute path. I would be willing to code this change, but I didn't want to start anything that may be considered backwards-incompatible. This seems related to [1]http://thread.gmane.org/gmane.comp.version-control.git/286038 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] worktree: add: introduce --checkout option
On Fri, Mar 25, 2016 at 7:41 AM, Duy Nguyen wrote: > On Fri, Mar 25, 2016 at 6:31 PM, Zhang Lei wrote: >> By the way, Duy, another unrelated question: why worktree name under >> .git/worktrees is being named >> after the working tree path basename? I think branch name is more >> reasonable since we don't allow checking out >> the same branch twice. > > Because branch name is not always available (e.g. detached HEAD) and > checkout branch can be switched later on. And normally you'll get > branch name there anyway with "git worktree add something" because the > branch "something" is automatically created. I've been wondering if > it's worth supporting "git worktree -b abc ./" where we create > worktree "./abc" based on branch name too. You can switch to any other branch in a worktree. Consider that you could switch branches in worktrees such that you could eventually end up having the branches swapped from original worktree setup. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/3] config: add '--sources' option to print the source of a config value
On Sat, Feb 13, 2016 at 9:24 AM, wrote: > From: Lars Schneider > > diff to v2: > * rename cmd to cmdline > * rename function current_config_name to current_config_type_name > * add 'type' to config_source struct that identifies config type > * fix config stdin error output and add test case "invalid stdin config" > * change delimiter between type and name from tab to colon > * remove is_query_action variable > * rename "--sources" to "--show-origin" > * add pathological test case "--show-origin stdin with file include" > * enumerate all valid commandline cases for "--show-origin" > * removed TODOs as there are no config include bugs > * describe '--includes' default behavior in git-config.txt > * split test cases > * use non-interpolated here-docs where possible > * improve readablity of 'show_config_origin' function by removing duality > > I renamed the flag from "--source" to "--show-origin" as I got the impression > that Sebastian, Peff and Ramsay seem to be all OK with "--show-origin". I know that I am late to the party here, but why not make the option `--verbose` or `-v`? `git config` doesn't have that now, and this seems like a logical thing to include as verbose data. I would venture to guess that `--verbose` is more likely to be the first thing that someone who is looking for this information will guess at before they run `git config --help`. > > Thanks Eric for the hint to simplify the 'show_config_origin' function. > I took your suggestion but modfied one line. Instead of "fputs" I used > "fwrite" to write the content. This was necssary as the last char of the > string could be \0 due to the "--null" flag. "fputs" would ignore that. > > In 959b545 Heiko introduced a config API to lookup .gitmodules values and > in "submodule-config.c" he uses the "git_config_from_buf" function [1]. I > wonder if my modifications to this function could trigger any unwanted side > effects in his implementation? (I can't imagine any but I want to make you > aware of this connection) > > > Thanks a lot Peff, Sebastian, Ramsey, and Eric for the review. > > > [1] > https://github.com/git/git/blob/494398473714dcbedb38b1ac79b531c7384b3bc4/submodule-config.c#L430-L431 > > > Lars Schneider (3): > git-config.txt: describe '--includes' default behavior > config: add 'type' to config_source struct that identifies config type > config: add '--show-origin' option to print the origin of a config > value > > Documentation/git-config.txt | 19 -- > builtin/config.c | 35 +++ > cache.h | 1 + > config.c | 23 +--- > t/t1300-repo-config.sh | 136 > ++- > t/t1308-config-set.sh| 4 +- > 6 files changed, 202 insertions(+), 16 deletions(-) > > -- > 2.5.1 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] show signature of commit in gitk
On Sun, Dec 13, 2015 at 9:15 AM, Daniel Fahlke wrote: > It seems my Patch got no attention yet, is there anything wrong? > Do I need to ping someone in particular? > gitk patches should cc +Paul Mackerras who maintains gitk -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] ff-refs: builtin command to fast-forward local refs
On Tue, Nov 17, 2015 at 10:28 AM, Michael J Gruber wrote: > Mike Rappazzo venit, vidit, dixit 17.11.2015 14:58: > > I still don't like the idea of having a new command just for the purpose > of fast-forwarding local branches from specified upstreams. > > What's wrong with "git merge --ff-only" once you check them out? We have > all the gory messages when you checkout a branch or use the git prompt > or "branch -vv". And if you don't - how is forgetting to "ff-refs" > better than forgetting to "merge --ff-only"? > > In short, I don't see a problem that this is solving, but maybe it's > because we use local branches differently, I dunno. For me I use this command more as a post-fetch: git fetch --all --prune && git-ff-refs I imagine that the big difference is in the number of branches that I maintain, and perhaps in the way that I use gitk to visualize them. I would be happy to add another option to git-fetch for --ff-refs as an alternative if that would feel better than a full-on builtin. > > If other people were interested they should or would have come up with > comments, I think (as a general rule). > > Cheers, > Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] ff-refs: builtin command to fast-forward local refs
On Wed, Nov 11, 2015 at 5:41 AM, Michael J Gruber wrote: > Michael Rappazzo venit, vidit, dixit 11.11.2015 03:11: >> This patch series is built on (based on) 'next' because it relies on >> worktree.c >> >> `ff-refs` will update local branches which can be fast-forwarded to their >> upstream tracking branch. Any branch which has diverged from the upstream >> will be left untouched by this command. Additionally, there are options >> for '--dry-run' and to '--skip-worktrees'. >> >> There are two primary update mechanisms for fast-forwarding a branch. >> - For a checked out branch, emulate `git-merge --ff-only` >> - For a non-checked out branch, emulate `git update-ref` >> >> When run on a repo with multiple worktrees (created with git-worktree add), >> git-ff-refs will take that into account when fast-forwarding. That is, it >> will run in 'merge --ff-only' emulation mode when a branch is checked out >> in a worktree, rather than in 'update-ref' mode. >> >> The primary benefit of ff-refs will come for those who maintain several >> local branches which track upstream remote branches that update often. The >> intended usage pattern is to run `git-fetch` followed by `git-ff-refs`. > > I'm sorry, but I don't see why this deserves a new command. If refspec > with and without "+" are not enough then maybe "git fetch --all" or "git > remote update" should learn a new "--ff-only" option (ignoring all "+") > like merge has. Maybe I wasn't clear in my description, or maybe I misunderstand something. This command is about updating local refs (branches, really), not the local copy of a remote ref. If, for example I have local branches: master -> origin/master next -> origin/next pu -> origin/pu feature1 -> features/feature1 feature2 -> features/feature2 feature3 -> features/feature3 bug1 -> features/bugs/bug1 bug2 -> features/bugs/bug2 If I don't use multiple worktrees, I probably only have one of those checked out at any one time. If any of the upstream branches are updated, then when I fetch those branches will be behind. If I wanted to make sure that the branches I am not touching are updated, I would have to do it individually (AFAIK). And why not update my local worktree if it is a fast-forward?. This command aims to put that local branch update into a single command. > git fetch --all fetching origin... abc1234..abc1235 next -> origin/next abd1234..abd1235 pu -> origin/pu fetching features... 123abcd..123abce feature1 -> features/feature1 + 124abcd...124abce feature2 -> features/feature2 125abcd..125abce feature3 -> features/feature3 > git ff-refs master -> origin/master.[UP-TO-DATE] next -> origin/next.[UPDATED] pu -> origin/pu.[UPDATED] feature1 -> features/feature1...[UPDATED] feature2 -> features/feature2...[NON-FAST-FORWARD] feature3 -> features/feature3...[UPDATED] bug1 -> features/bugs/bug1..[UP-TO-DATE] bug2 -> features/bugs/bug2..[UP-TO-DATE] For reference, I have been using a scripted version of this command [1]. Assuming that I change your mind on this command, I will add this example to the help doc. > > As for updating worktrees: This shouldn't be taken too lightly anyways. > But the worktree interface still has some rough edges, and I would hope > that it learns a "foreach" subcommand very much like the submodule > version. That would allow you to > > git worktree foreach git merge --ff-only > > with a systematic aproach that opens many other opportunities. > > Michael I am aware of the current status of the worktrees command (I worked on the 'list' command). If a user only wants to update unchecked out branches, there is a command line option provided, '--skip-worktrees'. The foreach command sounds like a good idea, but I don't know that it would help here, as ff-refs is looping through all of the refs already (ala for-each-ref). If you are proposing foreach-worktree as an alternative, that is good for half of the command, but I would still want to update the unchecked out refs. _Mike [1] https://github.com/rappazzo/dotfiles/blob/ff-refs/bin/git-ff-refs -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Wrong worktree path when using multiple worktree
On Tue, Nov 3, 2015 at 5:27 PM, Mike Rappazzo wrote:
> On Tue, Nov 3, 2015 at 11:58 AM, Nicolas Morey-Chaisemartin
> wrote:
>> Hi,
>>
>> There seem to be an issue with the path computed for a worktree when
>> multiple worktree were created (using git worktree)
>> In my Setup, I have 3 repos:
>> A/repo (main One)
>> A/repo-patches (worktree, using branch dev)
>> B/repo (worktree, using branch next)
>>
>> I'm working in A/repo-patches an run:
>> $ git checkout next
>> fatal: 'next' is already checked out at 'A/repo-patches'
>>
>> Which is partially true but not completely.
>> I looked a bit in the code and found that the issue comes from here
>> (get_linked_worktree):
>> if (!strbuf_strip_suffix(&worktree_path, "/.git")) {
>> strbuf_reset(&worktree_path);
>> strbuf_addstr(&worktree_path, absolute_path("."));
>> strbuf_strip_suffix(&worktree_path, "/.");
>> }
>> Because it wrongfully assumes that I am in the linked worktree.
>> I checked in the .git/worktree files and couldn't see anything that actually
>> points to where the repo are setup.
>>
>> Nicolas
>
> This is code that I worked on, but I am unable to reproduce it locally
> just yet. Before I dig too deep, could you report the contents of
> "A/repo/.git/worktrees/repo-patches/gitdir" (or similar)? There is
> another issue reported[1] where the contents of the gitdir for a
> linked worktree are overwritten in some cases. A fix for this is
> being worked on (see discussion).
>
> In the mean time, I will continue to try and reproduce.
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/280307
Followup: I was able to reproduce the error when I tried to simulate
the aforementioned bug. Here is a test which I added to t2027 (not
intended to publish):
+test_expect_success '"checkout" branch already checked out' '
+ git worktree add -b linked_1_br linked_1 master &&
+ git worktree add -b linked_2_br linked_2 master &&
+ echo ".git" > .git/worktrees/linked_2/gitdir &&
+ test_must_fail git -C linked_1 checkout linked_2_br
+'
+
Test run result:
Preparing linked_1 (identifier linked_1)
HEAD is now at 2519212 init
Preparing linked_2 (identifier linked_2)
HEAD is now at 2519212 init
fatal: 'linked_2_br' is already checked out at
'/Users/mike/code/git-source/t/trash
directory.t2027-worktree-list/linked_1'
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Wrong worktree path when using multiple worktree
On Tue, Nov 3, 2015 at 11:58 AM, Nicolas Morey-Chaisemartin
wrote:
> Hi,
>
> There seem to be an issue with the path computed for a worktree when multiple
> worktree were created (using git worktree)
> In my Setup, I have 3 repos:
> A/repo (main One)
> A/repo-patches (worktree, using branch dev)
> B/repo (worktree, using branch next)
>
> I'm working in A/repo-patches an run:
> $ git checkout next
> fatal: 'next' is already checked out at 'A/repo-patches'
>
> Which is partially true but not completely.
> I looked a bit in the code and found that the issue comes from here
> (get_linked_worktree):
> if (!strbuf_strip_suffix(&worktree_path, "/.git")) {
> strbuf_reset(&worktree_path);
> strbuf_addstr(&worktree_path, absolute_path("."));
> strbuf_strip_suffix(&worktree_path, "/.");
> }
> Because it wrongfully assumes that I am in the linked worktree.
> I checked in the .git/worktree files and couldn't see anything that actually
> points to where the repo are setup.
>
> Nicolas
This is code that I worked on, but I am unable to reproduce it locally
just yet. Before I dig too deep, could you report the contents of
"A/repo/.git/worktrees/repo-patches/gitdir" (or similar)? There is
another issue reported[1] where the contents of the gitdir for a
linked worktree are overwritten in some cases. A fix for this is
being worked on (see discussion).
In the mean time, I will continue to try and reproduce.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/280307
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's the ".git/gitdir" file?
On Tue, Oct 27, 2015 at 6:54 PM, Junio C Hamano wrote: > Kyle Meyer writes: > >> When a ".git" file points to another repo, a ".git/gitdir" file is >> created in that repo. >> >> For example, running >> >> $ mkdir repo-a repo-b >> $ cd repo-a >> $ git init >> $ cd ../repo-b >> $ echo "gitdir: ../repo-a/.git" > .git >> $ git status >> >> results in a file "repo-a/.git/gitdir" that contains >> >> $ cat repo-a/.git/gitdir >> .git > > Sounds like a bug in the recently added "worktree" stuff. Perhaps > update_linked_gitdir() tweaked by 82fde87f (setup: update the right > file in multiple checkouts, 2015-08-25) is misbehaving? I noticed that as I was working on the worktree list command that my linked worktree gitdir files were being clobbered to '.git'. I attributed it to my work, but now that you mention it, I think it has happened with the 2.6.1 release as well. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How to rebase when some commit hashes are in some commit messages
On Tue, Oct 13, 2015 at 1:07 PM, Jacob Keller wrote: > On Tue, Oct 13, 2015 at 6:29 AM, Philip Oakley wrote: >> My tuppence is that the only sha1's that could/would be rewritten would be >> those for the commits within the rebase. During rebasing it is expected that >> the user is re-adjusting things for later upstream consumption, with social >> controls and understandings with colleagues. >> > > Agreed here. There would be no need to change any sha1s that didn't > change during the rebase. This limits the scope. Alright. > >> Thus the only sha1 numbers that could be used are those that are within the >> (possibly implied) instruction sheet (which will list the current sha1s that >> will be converted by rebase to new sha1's). >> > > Correct, you would be able to limit the number of sha1s to search for. > > However, (see below), any reasonable reference to a sha1 should be > relatively stable. > >> It should be clear that the sha1's are always backward references (because >> of the impossibility of including a forward reference to an as yet >> un-created future commit's sha1). >> >> The key question (for me) is whether short sha1s are accepted, or if they >> must be full 40 char sha1's (perhaps an option). There are already options >> for making sure that short refs are not ambiguous. >> >> It sound to me like a sensible small project for those that have such a >> workflow. I'm not sure if it should work with a patch based flow when >> submitting upstream - I'm a little fuzzy on how would the upstream >> maintainer know which sha1 referred to which patch. >> > > My issue: the only sha1s in commit messages are *generally* things > which will NOT be changed in general. Supporting a work flow that > wants to change these is definitely crazy. > > Essentially: I don't see a reason that you would be rebasing a commit > and needing to change any references in it. You can reference a commit > which isn't changing, but here's the possible situations I see: > > a) you are rebasing a commit which references in the message a commit > that is not being changed (it is ancient) > > In this case, nothing needs to be done. > > b) you are rebasing a commit which references another commit in the same > rebase > > I see no valid reason to reference a sha1 in this case. If you're > referencing as a "fixes", then you are being silly since you can just > squash the fix into the original commit and thus prevent introduction > of bug at all. > > What other reason? If you are referencing such as "thix extends > implementation from sha1" then your commit message is probably poorly > formatted. I don't see a reason to support this flow. > > c) you are rebasing a commit which is referencing a commit that has > already been changed. (?) > > I think (maybe) this is your interesting case, but here are some caveats. > > Let's say you are fixing some old commit such as "Fixes: summary, date>" or something. > > If you do a "git pull --rebase", your commit might be updated to play > ontop of more new work, but the sha1 should still be valid, *unless* > the remote history did some rewind, at which point I don't think any > algorithm will work, see the issues above. > > It may be something worth doing in git-filter-branch, but then you're > looking at losing the two assumptions above making it really hard to > get right. > > Regards, > Jake It seems reasonable that this could be added as a feature of interactive rebase. The todo list could be automatically adjusted to "reword" for those commits which are referring to other commits within the same rebase. As each commit is re-written, a mapping could be kept of old sha1 -> new sha1. Then when one of the reworded commits is being applied, the old sha1 -> new sha1 mapping could be used to add a line to the $COMMIT_MSG. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2015, #01; Mon, 5)
On Tue, Oct 6, 2015 at 1:55 AM, Junio C Hamano wrote:
> Torsten Bögershausen writes:
>
>> Minor comment from my compiler:
>>
>> worktree.c:181: warning: assuming signed overflow does not occur when
>> assuming
>> that (X + c) > X is always true
>
> Thanks; I think the allocation in that function (the use it uses
> ALLOC_GROW()) is somewhat bogus.
>
> It does this:
>
> if ((linked = get_linked_worktree(d->d_name))) {
> ALLOC_GROW(list, alloc + 1, alloc);
> list[counter++] = linked;
> }
>
> where "alloc" keeps track of the size of the list, and "counter"
> keeps track of the first unused entry. The second argument to the
> helper macro smells bad.
>
> The correct way to use ALLOC_GROW() helper macro is:
>
> * Use three variables, an array, the size of the allocation and the
>size of the used part of the array. If you call the array $thing,
>the size of the allocation is typically called $thing_alloc and
>the size of the used part $thing_nr. E.g. opts[], opts_alloc & opts_nr.
>
> * When adding a new thing at the end of the $thing, do this:
>
> ALLOC_GROW($thing, $thing_nr + 1, $thing_alloc);
> $thing[$thing_nr++] = <>;
>
>
> Perhaps something like this needs squashing in.
>
> Subject: [PATCH] fixup! worktree: add a function to get worktree details
>
> ---
> worktree.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/worktree.c b/worktree.c
> index 90282d9..f7304a2 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -178,12 +178,13 @@ struct worktree **get_worktrees(void)
> continue;
>
> if ((linked =
> get_linked_worktree(d->d_name))) {
> - ALLOC_GROW(list, alloc + 1, alloc);
> + ALLOC_GROW(list, counter + 1, alloc);
> list[counter++] = linked;
> }
> }
> closedir(dir);
> }
> + ALLOC_GROW(list, counter + 1, alloc);
> list[counter] = NULL;
> return list;
> }
> --
> 2.6.1-284-g1f14bb6
>
Thanks for clearing this up for me. I will add it to my branch an
re-roll in a day or two.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Convenient shortcut to push delete current branch?
On Thu, Oct 1, 2015 at 2:37 PM, Robert Dailey wrote: > On Thu, Oct 1, 2015 at 1:22 PM, Jacob Keller wrote: >> On Thu, Oct 1, 2015 at 9:43 AM, Robert Dailey >> wrote: >>> For convenient pushing of current branch, git supports this syntax: >>> >>> $ git push origin HEAD >>> >>> This will push your current branch up. However, is there such a >>> shortcut for *deleting* the branch? The only goal here is to avoid >>> having to type the branch name in the push command. Normally I rely on >>> tab completion but we have tons of branches, all which start with some >>> prefix mixed with numbers, so it becomes cumbersome to rely on tab >>> completion. Ideally I'd like to be able to do: >>> >>> $ git push --delete origin HEAD >>> $ git push origin :HEAD >>> >>> Is there a syntax like this available? >> >> You can do >> >> git push origin: >> >> but I don't believe HEAD is supported. It might be valuable to extend >> push to have a --delete option which would maybe be useful for those >> who didn't learn the full refspec syntax. > > Push already has a --delete option. > I could see adding an option for --delete-upstream that would use the upstream remote and ref of the current branch (if they exist). External to git you could script this from the config (completely untested): if branch=$(git symbolic-ref --short HEAD); then if remote=$(git config branch.$branch.remote); then if remote_ref=$(git config branch.$branch.merge); then git push $remote --delete $remote_ref fi fi fi >> I don't think git push origin :HEAD makes too much sense, since that's >> on the remote side of a refspec, and you want it interpreted >> locally... I suppose it makes sense somewhat, but other refspecs with >> HEAD on the remote side of the refspec don't really make sense, where >> as HEAD always makes sense on the local side of the refspec. > > HEAD makes sense on the remote side if you think of it like an alias: > > HEAD -> branch-name -> SHA1 > > HEAD simply points to branch-name. It makes sense for git to assume > that we should never do anything with real HEAD ref on the remote > side, and instead treat it as a substitution for the remote name. My > assumption may not be correct, but at the very least it should be a > niche case. > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: can't install on OS X
Looks like you have it installed properly. The typical usage is from the terminal, (try `git --version` to be sure). There is also a pretty great UI packaged with git called git-gui. You should be able to make a link or an alias to it in your Applications folder (or invoke it from the terminal `git gui`). On Fri, Oct 2, 2015 at 2:50 AM, Spencer Graves wrote: > What's the procedure for installing Git under OS X 10.11? > > > I downloaded "git-2.5.3-intel-universal-mavericks.dmg" per instructions. > When I tried to install it, I first had trouble because it wasn't from the > Mac App Store nor an "identified developer". I ultimately found "System > Preferences" > "Security & Privacy" > "Click the lock to make changes" > > entered password > AND clicked to "Allow apps downloaded from: Anywhere". > Then the install appeared to run and proclaimed, "The installation was > successful." However, git is not listed under "Applications", and RStudio > says, "Git was not detected on the system path." > > > "README.txt" says I need "sudo mv /usr/bin/git /usr/bin/git-system". I > tried that and got, "mv: rename /usr/bin/git to /usr/bin/git-system: > Operation not permitted" (after entering my password). [My directory now > includes "/usr/local/git", and "/usr/bin" includes git, git-cvsserver, > git-receive-pack, git-shell, git-upload-archive, and git-upload-pack.] > > > Suggestions? Thanks, Spencer Graves > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 4/4] worktree: add 'list' command
On Tue, Sep 22, 2015 at 3:42 PM, Junio C Hamano wrote:
> Michael Rappazzo writes:
>
>>
>> +--porcelain::
>> + With `list`, output in an easy-to-parse format for scripts.
>> + This format will remain stable across Git versions and regardless of
>> user
>> + configuration.
>
> ... and exactly what does it output? That would be the first
> question a reader of this documentation would ask.
>
I will add that description. I have mostly followed what Eric
suggested in the v7 series for a porcelain format. Does the porcelain
format restrict additive changes? That is, is it OK for a future
patch to add another field in the format, as long as it doesn't alter
the other values? Is the format that I have used here acceptable
(assuming the changes proposed below are made)?
>
>> @@ -93,6 +106,7 @@ OPTIONS
>> --expire ::
>> With `prune`, only expire unused working trees older than .
>>
>> +
>
> ?
>
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index 71bb770..e6e36ac 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -8,10 +8,13 @@
>> #include "run-command.h"
>> #include "sigchain.h"
>> #include "refs.h"
>> +#include "utf8.h"
>> +#include "worktree.h"
>>
>> static const char * const worktree_usage[] = {
>> N_("git worktree add [] "),
>> N_("git worktree prune []"),
>> + N_("git worktree list []"),
>> NULL
>> };
>>
>> @@ -359,6 +362,79 @@ static int add(int ac, const char **av, const char
>> *prefix)
>> return add_worktree(path, branch, &opts);
>> }
>>
>> +static void show_worktree_porcelain(struct worktree *worktree)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> +
>> + strbuf_addf(&sb, "worktree %s\n", worktree->path);
>> + if (worktree->is_bare)
>> + strbuf_addstr(&sb, "bare");
>> + else {
>> + if (worktree->is_detached)
>> + strbuf_addf(&sb, "detached at %s",
>> find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));
>> + else
>> + strbuf_addf(&sb, "branch %s",
>> shorten_unambiguous_ref(worktree->head_ref, 0));
>> + }
>
> Writing the above like this:
>
> if (worktree->is_bare)
> ...
> else if (worktree->is_detached)
> ...
> else
> ...
>
> would make it a lot more clear that there are three cases.
>
> Also, I doubt --porcelain output wants shorten or abbrev.
>
> Human-readability is not a goal. Reproducibility is. When you run
> "worktree list" today and save the output, you want the output from
> "worktree list" taken tomorrow to exactly match it, even after
> creating many objects and tags with conflicting names with branches,
> as long as you didn't change their HEADs in the meantime.
>
>> +
>> + printf("%s\n\n", sb.buf);
>> +
>> + strbuf_release(&sb);
>
> I am not sure what the point of use of a strbuf is in this function,
> though. Two printf's for each case (one for the common "worktree
> %s", the other inside if/elseif/else cascade) should be sufficient.
>
>> +static void show_worktree(struct worktree *worktree, int path_maxlen)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> +
>
> Remove this blank line. You are still declaring variables.
>
>> + int cur_len = strlen(worktree->path);
>> + int utf8_adj = cur_len - utf8_strwidth(worktree->path);
>
> Have a blank line here, instead, as now you start your statements.
>
>> + strbuf_addf(&sb, "%-*s ", 1 + path_maxlen + utf8_adj, worktree->path);
>> + if (worktree->is_bare)
>> + strbuf_addstr(&sb, "(bare)");
>> + else {
>> + strbuf_addf(&sb, "%s ",
>> find_unique_abbrev(worktree->head_sha1, DEFAULT_ABBREV));
>
> Personally I am not a big fan of the the alignment and use of
> utf8_strwidth(), but by using find_unique_abbrev() here, you are
> breaking the alignment, aren't you? " [branchname]" that follows
> the commit object name would not start at the same column, when
> you have many objects that default-abbrev is not enough to uniquely
> identify them.
>
> And it can easily be fixed by computing the unique-abbrev length for
> all the non-bare worktree's HEADs in the same loop you computed
> path_maxlen() in the caller, passing that to this function, and use
> that as mininum abbrev length when computing the unique-abbrev.
>
I only intended for the path to be right padded, since paths can vary
in length so widely. I found it much easier to read with the path
right-padded. I think that doing a full column align isn't the best
looking output in this case. I tried to model this output after `git
branch -vv`. I didn't notice if that does a full align if the
shortened refs are differently sized. I will try it out, and see if
it makes a significant visual impact.
>> + if (!worktree->is_detached)
>> + strbuf_addf(&sb, "[%s]",
>> shorten_unambiguous_ref(worktree->head_ref, 0));
>> + else
>> +
Re: [PATCH v7 2/3] worktree: move/refactor find_shared_symref from branch.c
On Wed, Sep 16, 2015 at 5:09 PM, Eric Sunshine wrote:
> On Mon, Sep 14, 2015 at 1:44 PM, Mike Rappazzo wrote:
>> On Sat, Sep 12, 2015 at 11:19 PM, Eric Sunshine
>> wrote:
>>> On Fri, Sep 4, 2015 at 5:39 PM, Michael Rappazzo wrote:
>>>> + while (!matched && worktree_list) {
>>>> + if (strcmp("HEAD", symref)) {
>>>> + strbuf_reset(&path);
>>>> + strbuf_reset(&sb);
>>>> + strbuf_addf(&path, "%s/%s",
>>>> worktree_list->worktree->git_dir, symref);
>>>> +
>>>> + if (_parse_ref(path.buf, &sb, NULL)) {
>>>> + continue;
>>>> + }
>>>> +
>>>> + if (!strcmp(sb.buf, target))
>>>> + matched = worktree_list->worktree;
>>>
>>> The original code in branch.c, which this patch removes, did not need
>>> to make a special case of HEAD, so it's not immediately clear why this
>>> replacement code does so. This is the sort of issue which argues in
>>> favor of mutating the existing code (slowly) over the course of
>>> several patches into the final form, rather than having the final form
>>> come into existence out of thin air. When the changes are made
>>> incrementally, it is easier for reviewers to understand why such
>>> modifications are needed, which (hopefully) should lead to fewer
>>> questions such as this one.
>>
>> I reversed the the check here; it is intended to check if the symref
>> is _not_ the head, since the head
>> ref has already been parsed. This is used in notes.c to find
>> "NOTES_MERGE_REF".
>
> I'm probably being dense, but I still don't understand why the code
> now needs a special case for HEAD, whereas the original didn't. But,
> my denseness my be indicative of this change not being well-described
> (or described at all) by the commit message. Hopefully, when this is
> refactored into finer changes, the purpose will become clear.
>
> Thanks.
The special case for HEAD is because the HEAD is already included in
the worktree struct. This block is intended to save from re-parsing.
If you think the code would be easier to read, the HEAD check could be
removed, and the ref will just be parsed always.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/3] worktree: add top-level worktree.c
On Wed, Sep 16, 2015 at 4:32 PM, Eric Sunshine wrote:
> On Mon, Sep 14, 2015 at 8:20 AM, Mike Rappazzo wrote:
>> On Sat, Sep 12, 2015 at 10:39 PM, Eric Sunshine
>> wrote:
>>>> +struct worktree {
>>>> + char *path;
>>>> + char *git_dir;
>>>> + char *head_ref;
>>>> + unsigned char head_sha1[20];
>>>> + int is_detached;
>>>> + int is_bare;
>>>> +};
>>>> +
>>>> +struct worktree_list {
>>>> + struct worktree *worktree;
>>>> + struct worktree_list *next;
>>>> +};
>>>
>>> I don't care too strongly, but an alternate approach (which I probably
>>> would have taken) would be to have get_worktrees() simply return an
>>> array of 'struct worktree' objects, hence no need for the additional
>>> 'struct worktree_list'. The slight complication with this approach,
>>> though, is that get_worktrees() either also needs to return the length
>>> of the array, or the array should end with some sort of end-of-array
>>> sentinel. An obvious sentinel would be path==NULL or git_dir==NULL or
>>> all of the above.
>>>
>>> Client iteration is just about the same with the array approach as
>>> with the linked-list approach.
>>
>> I can't see what benefit this would provide. I would sooner change
>> the returned list into
>> an array-backed list struct. Alternatively, I think adding a
>> list_head pointer to this structure
>> could benefit client code.
>
> The benefit is a reduction in complexity, which is an important goal.
> Linked lists are inherently more complex than arrays, requiring extra
> effort, and especially extra care, to manage the head, each "next"
> pointer (and possibly a tail pointer). Arrays are used often in Git,
> thus are familiar in this code-base, and Git has decent support for
> making their management relatively painless. Your suggestions of
> changing into "an array-backed list structure" or "adding list_head
> pointer" increase complexity, rather than reduce it.
>
> Aside from the complexity issue, arrays allow random access, whereas
> linked lists allow only sequential access. While this limitation may
> not impact your current, planned use of get_worktrees(), it places a
> potentially unnecessary restriction on future clients.
>
> And, as noted, client iteration is at least as convenient with arrays,
> if not moreso, due to the reduction in noise ("p++" rather than "p =
> p->next").
>
> struct worktree *p;
> for (p = get_worktrees(); p->path; p++)
> blarp(p);
>
> There are cases where linked lists are an obvious win, but this
> doesn't seem to be such a case. On the other hand, there are obvious
> benefits to making this an array, such as reduced complexity and
> removal of the sequential-access-only restriction.
What you are suggesting makes sense, but I feel it falls short when it
comes to the "sentinel". Would the last element in the list be a
dummy worktree? I would sooner add a NULL to the end. Would that be
an acceptable implementation?
I have re-coded it to put the next pointer in the worktree structure
as Junio has suggested, but I am open to changing it to use the array
approach.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/3] worktree: move/refactor find_shared_symref from branch.c
On Sat, Sep 12, 2015 at 11:19 PM, Eric Sunshine wrote:
> On Fri, Sep 4, 2015 at 5:39 PM, Michael Rappazzo wrote:
>> The code formerly in branch.c was largely the basis of the
>> get_worktree_list implementation is now moved to worktree.c,
>> and the find_shared_symref implementation has been refactored
>> to use get_worktree_list
>
> Some comments below in addition to those by Junio...
>
>> Signed-off-by: Michael Rappazzo
>> ---
>> diff --git a/branch.h b/branch.h
>> index d3446ed..58aa45f 100644
>> --- a/branch.h
>> +++ b/branch.h
>> @@ -59,12 +59,4 @@ extern int read_branch_desc(struct strbuf *, const char
>> *branch_name);
>> */
>> extern void die_if_checked_out(const char *branch);
>>
>> -/*
>> - * Check if a per-worktree symref points to a ref in the main worktree
>> - * or any linked worktree, and return the path to the exising worktree
>> - * if it is. Returns NULL if there is no existing ref. The caller is
>> - * responsible for freeing the returned path.
>> - */
>> -extern char *find_shared_symref(const char *symref, const char *target);
>> -
>> #endif
>> diff --git a/worktree.c b/worktree.c
>> index 33d2e57..e45b651 100644
>> --- a/worktree.c
>> +++ b/worktree.c
>> @@ -155,3 +155,43 @@ done:
>> return list;
>> }
>>
>> +char *find_shared_symref(const char *symref, const char *target)
>> +{
>> + char *existing = NULL;
>> + struct strbuf path = STRBUF_INIT;
>> + struct strbuf sb = STRBUF_INIT;
>> + struct worktree_list *worktree_list = get_worktree_list();
>> + struct worktree_list *orig_list = worktree_list;
>> + struct worktree *matched = NULL;
>> +
>> + while (!matched && worktree_list) {
>> + if (strcmp("HEAD", symref)) {
>
> The result of strcmp() never changes, so this (expensive) check could
> be lifted out of the 'while' loop, however...
>
>> + strbuf_reset(&path);
>> + strbuf_reset(&sb);
>> + strbuf_addf(&path, "%s/%s",
>> worktree_list->worktree->git_dir, symref);
>> +
>> + if (_parse_ref(path.buf, &sb, NULL)) {
>> + continue;
>> + }
>> +
>> + if (!strcmp(sb.buf, target))
>> + matched = worktree_list->worktree;
>
> The original code in branch.c, which this patch removes, did not need
> to make a special case of HEAD, so it's not immediately clear why this
> replacement code does so. This is the sort of issue which argues in
> favor of mutating the existing code (slowly) over the course of
> several patches into the final form, rather than having the final form
> come into existence out of thin air. When the changes are made
> incrementally, it is easier for reviewers to understand why such
> modifications are needed, which (hopefully) should lead to fewer
> questions such as this one.
>
I reversed the the check here; it is intended to check if the symref
is _not_ the head, since the head
ref has already been parsed. This is used in notes.c to find
"NOTES_MERGE_REF". I will move the
check out of the loop as you suggest above.
>> + } else {
>> + if (worktree_list->worktree->head_ref &&
>> !strcmp(worktree_list->worktree->head_ref, target))
>> + matched = worktree_list->worktree;
>> + }
>> + worktree_list = worktree_list->next ? worktree_list->next :
>> NULL;
>> + }
>> +
>> + if (matched) {
>> + existing = malloc(strlen(matched->path) + 1);
>> + strcpy(existing, matched->path);
>
> A couple issues:
>
> This can be done more concisely and safely with xstrdup().
>
> In this codebase, it probably would be more idiomatic to use goto to
> drop out of the loop rather than setting 'matched' and then having to
> check 'matched' in the loop condition. So, for instance, each place
> which sets 'matched' could instead say:
>
> existing = xstrdup(...);
> goto done;
>
>> + }
>> +
>> +done:
>
> This label doesn't have any matching goto's.
>
>> + strbuf_release(&path);
>> + strbuf_release(&sb);
>> + worktree_list_release(orig_list);
>> +
>> + return existing;
>> +}
>> diff --git a/worktree.h b/worktree.h
>> index 2bc0ab8..320f17e 100644
>> --- a/worktree.h
>> +++ b/worktree.h
>> @@ -45,4 +45,11 @@ struct worktree *get_worktree(const char *id);
>> extern void worktree_list_release(struct worktree_list *);
>> extern void worktree_release(struct worktree *);
>>
>> +/*
>> + * Look for a symref in any worktree, and return the path to the first
>> + * worktree found or NULL if not found. The caller is responsible for
>> + * freeing the returned path.
>> + */
>
> For some reason, this comment differs a fair bit from the original in
> branch.h which was removed by this patch, however, the original
> comment was a bit more explanatory (I think).
>
> As a general rule, it's be
Re: [PATCH v7 1/3] worktree: add top-level worktree.c
On Sat, Sep 12, 2015 at 10:39 PM, Eric Sunshine wrote:
> I realize that this is modeled closely after existing code in
> branch.c, but, with the exception of parsing the ref file and
> constructing a worktree structure, the main worktree case (id == NULL)
> is entirely disjoint from the linked worktree case (id != NULL). This
> suggests strongly that get_worktree() should be split into two
> functions, one for the main worktree and one for linked worktrees,
> which would make the code easier to understand. You might call the
> functions get_main_worktree() and get_linked_worktree(id) (or perhaps
> drop "linked" from the latter name).
I originally wrote it like that, but I felt that the code looked like
it was mostly duplicated in the functions.
I can give it a relook.
>> +
>> +struct worktree_list *get_worktree_list()
>
> Can we be more concise and call this get_worktrees()?
>
I prefer 'get_worktree_list' because I also added the 'get_worktree'
function, and I wanted to differentiate
the function names.
>> diff --git a/worktree.h b/worktree.h
>> new file mode 100644
>> index 000..2bc0ab8
>> --- /dev/null
>> +++ b/worktree.h
>> @@ -0,0 +1,48 @@
>> +#ifndef WORKTREE_H
>> +#define WORKTREE_H
>> +
>> +struct worktree {
>> + char *path;
>> + char *git_dir;
>> + char *head_ref;
>> + unsigned char head_sha1[20];
>> + int is_detached;
>> + int is_bare;
>> +};
>> +
>> +struct worktree_list {
>> + struct worktree *worktree;
>> + struct worktree_list *next;
>> +};
>
> I don't care too strongly, but an alternate approach (which I probably
> would have taken) would be to have get_worktrees() simply return an
> array of 'struct worktree' objects, hence no need for the additional
> 'struct worktree_list'. The slight complication with this approach,
> though, is that get_worktrees() either also needs to return the length
> of the array, or the array should end with some sort of end-of-array
> sentinel. An obvious sentinel would be path==NULL or git_dir==NULL or
> all of the above.
>
> Client iteration is just about the same with the array approach as
> with the linked-list approach.
>
I can't see what benefit this would provide. I would sooner change
the returned list into
an array-backed list struct. Alternatively, I think adding a
list_head pointer to this structure
could benefit client code.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/3] worktree: move/refactor find_shared_symref from branch.c
On Fri, Sep 11, 2015 at 7:10 PM, Eric Sunshine wrote: > On Fri, Sep 11, 2015 at 5:52 PM, Junio C Hamano wrote: > > Thanks for bringing this up. I haven't found a sufficient block of > time yet to review these patches, however, I had the same thought upon > a quick initial read, and is how I was hoping to see the patch series > constructed based upon my earlier two reviews suggesting refactoring > the existing branch.c functions into a new get_worktrees(). There are > at least a couple important reasons for taking this approach. > > First, it keeps the "blame" trail intact, the full context of which > can be helpful to someone trying to understand why the code is the way > it is. The current approach of having get_worktree_list() materialize > out of thin air (even though it may have been patterned after existing > code) doesn't give the same benefit. > > Second, it's easier for reviewers to understand and check the code for > correctness if it mutates to the final form in small increments than > it is to have get_worktrees() come into existence fully matured. > > A final minor comment: Since all three branch.c functions, > die_if_checked_out(), find_shared_symref(), and find_linked_symref(), > ought to be moved to top-level worktree.c, I'd probably have patch 1 > do the relocation (with no functional changes), and subsequent patches > refactor the moved code into a general purpose get_worktrees(). The > final patch would then implement "git worktrees list". I like the way this history works out, and I have reworked the history to follow this idea. The only thing I didn't do was move the die_if_checked_out() function from branch.c, as I feel that this function is more of a branch feature than a worktree feature. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 2/3] worktree: move/refactor find_shared_symref from branch.c
On Fri, Sep 11, 2015 at 12:16 PM, Junio C Hamano wrote: > Michael Rappazzo writes: > >> The code formerly in branch.c was largely the basis of the >> get_worktree_list implementation is now moved to worktree.c, >> and the find_shared_symref implementation has been refactored >> to use get_worktree_list >> > > Copying the bulk of the function in 1/3 and then removing the > original here made it somewhat hard to compare what got changed in > the implementation. > > I _think_ the code structure in the end result is more or less > right, though. Should I squash these first two commits together in the next series? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/3] worktree: add top-level worktree.c
On Thu, Sep 10, 2015 at 4:04 PM, Junio C Hamano wrote:
> Michael Rappazzo writes:
>
>> Including functions to get the list of all worktrees, and to get
>> a specific worktree (primary or linked).
>
> Was this meant as a continuation of the sentence started on the
> Subject line, or is s/Including/Include/ necessary?
I think I was continuing the subject line. I will make it nicer.
>
>> + worktree_list = next;
>> + }
>> +}
>> +
>> +/*
>> + * read 'path_to_ref' into 'ref'. Also set is_detached to 1 if the ref is
>> detatched
>> + *
>> + * return 1 if the ref is not a proper ref, 0 otherwise (success)
>
> These lines are overly long.
>
>> + */
>> +int _parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
>> +{
>> + if (!strbuf_readlink(ref, path_to_ref, 0)) {
>> + if (!starts_with(ref->buf, "refs/") ||
>> check_refname_format(ref->buf, 0)) {
>
> An overly long line. Perhaps
>
> if (!starts_with(ref->buf, "refs/") ||
> check_refname_format(ref->buf, 0)) {
>
> (I see many more "overly long line" after this point, which I won't mention).
Is the limit 80 characters?
>
>> + /* invalid ref - something is awry with this repo */
>> + return 1;
>> + }
>> + } else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
>> + if (starts_with(ref->buf, "ref:")) {
>> + strbuf_remove(ref, 0, strlen("ref:"));
>> + strbuf_trim(ref);
>
> We don't do the same "starts_with() and format is sane" check?
The code from this snippet was mostly ripped from branch.c (see the
second commit).
I will add the sanity check.
>
>
> An idiomatic way to extend a singly-linked list at the end in our
> codebase is to use a pointer to the pointer that has the element at
> the end, instead of to use a pointer that points at the element at
> the end or NULL (i.e. the pointer the above code calls current_entry
> is "struct worktree_list **end_of_list"). Would it make the above
> simpler if you followed that pattern?
>
I think I follow what you are saying here. I will explore this route.
I am also unhappy about
having to separately maintain a point to the head of the list when
using it. Would it be
"normal" to add a head pointer to the list structure?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/2] worktree: add 'for_each_worktree' function
On Mon, Aug 31, 2015 at 3:47 PM, Eric Sunshine wrote: > On Mon, Aug 31, 2015 at 2:57 PM, Mike Rappazzo wrote: >> I wasn't sure that a bare repo would be considered a worktree. I >> don't think that it would be >> a good idea to include it. In the same vein that I can't checkout a >> branch in a bare repo, it >> figure that it shouldn't be in the list. > > I forgot to mention in my previous response that I have the opposite > view, and think that a bare repo should be included in the output of > "git worktree list". The reason is that the intention of "git worktree > list" is to give the user a consolidated view of the locations of all > components of his "workspace". By "workspace", I mean the repository > (bare or not) and its worktrees. > > In the typical case, the .git directory resides within the main > worktree (the first item output by "git worktree list"), thus is > easily found, however, if "git worktree list" hides bare repos, then > the user will have no way to easily locate the repository (without > resorting to lower-level commands or peeking at configuration files). This makes sense, but would we also want to decorate it in the `git worktree list` command? Would porcelain list output be able to differentiate it? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/2] worktree: add 'for_each_worktree' function
On Mon, Aug 31, 2015 at 1:11 AM, Eric Sunshine wrote:
> Thanks for working on this. I apologize for not reviewing earlier
> rounds (other than v2 [1]); it's been difficult to find a block of
> time to do so...
I appreciate your time and comments.
>
> On Sun, Aug 30, 2015 at 3:10 PM, Michael Rappazzo wrote:
>> for_each_worktree iterates through each worktree and invokes a callback
>> function. The main worktree (if not bare) is always encountered first,
>> followed by worktrees created by `git worktree add`.
>
> Why does this iteration function specially filter out a bare main
> worktree? If it instead unconditionally vends the main worktree (bare
> or not), then the caller can make its own decision about what to do
> with it, thus empowering the caller, rather than imposing a (possibly)
> arbitrary restriction upon it.
>
> For instance, the "git worktree list" command may very well want to
> show the main worktree, even if bare (possibly controlled by a
> command-line option), annotated appropriately ("[bare]"). This may be
> exactly the sort of information a user wants to know, and by leaving
> the decision up to the caller, then the caller ("git worktree list" in
> this example) has the opportunity to act accordingly, whereas if
> for_each_worktree() filters out a bare main worktree unconditionally,
> then the caller ("git worktree list") will never be able to offer such
> an option.
I wasn't sure that a bare repo would be considered a worktree. I
don't think that it would be
a good idea to include it. In the same vein that I can't checkout a
branch in a bare repo, it
figure that it shouldn't be in the list.
>
>> If the callback function returns a non-zero value, iteration stops, and
>> the callback's return value is returned from the for_each_worktree
>> function.
>
> Stepping back a bit, is a for-each-foo()-style interface desirable?
> This sort of interface imposes a good deal of complexity on callers,
> demanding a callback function and callback data (cb_data), and is
> generally (at least in C) more difficult to reason about than other
> simpler interfaces. Is such complexity warranted?
>
> An alternate, much simpler interface would be to have a function, say
> get_worktrees(), return an array of 'worktree' structures to the
> caller, which the caller would iterate over (which is a common
> operation in C, thus easily reasoned about).
>
> The one benefit of a for-each-foo()-style interface is that it's
> possible to "exit early", thus avoiding the cost of interrogating
> meta-data for worktrees in which the caller is not interested,
> however, it seems unlikely that there will be so many worktrees linked
> to a repository for this early exit to translate into any real
> savings.
I am not opposed to making a simple function as you describe. I think David was
looking for a callback style function. I don't think it would be
terrible to keep the
callback and then also include the simple function to return the
struct array. I like
the memory management of the callback better than the struct array though.
>
>> Signed-off-by: Michael Rappazzo
>> ---
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index 430b51e..7b3cb96 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -26,6 +26,14 @@ static int show_only;
>> static int verbose;
>> static unsigned long expire;
>>
>> +/*
>> + * The signature for the callback function for the for_each_worktree()
>> + * function below. The memory pointed to by the callback arguments
>> + * is only guaranteed to be valid for the duration of a single
>> + * callback invocation.
>> + */
>> +typedef int each_worktree_fn(const char *path, const char *git_dir, void
>> *cb_data);
>
> In my review[1] of v2, I mentioned that (at some point) the "git
> worktree list" command might like to show additional information about
> the worktree other than just its location. Such information might
> include its tag, the checked out branch (or detached HEAD), whether
> it's locked (and the lock reason and whether the worktree is currently
> "online"), whether it can be pruned (and the prune reason).
>
> Commands such as "git worktree add" and "git checkout" need to know if
> a branch is already checked out in some (other) worktree, thus they
> also need the "branch" information.
>
> This need by clients for additional worktree meta-data suggests that
> the additional information ought to be encapsulated into a 'struct
> worktree', and that for_each_worktree() should vend a 'struct
> worktree' rather than vending merely the "git dir". (Alternately, the
> above-suggested get_worktrees() should return an array of 'struct
> worktree' items.)
>
> An important reason for making for_each_worktree() vend a 'struct
> worktree' is that it facilitates centralizing all the logic for
> determining and computing the extra worktree meta-data in one place,
> thus relieving callers of burden of having to compute the extra
> information themselves. (Junio alluded t
Re: [PATCH 1/2 v4] worktree: add 'for_each_worktree' function
I will reroll this series with adjustments as you suggested, and I
will add the extra tests for bare repos.
On Thu, Aug 13, 2015 at 3:23 PM, David Turner wrote:
> The scope of d can be smaller; move it down to the place I've marked
> below
I have adjusted the scoping. I misunderstood the meaning of some
comments from the v3 patch, but your statements have helped me
understand.
>
> strbuf_strip_suffix returns 1 if the suffix was stripped and 0
> otherwise, so there is no need for this strcmp.
Done.
>
> I'm a little worried about this path manipulation; it looks like the
> config setting core.bare is the actual thing to check? (Along with
> maybe the GIT_WORK_TREE environment var; not sure how that interacts
> with worktrees).
As Junio pointed out in a previous version of this patch, the
core.bare will always be 'true' since they share the config. He also
suggested that this could be the cause for concern. Therefore, I can
use core.bare to check if the main is a bare repo. I guess that with
the inclusion of tests using a bare repo, that will catch it if things
change in the future.
>
>> + }
>> +
>> + if (!main_is_bare) {
>> + strbuf_addstr(&worktree_path, main_path.buf);
>> +
>> + strbuf_addstr(&main_path, "/.git");
>> + strbuf_addstr(&worktree_git, main_path.buf);
>> +
>> + ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
>> + if (ret)
>> + goto done;
>> + }
>> + strbuf_addstr(&main_path, "/worktrees");
>
> Earlier, you said:
> strbuf_addstr(&main_path, common_dir);
> strbuf_strip_suffix(&main_path, "/.git");
>
> Now you are adding /worktrees. Doesn't this mean, in the non-bare case,
> that you'll be looking in $common_dir/worktrees instead of
> $common_dir/.git/worktrees ?
I read the gitdir file from the common dir.
>> + while ((d = readdir(dir)) != NULL) {
>> + if (!strcmp(d->d_name, ".") ||
>> !strcmp(d->d_name, ".."))
>> + continue;
>> +
>> + strbuf_reset(&worktree_git);
>> + strbuf_addf(&worktree_git, "%s/%s/gitdir",
>> main_path.buf, d->d_name);
>
> tioli: main_path never changes, so no need to keep chopping it off and
> adding it back on; just truncate worktree_git to main_path.len + 1 here
> and then add d->name.
I will try to clean it up a bit. I am not very experienced with C,
but I will do my best.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] worktree: add 'list' command
On Mon, Aug 10, 2015 at 10:55 PM, David Turner wrote:
> On Mon, 2015-08-10 at 16:53 -0400, Michael Rappazzo wrote:
>> + while ((d = readdir(dir)) != NULL) {
>
> I think it would be useful to break this loop out into a
> for_each_worktree function.
>
> While looking into per-worktree ref stuff, I have just noticed that git
> prune will delete objects that are only referenced in a different
> worktree's detached HEAD. To fix this, git prune will need to walk over
> each worktree, looking at that worktree's HEAD (and other per-worktree
> refs). It would be useful to be able to reuse some of this code for
> that task.
>
I agree, but I will save that for another round.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] worktree: add 'list' command
On Mon, Aug 10, 2015 at 6:10 PM, Junio C Hamano wrote:
> Michael Rappazzo writes:
>
>> +static int list(int ac, const char **av, const char *prefix)
>> +{
>> + int main_only = 0;
>> + struct option options[] = {
>> + OPT_BOOL(0, "main-only", &main_only, N_("only list the main
>> worktree")),
>> + OPT_END()
>> + };
>
> Hmm, main-only is still there?
Sorry, I missed that.
>
>> + int is_bare = is_bare_repository();
>
> Please do not introduce decl-after-stmt.
Since I reused this value below, I thought it would be acceptable.
>> + if (is_bare) {
>> + strbuf_addstr(&main_path, absolute_path(common_dir));
>
> Hmm, interesting.
>
> Because .git/config is shared, core.bare read from that tells us if
> the "main" one is bare, even if you start this command from one of
> its linked worktrees. So in that sense, this test of is_bare
> correctly tells if "main" one is a bare repository.
>
> But that by itself feels wrong. Doesn't the presense of a working
> tree mean that you should not get "is_bare==true" in such a case
> (i.e. your "main" one is bare, you are in a linked worktree of it
> that has the index and the working tree)?
Is is even correct for a bare repo to be included in the list? I
think that is part of what you are asking here.
>
>> + strbuf_strip_suffix(&main_path, "/.");
>
> In any case, what is that stripping of "/." about? Who is adding
> that extra trailing string?
>
> What I am getting at is (1) perhaps it shouldn't be adding that in
> the first place, and (2) if some other code is randomly adding "/."
> at the end, what guarantees you that you would need to strip it only
> once here---if the other code added that twice, don't you have to
> repeatedly remove "/." from the end?
In the case of a common dir, the value returned is just '.'. I wanted
to resolve that to the full path, so I called
`absolute_path(common_dir)`. Hence the trailing "/.". Similarly, in
the main repo, the common dir is just ".git", and I handled that by
using `get_git_work_tree()`.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] worktree: list operation
Withdrawn -- I staged but did not amend the final commit. I will
adjust and resend.
On Sat, Aug 8, 2015 at 4:34 PM, Michael Rappazzo wrote:
> 'git worktree list' will list the main worktree followed by any linked
> worktrees which were created using 'git worktree add'. The option
> '--main-only' will restrict the list to only the main worktree.
> ---
> Documentation/git-worktree.txt | 9 -
> builtin/worktree.c | 80
> ++
> t/t2027-worktree-list.sh | 68 +++
> 3 files changed, 150 insertions(+), 7 deletions(-)
> create mode 100755 t/t2027-worktree-list.sh
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 3387e2f..39b1330 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -11,6 +11,7 @@ SYNOPSIS
> [verse]
> 'git worktree add' [-f] [--detach] [-b ] []
> 'git worktree prune' [-n] [-v] [--expire ]
> +'git worktree list' [--primary]
>
> DESCRIPTION
> ---
> @@ -59,6 +60,10 @@ prune::
>
> Prune working tree information in $GIT_DIR/worktrees.
>
> +list::
> +
> +List the primary worktree followed by all of the linked worktrees.
> +
> OPTIONS
> ---
>
> @@ -86,6 +91,9 @@ OPTIONS
> With `prune`, do not remove anything; just report what it would
> remove.
>
> +--primary::
> + With `list`, only list the primary worktree.
> +
> -v::
> --verbose::
> With `prune`, report all removals.
> @@ -167,7 +175,6 @@ performed manually, such as:
> - `remove` to remove a linked worktree and its administrative files (and
>warn if the worktree is dirty)
> - `mv` to move or rename a worktree and update its administrative files
> -- `list` to list linked worktrees
> - `lock` to prevent automatic pruning of administrative files (for instance,
>for a worktree on a portable device)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 6a264ee..1ac1776 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -10,6 +10,7 @@
> static const char * const worktree_usage[] = {
> N_("git worktree add [] "),
> N_("git worktree prune []"),
> + N_("git worktree list []"),
> NULL
> };
>
> @@ -36,7 +37,7 @@ static int prune_worktree(const char *id, struct strbuf
> *reason)
> fd = open(git_path("worktrees/%s/gitdir", id), O_RDONLY);
> if (fd < 0) {
> strbuf_addf(reason, _("Removing worktrees/%s: unable to read
> gitdir file (%s)"),
> - id, strerror(errno));
> +id, strerror(errno));
> return 1;
> }
> len = st.st_size;
> @@ -59,7 +60,7 @@ static int prune_worktree(const char *id, struct strbuf
> *reason)
> * accessed since?
> */
> if (!stat(git_path("worktrees/%s/link", id), &st_link) &&
> - st_link.st_nlink > 1)
> +st_link.st_nlink > 1)
> return 0;
> if (st.st_mtime <= expire) {
> strbuf_addf(reason, _("Removing worktrees/%s: gitdir
> file points to non-existent location"), id);
> @@ -187,7 +188,7 @@ static int add_worktree(const char *path, const char
> **child_argv)
>
> name = worktree_basename(path, &len);
> strbuf_addstr(&sb_repo,
> - git_path("worktrees/%.*s", (int)(path + len - name),
> name));
> + git_path("worktrees/%.*s", (int)(path + len -
> name), name));
> len = sb_repo.len;
> if (safe_create_leading_directories_const(sb_repo.buf))
> die_errno(_("could not create leading directories of '%s'"),
> @@ -225,7 +226,7 @@ static int add_worktree(const char *path, const char
> **child_argv)
> strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
> write_file(sb.buf, 1, "%s\n", real_path(sb_git.buf));
> write_file(sb_git.buf, 1, "gitdir: %s/worktrees/%s\n",
> - real_path(get_git_common_dir()), name);
> + real_path(get_git_common_dir()), name);
> /*
> * This is to keep resolve_ref() happy. We need a valid HEAD
> * or is_git_directory() will reject the directory. Moreover, HEAD
> @@ -280,9 +281,9 @@ static int add(int ac, const char **av, const char
> *prefix)
> struct option options[] = {
> OPT__FORCE(&force, N_("checkout even if already
> checked out in other worktree")),
> OPT_STRING('b', NULL, &new_branch, N_("branch"),
> - N_("create a new branch")),
> + N_("create a new branch")),
> OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
> - N_("create or reset a branch")),
> + N_("create or reset a branch")),
> OPT_BOOL(0,
Re: git send-email Connection Closed
I believe that this is due to gmail not allowing the email. I think there are two ways to fix this: 1. Temporarily enable less secure apps for your gmail account while you send the email [see here](https://support.google.com/accounts/answer/6010255?hl=en). 2. Setup multi-factor authentication on your account and create an app specific password for git-send-email [see here](https://support.google.com/accounts/answer/185833?hl=en) Obviously the second method is more secure. I have had success with both of these techniques. On Wed, Jul 15, 2015 at 1:11 AM, Juston Li wrote: > Hello > > Recently, I have had trouble using git send-email to send a patchset. > After the confirmation to send the email I get the following: > Send this email? ([y]es|[n]o|[q]uit|[a]ll): y > [Net::SMTP::SSL] Connection closed at /usr/lib/git-core/git-send-email > line 1320. > fatal: 'send-email' appears to be a git command, but we were not > able to execute it. Maybe git-send-email is broken? > > This message first appeared when I tried to send a 19 commit patchset > via 'git send-email HEAD~19'. It also fails when I try to format-patch > and send the patchset separately via 'git send-email 0001...' > Oddly enough, it works when I send anything other than 19 commits > for example 'git send-email HEAD~1' or 'git send-email HEAD~20' > > I thought it was something with the gmail servers but I was able to send > the patchset by downgrading a couple perl packages > warning: downgrading package perl (5.22.0-1 => 5.20.2-1) > warning: downgrading package perl-net-ssleay (1.68-2 => 1.67-1) > > Note perl-net-ssleay depends on perl 5.22 so I can't isolate which > package but I can consistently get the failure with the newest packges > and it works fine with the downgraded packages. > > Running Arch Linux > git 2.4.5 > perl 5.22.0 > perl-authen-sasl 2.16 > perl-mime-tools 5.505 > perl-net-smtp-ssl 1.03 > > .gitconfig > [sendemail] > smtpEncryption = tls > smtpServer = smtp.gmail.com > smtpUser = juston.h...@gmail.com > smtpServerPort = 587 > > Regards > Juston > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: add config option to disable auto-merge
On Thu, Jun 18, 2015 at 4:43 AM David Aguilar wrote: > > On Wed, Jun 17, 2015 at 10:27:58PM -0400, Mike Rappazzo wrote: > > > > I feel that the auto-merge takes away the context of the changes. > > > > I use araxis merge as my mergetool of choice. I almost always immediately > > undo the auto-merge. After taking a moment to look at each file, I will > > then (usually) use the keyboard shortcut for auto-merge. > > > > It sure would be nice to "set-and-forget" a config variable to remove the > > annoyance of having to undo the auto-merge. I think that this config > > option is reasonable. Perhaps my documentation leaves something to be > > desired. I can take another stab at it. > > If this is the case then I would recommend making it more > granular. Just because Araxis' automerge is undesirable does > not mean that some other tools' automerge is as well. > e.g. the config variable could be "mergetool..automerge" > rather than the top-level "mergetool.automerge" variable. I don't necessarily think that araxis' automerge is bad, but I like to look at the before and after to understand the context of a conflict. I can't imagine that this is a quirk of araxis, but is probably something that exists for any auto-merging tool. The feature doesn't seem to be that widely supported among the other tooling. I only found the three to use such a feature. Since the automerge option is not available on every merge tool, it seems reasonable to use "mergetool..automerge" instead of "merge.automerge". > > > But, as Junio suggested, having a command-line flag to skip the > behavior might be a better first step. Something like, > "git mergetool --no-automerge". > > Most of Git's behavior that can be modified via configuration > can also be modified on the command-line, so exposing this > feature as a flag would probably be a good idea. This makes sense, and if this change is to go forward, I will implement the command line option. > > Even without a config variable, it can still be fire-and-forget > convenient by using a git alias to supply the flag. > > In lieu of any of these features, another option is that you can > override the default command by setting "mergetool.araxis.cmd", > and "git mergetool" will use your definition rather than its > built-in command. We left that escape hatch in for just this > purpose. I guess that if this patch does not go forward, I will have to use this workaround. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: add config option to disable auto-merge
On Wed, Jun 17, 2015 at 3:41 PM, Junio C Hamano wrote: > Michael Rappazzo writes: > >> For some mergetools, the current invocation of git mergetool will >> include an auto-merge flag. By default the flag is included, however if >> the git config option 'merge.automerge' is set to 'false', then that >> flag will now be omitted. > > ... and why is the "automerge" a bad thing that user would want to > avoid triggering under which condition? That description may not > have to be in the proposed log message, but it would help users when > they decide if they want to use the configuration to describe it in > the mergetool.automerge configuration. > > And depending on the answer to the above question, a configuration > variable may turn out be a bad mechanism to customize this (namely, > set-and-forget configuration variable is a bad match for a knob that > is more "per invocation" than "user taste"). > > Is this not about "automerge" but more about "always-show-UI because > I like GUI?" Then that may be a "user taste" thing that is a good > match for a configuration variable. I simply cannot tell from what > was in the message I am responding to. I feel that the auto-merge takes away the context of the changes. I use araxis merge as my mergetool of choice. I almost always immediately undo the auto-merge. After taking a moment to look at each file, I will then (usually) use the keyboard shortcut for auto-merge. It sure would be nice to "set-and-forget" a config variable to remove the annoyance of having to undo the auto-merge. I think that this config option is reasonable. Perhaps my documentation leaves something to be desired. I can take another stab at it. > >> -TEMPORARY FILES >> >> -`git mergetool` creates `*.orig` backup files while resolving merges. >> -These are safe to remove once a file has been merged and its >> -`git mergetool` session has completed. >> - >> +CONFIGURATION OPTIONS >> +- >> +mergetool.keepBackup:: >> + `git mergetool` creates `*.orig` backup files while resolving merges. >> + These are safe to remove once a file has been merged and its >> + `git mergetool` session has completed. >> ++ > > This is an unrelated change; I think it is a good change, though. > > I however suspect that we would not want to repeat the configuration > description in this file and instead mention these in "see also" > section referring the readers to git-config(1). > I felt that adding a separate header for a different config option was more appropriate, so I went with this. Pointing to the config.txt doc is probably better. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-rebase--interactive.sh: add config option for custom instruction format
On Fri, Jun 12, 2015 at 6:05 PM, Junio C Hamano wrote:
> But because you overwrite the $message variable you read from the
> original insn sheet (which uses the custom format) and compute $rest
> based on the default "%s" and store that in "$1.sq", lines in
> "$1.sq" do not know anything about the custom format, do they?
>
> And then they are injected to appropriate places in "$1.rearranged".
> Moved lines in the the rearranged result would end up written in the
> default "%s" format, no?
>
> That was the part that made me uneasy.
>
> I do not think that is a bug worth fixing, but I view it as a sign
> that fundamentally the autosquash and the idea of configurable
> format do not mesh well with each other.
My understanding of the rearrange_squash function is this:
There are two loops. The first loop collects the commits which should
be moved (squashed). The second loop re-constructs the instruction
list using the info from the first loop.
In the second loop, I changed it to recalculate the presented message
when the re-ordered commit is added:
+ if test -n "${format}"
+ then
+msg_content=$(git log -n 1 --format="${format}" ${squash})
That is the "$rest".
I have patched my locally installed `git-rebase--interactive` with
these changes, and I did see the proper rearrangement of commits with
the custom formatted message.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git-rebase--interactive.sh: add config option for custom instruction format
It only needs the '%s' for the autosquash when the todo/instruction
list order is determined. For this, in the rearrange_squash function,
it will re-calculate the message:
+ test -z "${format}" || message=$(git log -n 1
--format="%s" ${sha1})
Additionally, it may also rerun the log command when preparing the final list.
It is possible that this could be made more efficient by separating
the list arrangement from the list presentation. I can look into that
for a future patch.
I did add a test which uses the instructionFormat config, and then
interactively auto-squashes using both a 'squash! ' and a
'squash! '. in the commits.
On Fri, Jun 12, 2015 at 4:56 PM, Junio C Hamano wrote:
> Michael Rappazzo writes:
>
>> A config option 'rebase.instructionFormat' can override the
>> default 'oneline' format of the rebase instruction list.
>>
>> Since the list is parsed using the left, right or boundary mark plus
>> the sha1, they are prepended to the instruction format.
>>
>> Signed-off-by: Michael Rappazzo
>> ---
>> Documentation/git-rebase.txt | 7 +++
>> git-rebase--interactive.sh | 20 +---
>> t/t3415-rebase-autosquash.sh | 21 +
>> 3 files changed, 45 insertions(+), 3 deletions(-)
>
> Thanks, will replace.
>
> The autosquash part somehow makes me feel uneasy, though. The
> feature fundamentally has to have %s as the first thing in the
> format to work, but by making the format overridable, you are
> potentially breaking that feature, aren't you?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase--interactive.sh: add config option for custom instruction format
On Thu, Jun 11, 2015 at 9:40 AM, Johannes Schindelin
wrote:
> Hi Michael,
>
> On 2015-06-11 03:30, Michael Rappazzo wrote:
>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index dc3133f..6d14315 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -740,10 +740,19 @@ collapse_todo_ids() {
>> # "pick sha1 fixup!/squash! msg" appears in it so that the latter
>> # comes immediately after the former, and change "pick" to
>> # "fixup"/"squash".
>> +#
>> +# Note that if the config has specified a custom instruction format
>> +# each log message will be re-retrieved in order to normalize the
>> +# autosquash arrangement
>> rearrange_squash () {
>> # extract fixup!/squash! lines and resolve any referenced sha1's
>> - while read -r pick sha1 message
>> + while read -r pick sha1 todo_message
>> do
>> + message=${todo_message}
>
> Why not just leave the `read -r pick sha1 message` as-is and simply write
>
> # For "autosquash":
> test -z "$format" ||
> message="$(git log -n 1 --format="%s" $sha1)"
>
> here?
I did notice that I am not using '$todo_message' in the first loop at
all, so I will adjust it. In the second loop, I do use both the
original and the reformatted. I will apply your suggestion there if
applicable.
>
>> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
>> + git rebase --autosquash -i HEAD^^^ &&
>
> We usually write HEAD~3 instead of HEAD^^^...
>
Sure, I'll adjust it. I personally usually use up to 3 '^' and then
switch to '~' for > 3
>
> [The two test functions are] copied almost verbatim, except for the commit
> message. The code would be easier to maintain if it did not repeat so much
> code e.g. by refactoring out a function that takes the commit message as a
> parameter.
Makes sense. I'll implement that.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
I have since reworked this script to support the short hash in the
custom format as a special case:
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+no_format=$?
+if test ${no_format} -ne 0
+then
+ format="%H %s"
+elif test "${format:0:3}" != "%H " && test "${format:0:3}" != "%h "
+then
+ format="%H ${format}"
+fi
+# the 'rev-list .. | sed' requires %m to parse; the instruction
requires %H to parse
+git rev-list $merges_option --format="%m${format}" \
+ --reverse --left-right --topo-order \
I also use the $no_format variable later on in the autosquash
re-ordering, and have the tests passing. I want to add some new tests
on the custom format, and will send a new patch when that is complete.
On Tue, Jun 9, 2015 at 3:23 PM, Junio C Hamano wrote:
> Johannes Schindelin writes:
>
>> Besides, are you sure you don't want to substitute an empty
>> rebase.instructionFormat' by '%s'? I would have expected to read
>> ${format:-%s}` (note the colon), but then, this was Junio's
>> suggestion...
>
> That was me simply being sloppy myself, expecting people not to copy
> and paste literally without thinking. Thanks for noticing.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
I see your point, and I'll explore that avenue.
Personally, I like the idea that one could also use the short hash if
the custom instruction started with "%h ", but I see the value in
leaving the variable blank.
After running the tests with a custom format enabled, I did find that
autosquash doesn't work, so I am working to correct that.
On Tue, Jun 9, 2015 at 5:36 AM, Johannes Schindelin
wrote:
> Hi,
>
> On 2015-06-08 23:00, Michael Rappazzo wrote:
>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index dc3133f..b92375e 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -977,7 +977,9 @@ else
>> revisions=$onto...$orig_head
>> shortrevisions=$shorthead
>> fi
>> -git rev-list $merges_option --pretty=oneline --reverse --left-right
>> --topo-order \
>> +format=$(git config --get rebase.instructionFormat)
>> +# the 'rev-list .. | sed' requires %m to parse; the instruction
>> requires %H to parse
>> +git rev-list $merges_option --format="%m%H ${format-%s}" --reverse
>> --left-right --topo-order \
>
> These two lines are too long (longer than 80 columns)...
>
> Besides, are you sure you don't want to substitute an empty
> 'rebase.instructionFormat' by '%s'? I would have expected to read
> `${format:-%s}` (note the colon), but then, this was Junio's suggestion...
> Junio, what do you think, should we not rather substitute empty values by
> `%s` as if the config setting was unset?
>
>> $revisions ${restrict_revision+^$restrict_revision} | \
>> sed -n "s/^>//p" |
>> while read -r sha1 rest
>
> Ciao,
> Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase--interactive.sh: add config option for custom
On Mon, Jun 8, 2015 at 11:28 AM, Junio C Hamano wrote:
> Michael Rappazzo writes:
>
>> A config option 'rebase.instructionFormat' can override the
>> default 'oneline' format of the rebase instruction list.
>>
>> Since the list is parsed using the left, right or boundary mark plus
>> the sha1, they are prepended to the instruction format.
>>
>> Signed-off-by: Michael Rappazzo
>> ---
>
> Thanks. Roberto's gizmo seems to be working OK ;-)
Will see if the pull request -> email contraption will allow me to put
[patch v2] in there. I also need to see if it can make a [patch 0/1]
>
>> git-rebase--interactive.sh | 9 -
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index dc3133f..cc79b81 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -977,7 +977,14 @@ else
>> revisions=$onto...$orig_head
>> shortrevisions=$shorthead
>> fi
>> -git rev-list $merges_option --pretty=oneline --reverse --left-right
>> --topo-order \
>> +format=$(git config --get rebase.instructionFormat)
>> +if test -z "$format"
>> +then
>> + format="%s"
>
> Style. One indent level in our shell scripts is one HT, not a few spaces.
>
>> +fi
>> +# the 'rev-list .. | sed' requires %m to parse; the instruction requires %h
>> to parse
>> +format="%m%h ${format}"
>
> I think you want %H not %h here. If you check how the default
> "--pretty=online" is shown, you would see something like this:
>
> >1e9676ec0a771de06abca3009eb4bdc5a4ae3312 lockfile: replace ...
> >2024d3176536fd437b4c0a744161e96bc150a24e help.c: wrap wait-...
>
>> +git rev-list $merges_option --pretty="${format}" --reverse --left-right
>> --topo-order \
>> $revisions ${restrict_revision+^$restrict_revision} | \
>> sed -n "s/^>//p" |
I will make the changes from above, and resubmit a patch.
>
> This is optional, but I still wonder why the command line cannot be
> more like this, though:
>
> format=$(git config --get rebase.insnFormat)
> git log --format="%H ${format-%s}" --reverse --right-only
> --topo-order \
> $revisions ${restrict_revision+^$restrict_revision} |
> while read -r sha1 junk
> do
> ...
>
> That way we can optimize one "sed" process away.
>
> If this is a good idea, it needs to be a separate follow-up patch
> that changes "%m filtered by sed" to "use --right-only". I do not
> think such a change breaks anything, but I do not deal with complex
> histories myself, so...
>
As far as I can tell, the rev-list will return multiple lines when not
using 'oneline'. The 'sed -n' will join the lines back together. I
will take a look at moving it to 'git log' for a future change. I
have a huge codebase with tons of branches to experiment with.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: add author info to TODO list in git-rebase--interactive
I think the custom format makes sense. I took a first pass.
A config option 'rebase.interactive.todo-format' can override the
default 'oneline' format of the TODO list. Since the list is
parsed using the left, right or boundary mark plus the sha1, then if the
custom format does not start with those values, they will be
automatically added to the beginning of the custom format.
For example, if author information is desired for the TODO list, then
setting the config value to '[%an] %s' will actually result in the
format being '%m%h [%an] %s'
---
git-rebase--interactive.sh | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..e2d5ffc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,18 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+custom_format=$(git config --get rebase.interactive.todo-format)
+if test -z "$custom_format"
+then
+ custom_format="oneline"
+else
+ # the custom format MUST start with %m%h or %m%H
+ if test "${custom_format:0:5}" != '%m%h '
+ then
+ custom_format="%m%h ${custom_format}"
+ fi
+fi
+git rev-list $merges_option --pretty="${custom_format}" --reverse
--left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n "s/^>//p" |
while read -r sha1 rest
--
Is this closer to what you are looking for? I also tried changing the
'--left-right' to '--left-only', but that seemed to not produce any
results.
On Fri, Jun 5, 2015 at 3:39 PM, Eric Sunshine wrote:
> On Fri, Jun 5, 2015 at 3:35 PM, Junio C Hamano wrote:
>> Mike Rappazzo writes:
>>> I find that If I am doing a rebase with the intention to squash or
>>> re-order commits, it is helpful to know the commit author.
>>
>> There is not a fundamental reason why the remainder of the line
>> after the object name in the rebase insn sheet should not be
>> customizable, and I think your patch is a good first step to
>> identify where that customization should go.
>>
>> But that is a customization issue, not changing the default and the
>> only format used.
>
> The idea of being able to provide a custom format for insn sheet lines
> came up within the last year and was somewhat more well-developed and
> a bit more heavily discussed. I don't recall whether there was an
> accompanying patch, and I am unfortunately unable to locate the
> discussion in the archive.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Suggestion: add author info to TODO list in git-rebase--interactive
I find that If I am doing a rebase with the intention to squash or
re-order commits, it is helpful to know the commit author.
However, the alteration that I have made to git-rebase--interactive
may not be entirely correct. Here is the change:
---
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..ec44d41 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,7 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+git rev-list $merges_option --pretty="%m%h [%an] %x09%s" --reverse
--left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n "s/^>//p" |
while read -r sha1 rest
--
2.4.2
The problem, as I see it is that the original '--pretty=oneline' only
produces a single line of output (of course). However, the changed
version '--pretty="%m%h [%an] %x09%s"' produces multiple lines. The
command seems to ignore the unimportant lines, and the expected output
is put into the TODO list though.
Is there a better way of providing this information, or is this still
acceptable?
_Mike
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
