* [PATCH 0/3] push only submodules
From: Brandon Williams @ 2016-12-19 18:25 UTC (permalink / raw)
To: git; +Cc: sbeller, Brandon Williams
This series teaches 'git push' to be able to only push submodules while leaving
a superproject unpushed.
This is a desirable feature in a scenario where updates to the
superproject are handled automatically by some other means, perhaps a
code review tool. In this scenario a developer could make a change
which spans multiple submodules and then push their commits for code
review. Upon completion of the code review, their commits can be
accepted and applied to their respective submodules while the code
review tool can then automatically update the superproject to the most
recent SHA1 of each submodule. This would eliminate the merge conflicts
in the superproject that could occur if multiple people are contributing
to the same submodule.
Brandon Williams (3):
transport: refactor flag #defines to be more readable
submodules: add RECURSE_SUBMODULES_ONLY value
push: add option to push only submodules
builtin/push.c | 2 ++
submodule-config.c | 2 ++
submodule.h | 1 +
t/t5531-deep-submodule-push.sh | 21 +++++++++++++++++++++
transport.c | 15 +++++++++++----
transport.h | 31 ++++++++++++++++---------------
6 files changed, 53 insertions(+), 19 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* Re: [PATCH 0/5] git check-ref-format --stdin --report-errors
From: Junio C Hamano @ 2016-12-19 18:23 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Ian Jackson, git
In-Reply-To: <561c0338-66cd-f806-7b3b-b422f98a1564@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Especially given that the output is not especially machine-readable, it
> might be more consistent with other commands to call the new feature
> `--verbose` rather than `--report-errors`.
Don't we instead want to structure the output to be machine-readable
instead, given that check-ref-format is a very low level plumbing
command that is primarily meant for scriptors?
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2016, #02; Mon, 12)
From: Junio C Hamano @ 2016-12-19 18:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1612191145080.54750@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Otherwise I'd prefer to drop it---at this point, the series is merely
>> "just because we can", not "because we need it to further improve this
>> or that".
>
> Oh, I thought that this was meant as a starting point for anybody
> interested in playing with resumable clones or with easing server loads.
It started as such, but the effort stalled and nobody seems to be
taking it further. I do not know it is clear it would be a good
starting point at this point. Perhaps it is.
^ permalink raw reply
* Re: [PATCHv8 6/6] submodule: add absorb-git-dir function
From: Stefan Beller @ 2016-12-19 18:15 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Junio C Hamano, git@vger.kernel.org, Brandon Williams
In-Reply-To: <20161219053507.GA2335@duynguyen.vn.dektech.internal>
On Sun, Dec 18, 2016 at 9:35 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, Dec 12, 2016 at 11:04:35AM -0800, Stefan Beller wrote:
>> diff --git a/dir.c b/dir.c
>> index e0efd3c2c3..d872cc1570 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -2773,3 +2773,15 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
>> free(work_tree);
>> free(git_dir);
>> }
>> +
>> +/*
>> + * Migrate the git directory of the given path from old_git_dir to new_git_dir.
>> + */
>> +void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_git_dir)
>> +{
>> + if (rename(old_git_dir, new_git_dir) < 0)
>> + die_errno(_("could not migrate git directory from '%s' to '%s'"),
>> + old_git_dir, new_git_dir);
>> +
>> + connect_work_tree_and_git_dir(path, new_git_dir);
>
> Should we worry about recovering (e.g. maybe move new_git_dir back to
> old_git_dir) if this connect_work_tree_and_git_dir() fails?
What if the move back fails?
>
> Both write_file() and git_config_set_.. in this function may die(). In
> such a case the repo is in broken state and the user needs pretty good
> submodule understanding to recover from it, I think.
>
> Recovering is not easy (nor entirely safe) either, though I suppose if
> we keep original copies for modified files, then we could restore them
> after moving the directory back and pray the UNIX gods that all
> operations succeed.
There are different levels of brokenness available.
I just tried what happens if core.worktree is unset in a submodule
and that seems to not impact git operations (I only tested git-status
both in the superproject as well as in the submodule).
So I wonder why we set core.worktree at all here as it doesn't
seem to be needed.
Which means that the move of the git directory as well as the .git file
update to point at that moved directory are the important things
to get right.
So maybe:
1) rename the git dir or die
2) write the new gitlink
If that fails remove the .git file (if it exists partially or empty)
and undo 1) by calling rename again with swapped arguments
and then die
3) set core.worktree
If that fails, just warn the user
^ permalink raw reply
* Re: [PATCH] config.c: handle error case for fstat() calls
From: Junio C Hamano @ 2016-12-19 18:14 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, josharian
In-Reply-To: <20161219092155.20359-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> Will this fix the problem I'm replying to? I don't know. I found this
> while checking the code and it should be fixed regardless.
Yeah, from a cursory read, it is a step in the right direction to
check the return value of fstat().
Shouldn't the error-return path in the second hunk rollback the
lockfile to clean after itself? The existing "Oh, we cannot chmod
to match the original" check that comes immediately after shares the
same issue, so this is not a new problem, but making an existing one
worse.
> config.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index 83fdecb..4973256 100644
> --- a/config.c
> +++ b/config.c
> @@ -2194,7 +2194,12 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
> goto out_free;
> }
>
> - fstat(in_fd, &st);
> + if (fstat(in_fd, &st) == -1) {
> + error_errno(_("fstat on %s failed"), config_filename);
> + ret = CONFIG_INVALID_FILE;
> + goto out_free;
> + }
> +
> contents_sz = xsize_t(st.st_size);
> contents = xmmap_gently(NULL, contents_sz, PROT_READ,
> MAP_PRIVATE, in_fd, 0);
> @@ -2414,7 +2419,10 @@ int git_config_rename_section_in_file(const char *config_filename,
> goto unlock_and_out;
> }
>
> - fstat(fileno(config_file), &st);
> + if (fstat(fileno(config_file), &st) == -1) {
> + ret = error_errno(_("fstat on %s failed"), config_filename);
> + goto out;
> + }
>
> if (chmod(get_lock_file_path(lock), st.st_mode & 07777) < 0) {
> ret = error_errno("chmod on %s failed",
^ permalink raw reply
* Re: [PATCH v2] git-p4: Fix multi-path changelist empty commits
From: Luke Diamand @ 2016-12-19 17:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Users, George Vanburgh
In-Reply-To: <xmqqy3zbq05o.fsf@gitster.mtv.corp.google.com>
On 19 December 2016 at 17:49, Junio C Hamano <gitster@pobox.com> wrote:
> George Vanburgh <george@vanburgh.me> writes:
>
>> From: George Vanburgh <gvanburgh@bloomberg.net>
>>
>> When importing from multiple perforce paths - we may attempt to import
>> a changelist that contains files from two (or more) of these depot
>> paths. Currently, this results in multiple git commits - one
>> containing the changes, and the other(s) as empty commit(s). This
>> behavior was introduced in commit 1f90a64
>> ("git-p4: reduce number of server queries for fetches", 2015-12-19).
>>
>> Reproduction Steps:
>>
>> 1. Have a git repo cloned from a perforce repo using multiple depot
>> paths (e.g. //depot/foo and //depot/bar).
>> 2. Submit a single change to the perforce repo that makes changes in
>> both //depot/foo and //depot/bar.
>> 3. Run "git p4 sync" to sync the change from #2.
>>
>> Change is synced as multiple commits, one for each depot path that was
>> affected.
>>
>> Using a set, instead of a list inside p4ChangesForPaths() ensures that
>> each changelist is unique to the returned list, and therefore only a
>> single commit is generated for each changelist.
>>
>> Reported-by: James Farwell <jfarwell@vmware.com>
>> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
>> ---
>
> Thanks, George. Luke, can I add your "Reviewed-by:" here?
Yes, thanks.
Luke
^ permalink raw reply
* Re: [PATCH v2] git-p4: Fix multi-path changelist empty commits
From: Junio C Hamano @ 2016-12-19 17:49 UTC (permalink / raw)
To: Luke Diamand; +Cc: git, George Vanburgh
In-Reply-To: <010201590ed6ecaa-740c2532-827e-4f5a-af46-0f58d0722db6-000000@eu-west-1.amazonses.com>
George Vanburgh <george@vanburgh.me> writes:
> From: George Vanburgh <gvanburgh@bloomberg.net>
>
> When importing from multiple perforce paths - we may attempt to import
> a changelist that contains files from two (or more) of these depot
> paths. Currently, this results in multiple git commits - one
> containing the changes, and the other(s) as empty commit(s). This
> behavior was introduced in commit 1f90a64
> ("git-p4: reduce number of server queries for fetches", 2015-12-19).
>
> Reproduction Steps:
>
> 1. Have a git repo cloned from a perforce repo using multiple depot
> paths (e.g. //depot/foo and //depot/bar).
> 2. Submit a single change to the perforce repo that makes changes in
> both //depot/foo and //depot/bar.
> 3. Run "git p4 sync" to sync the change from #2.
>
> Change is synced as multiple commits, one for each depot path that was
> affected.
>
> Using a set, instead of a list inside p4ChangesForPaths() ensures that
> each changelist is unique to the returned list, and therefore only a
> single commit is generated for each changelist.
>
> Reported-by: James Farwell <jfarwell@vmware.com>
> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
> ---
Thanks, George. Luke, can I add your "Reviewed-by:" here?
> git-p4.py | 4 ++--
> t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index fd5ca52..6307bc8 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
> die("cannot use --changes-block-size with non-numeric revisions")
> block_size = None
>
> - changes = []
> + changes = set()
>
> # Retrieve changes a block at a time, to prevent running
> # into a MaxResults/MaxScanRows error from the server.
> @@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
>
> # Insert changes in chronological order
> for line in reversed(p4_read_pipe_lines(cmd)):
> - changes.append(int(line.split(" ")[1]))
> + changes.add(int(line.split(" ")[1]))
>
> if not block_size:
> break
> diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
> index 0730f18..4d93522 100755
> --- a/t/t9800-git-p4-basic.sh
> +++ b/t/t9800-git-p4-basic.sh
> @@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '
> )
> '
>
> +test_expect_success 'clone two dirs, each edited by submit, single git commit' '
> + (
> + cd "$cli" &&
> + echo sub1/f4 >sub1/f4 &&
> + p4 add sub1/f4 &&
> + echo sub2/f4 >sub2/f4 &&
> + p4 add sub2/f4 &&
> + p4 submit -d "sub1/f4 and sub2/f4"
> + ) &&
> + git p4 clone --dest="$git" //depot/sub1@all //depot/sub2@all &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git ls-files >lines &&
> + test_line_count = 4 lines &&
> + git log --oneline p4/master >lines &&
> + test_line_count = 5 lines
> + )
> +'
> +
> revision_ranges="2000/01/01,#head \
> 1,2080/01/01 \
> 2000/01/01,2080/01/01 \
> @@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '
> (
> cd "$git" &&
> git ls-files >lines &&
> - test_line_count = 6 lines
> + test_line_count = 8 lines
> )
> done
> '
>
> --
> https://github.com/git/git/pull/311
^ permalink raw reply
* Re: [PATCH] mailinfo.c: move side-effects outside of assert
From: Johannes Schindelin @ 2016-12-19 17:45 UTC (permalink / raw)
To: Kyle J. McKay; +Cc: Jonathan Tan, Junio C Hamano, Git mailing list
In-Reply-To: <900a55073f78a9f19daca67e468d334@3c843fe6ba8f3c586a21345a2783aa0>
Hi,
On Sat, 17 Dec 2016, Kyle J. McKay wrote:
> Since 6b4b013f18 (mailinfo: handle in-body header continuations,
> 2016-09-20, v2.11.0) mailinfo.c has contained new code with an
> assert of the form:
>
> assert(call_a_function(...))
>
> The function in question, check_header, has side effects. This
> means that when NDEBUG is defined during a release build the
> function call is omitted entirely, the side effects do not
> take place and tests (fortunately) start failing.
>
> Move the function call outside of the assert and assert on
> the result of the function call instead so that the code
> still works properly in a release build and passes the tests.
>
> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
ACK. I noticed this problem (and fixed it independently as a part of a
huge patch series I did not get around to submit yet) while trying to get
Git to build correctly with Visual C.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 14/34] sequencer (rebase -i): update refs after a successful rebase
From: Johannes Schindelin @ 2016-12-19 17:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqq8trfsmvr.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 16 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > An interactive rebase operates on a detached HEAD (to keep the reflog
> > of the original branch relatively clean), and updates the branch only
> > at the end.
> >
> > Now that the sequencer learns to perform interactive rebases, it also
> > needs to learn the trick to update the branch before removing the
> > directory containing the state of the interactive rebase.
> >
> > We introduce a new head_ref variable in a wider scope than necessary at
> > the moment, to allow for a later patch that prints out "Successfully
> > rebased and updated <ref>".
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > sequencer.c | 32 +++++++++++++++++++++++++++++++-
> > 1 file changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/sequencer.c b/sequencer.c
> > index a6625e765d..a4e9b326ba 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -100,6 +100,8 @@ static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
> > static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
> > static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head")
> > static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
> > +static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name")
> > +static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
> >
> > static inline int is_rebase_i(const struct replay_opts *opts)
> > {
> > @@ -1793,12 +1795,39 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
> > }
> >
> > if (is_rebase_i(opts)) {
> > - struct strbuf buf = STRBUF_INIT;
> > + struct strbuf head_ref = STRBUF_INIT, buf = STRBUF_INIT;
> >
> > /* Stopped in the middle, as planned? */
> > if (todo_list->current < todo_list->nr)
> > return 0;
> >
> > + if (read_oneliner(&head_ref, rebase_path_head_name(), 0) &&
> > + starts_with(head_ref.buf, "refs/")) {
> > + unsigned char head[20], orig[20];
> > +
> > + if (get_sha1("HEAD", head))
> > + return error(_("cannot read HEAD"));
> > + if (!read_oneliner(&buf, rebase_path_orig_head(), 0) ||
> > + get_sha1_hex(buf.buf, orig))
> > + return error(_("could not read orig-head"));
> > + strbuf_addf(&buf, "rebase -i (finish): %s onto ",
> > + head_ref.buf);
> > + if (!read_oneliner(&buf, rebase_path_onto(), 0))
> > + return error(_("could not read 'onto'"));
> > + if (update_ref(buf.buf, head_ref.buf, head, orig,
> > + REF_NODEREF, UPDATE_REFS_MSG_ON_ERR))
> > + return error(_("could not update %s"),
> > + head_ref.buf);
> > + strbuf_reset(&buf);
> > + strbuf_addf(&buf,
> > + "rebase -i (finish): returning to %s",
> > + head_ref.buf);
> > + if (create_symref("HEAD", head_ref.buf, buf.buf))
> > + return error(_("could not update HEAD to %s"),
> > + head_ref.buf);
>
> All of the above return error() calls leak head_ref.buf; in addition
> some leak buf.buf, too.
Thanks, fixed.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v3 02/16] dir: remove struct path_simplify
From: Stefan Beller @ 2016-12-19 17:33 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Duy Nguyen, Junio C Hamano
In-Reply-To: <1481670870-66754-3-git-send-email-bmwill@google.com>
On Tue, Dec 13, 2016 at 3:14 PM, Brandon Williams <bmwill@google.com> wrote:
> Teach simplify_away() and exclude_matches_pathspec() to handle struct
> pathspec directly, eliminating the need for the struct path_simplify.
>
> Also renamed the len parameter to pathlen in exclude_matches_pathspec()
> to match the parameter names used in simplify_away().
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
Looks good to me,
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v1] t0021: fix flaky test
From: Ramsay Jones @ 2016-12-19 17:24 UTC (permalink / raw)
To: larsxschneider, git; +Cc: peff, gitster
In-Reply-To: <20161218123748.72101-1-larsxschneider@gmail.com>
On 18/12/16 12:37, larsxschneider@gmail.com wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> t0021.15 creates files, adds them to the index, and commits them. All
> this usually happens in a test run within the same second and Git cannot
> know if the files have been changed between `add` and `commit`. Thus,
> Git has to run the clean filter in both operations. Sometimes these
> invocations spread over two different seconds and Git can infer that the
> files were not changed between `add` and `commit` based on their
> modification timestamp. The test would fail as it expects the filter
> invocation. Remove this expectation to make the test stable.
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>
> Notes:
> Base Commit: f8bf8f2a7b (next)
> Diff on Web: https://github.com/git/git/compare/f8bf8f2a7b...larsxschneider:9d88b66e03
> Checkout: git fetch https://github.com/larsxschneider/git filter-process/fix-flaky-test-v1 && git checkout 9d88b66e03
>
> t/t0021-conversion.sh | 17 +----------------
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index 6f16983d3e..161f560446 100755
> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh
> @@ -377,22 +377,7 @@ test_expect_success PERL 'required process filter should filter data' '
> EOF
> test_cmp_count expected.log rot13-filter.log &&
>
> - filter_git commit -m "test commit 2" &&
> - cat >expected.log <<-EOF &&
> - START
> - init handshake complete
> - IN: clean test.r $S [OK] -- OUT: $S . [OK]
> - IN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]
> - IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
> - IN: clean testsubdir/test3 '\''sq'\'',\$x=.r $S3 [OK] -- OUT: $S3 . [OK]
> - IN: clean test.r $S [OK] -- OUT: $S . [OK]
> - IN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]
> - IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
> - IN: clean testsubdir/test3 '\''sq'\'',\$x=.r $S3 [OK] -- OUT: $S3 . [OK]
> - STOP
> - EOF
> - test_cmp_count expected.log rot13-filter.log &&
> -
> + git commit -m "test commit 2" &&
> rm -f test2.r "testsubdir/test3 '\''sq'\'',\$x=.r" &&
>
> filter_git checkout --quiet --no-progress . &&
>
I applied this to the pu branch and ran the test by hand
48 times in a row without failure. (the most trials without
error beforehand was 24).
Thanks.
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH v2 11/34] sequencer (rebase -i): remove CHERRY_PICK_HEAD when no longer needed
From: Johannes Schindelin @ 2016-12-19 17:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqd1grsn53.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 16 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > The scripted version of the interactive rebase already does that.
>
> Sensible. I was wondering why this wasn't there while reviewing
> 10/34, comparing the two (this is not a suggestion to squash this
> into the previous step).
It's a bit of a historical wart, as I discovered test breakages only long
after implementing the fixup/squash commands (as you may have guessed, I
implemented the commands one after another, fixing things as discovered by
the test suite; it took something like a month until I got the
rebase--helper based rebase -i to pass t3404).
In the end, I decided not to squash this into 10/34 because it seemed to
be a significant enough change to merit its own commit.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Jeff King @ 2016-12-19 17:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1612191758290.54750@virtualbox>
On Mon, Dec 19, 2016 at 05:59:06PM +0100, Johannes Schindelin wrote:
> > > > + sprintf((char *)p, "%d", ++count);
> > >
> > > Do we know the area pointed at p (which is inside buf) long enough
> > > not to overflow? If the original were 9 and you incremented to get
> > > 10, you would need one extra byte.
> >
> > Even if it is enough, I'd ask to please use xsnprintf(). In the off
> > chance that there's a programming error, we'd get a nice die("BUG")
> > instead of a buffer overflow (and it makes the code base easier to audit
> > for other overflows).
>
> I ended up with more verbose, easier-to-read code that does not try to do
> things in-place, in favor of being slightly more wasteful with strbufs.
Great. I agree that should make the whole thing way more readable.
-Peff
^ permalink raw reply
* Re: [PATCH] fix pushing to //server/share/dir paths on Windows
From: Johannes Schindelin @ 2016-12-19 17:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, Git Mailing List
In-Reply-To: <2ff2613c-47da-a780-5d38-93e16cb16328@kdbg.org>
Hi Hannes,
On Tue, 13 Dec 2016, Johannes Sixt wrote:
> Dscho, it looks like this could fix the original report at
> https://github.com/git-for-windows/git/issues/979
Yes, thank you so much for fixing it!
I think your v2 is safe enough to cherry-pick directly into Git for
Windows' `master`.
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-12-19 17:14 UTC (permalink / raw)
To: Jeff King
Cc: Stefan Beller, git, Junio C Hamano, Kevin Daudt,
Dennis Kaarsemaker
In-Reply-To: <20161215190351.as76panrcz5rgibj@sigill.intra.peff.net>
Hi Peff,
On Thu, 15 Dec 2016, Jeff King wrote:
> On Tue, Dec 13, 2016 at 04:30:01PM +0100, Johannes Schindelin wrote:
>
> > + else {
> > + unsigned char head[20];
> > + struct commit *head_commit;
> > + const char *head_message, *body;
> > +
> > + if (get_sha1("HEAD", head))
> > + return error(_("need a HEAD to fixup"));
> > + if (!(head_commit = lookup_commit_reference(head)))
> > + return error(_("could not read HEAD"));
> > + if (!(head_message = get_commit_buffer(head_commit, NULL)))
> > + return error(_("could not read HEAD's commit message"));
>
> This get_commit_buffer() may allocate a fresh buffer...
>
> > + body = strstr(head_message, "\n\n");
> > + if (!body)
> > + body = "";
> > + else
> > + body = skip_blank_lines(body + 2);
> > + if (write_message(body, strlen(body),
> > + rebase_path_fixup_msg(), 0))
> > + return error(_("cannot write '%s'"),
> > + rebase_path_fixup_msg());
>
> ...and then this return leaks the result (the other code path hits
> unuse_commit_buffer(), and is fine).
Good point.
I found another leaked commit buffer in make_patch() and fixed it, too.
> This leak was noticed by Coverity. It has a _ton_ of false positives
> across the whole project, but it sends out a mail with new ones every
> few days, which is usually short enough that I can process it in 30
> seconds or so.
Yeah, I get these mails now, thanks to Stephan adding me in response to
some issues I introduced with the builtin difftool (and hence I did not
get the warnings when I introduced the problems with sequencer-i).
Ciao,
Dscho
^ permalink raw reply
* Re: Bug report: $program_name in error message
From: Stefan Beller @ 2016-12-19 17:07 UTC (permalink / raw)
To: Josh Bleecher Snyder, vascomalmeida; +Cc: git@vger.kernel.org
In-Reply-To: <CAFAcib9-rUSqyBRpauw3pTf9OPTKLYNf7bdh2gyykBNtJTZKGg@mail.gmail.com>
+ Vasco Almeida, who authored d323c6b641,
(i18n: git-sh-setup.sh: mark strings for translation, 2016-06-17)
On Sun, Dec 18, 2016 at 1:44 PM, Josh Bleecher Snyder
<josharian@gmail.com> wrote:
>>> To reproduce, run 'git submodule' from within a bare repo. Result:
>>>
>>> $ git submodule
>>> fatal: $program_name cannot be used without a working tree.
>>>
>>> Looks like the intent was for $program_name to be interpolated.
>>
>> Which version of git do you use?
>
> $ git version
> git version 2.11.0
>
>
>>> As an aside, I sent a message a few days ago about a segfault when
>>> working with a filesystem with direct_io on, but it appears not to
>>> have made it to the archives on marc.info. Am I perhaps still
>>> greylisted?
>>
>> Both emails show up in my mailbox (subscribed to the mailing list),
>> so I think that just nobody answered your first email as the answer
>> may be non trivial.
>
> Thanks for the confirmation.
>
> -josh
^ permalink raw reply
* Re: [PATCH v2 09/34] sequencer (rebase -i): write an author-script file
From: Johannes Schindelin @ 2016-12-19 17:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqd1gtuivc.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 15 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > + strbuf_addstr(&buf, "GIT_AUTHOR_NAME='");
> > + while (*message && *message != '\n' && *message != '\r')
> > + if (skip_prefix(message, " <", &message))
> > + break;
> > + else if (*message != '\'')
> > + strbuf_addch(&buf, *(message++));
> > + else
> > + strbuf_addf(&buf, "'\\\\%c'", *(message++));
> > + strbuf_addstr(&buf, "'\nGIT_AUTHOR_EMAIL='");
> > + while (*message && *message != '\n' && *message != '\r')
> > + if (skip_prefix(message, "> ", &message))
> > + break;
> > + else if (*message != '\'')
> > + strbuf_addch(&buf, *(message++));
> > + else
> > + strbuf_addf(&buf, "'\\\\%c'", *(message++));
>
> Aren't these reading from an in-core commit object?
>
> If so, it should use split_ident_line() for consistency with other
> parts of the system to do this parsing. We should also already have
> a helper for simple shell-quoting in quote.c and you would want to
> use that instead of open coding like this.
We keep coming back to the same argument. You want this quoting/dequoting
to be turned into a full-fledged parser. And I keep pointing out that the
code here does not *need* to parse but only construct an environment
block.
Hopefully the next iteration that integrates Peff's suggestions will find
more of your approval.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Tweak help auto-correct phrasing.
From: Marc Branchaud @ 2016-12-19 17:01 UTC (permalink / raw)
To: git; +Cc: Kaartic Sivaraam, Chris Packham, Alex Riesen
In-Reply-To: <CAFOYHZDnpzdYq9j4-xGSdKZQX9deLBpZZhz209qV7cCtq537SA@mail.gmail.com>
Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
On 2016-12-18 07:48 PM, Chris Packham wrote:
>
> This feature already exists (although it's not interactive). See
> help.autoCorrect in the git-config man page. "git config
> help.autoCorrect -1" should to the trick.
Awesome, I was unaware of this feature. Thanks!
I found the message it prints a bit awkward, so here's a patch to fix it up.
Instead of:
WARNING: You called a Git command named 'lgo', which does not exist.
Continuing under the assumption that you meant 'log'
in 1.5 seconds automatically...
it's now:
WARNING: You called a Git command named 'lgo', which does not exist.
Continuing in 1.5 seconds under the assumption that you meant 'log'.
M.
help.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/help.c b/help.c
index 53e2a67e00..55350c0673 100644
--- a/help.c
+++ b/help.c
@@ -381,12 +381,18 @@ const char *help_unknown_cmd(const char *cmd)
clean_cmdnames(&main_cmds);
fprintf_ln(stderr,
_("WARNING: You called a Git command named '%s', "
- "which does not exist.\n"
- "Continuing under the assumption that you meant '%s'"),
- cmd, assumed);
- if (autocorrect > 0) {
- fprintf_ln(stderr, _("in %0.1f seconds automatically..."),
- (float)autocorrect/10.0);
+ "which does not exist."),
+ cmd);
+ if (autocorrect < 0)
+ fprintf_ln(stderr,
+ _("Continuing under the assumption that "
+ "you meant '%s'."),
+ assumed);
+ else {
+ fprintf_ln(stderr,
+ _("Continuing in %0.1f seconds under the "
+ "assumption that you meant '%s'."),
+ (float)autocorrect/10.0, assumed);
sleep_millisec(autocorrect * 100);
}
return assumed;
--
2.11.0.dirty
^ permalink raw reply related
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-12-19 16:59 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <20161215185630.xxeimack63wqwv2e@sigill.intra.peff.net>
Hi Peff,
On Thu, 15 Dec 2016, Jeff King wrote:
> On Thu, Dec 15, 2016 at 10:42:53AM -0800, Junio C Hamano wrote:
>
> > > + sprintf((char *)p, "%d", ++count);
> >
> > Do we know the area pointed at p (which is inside buf) long enough
> > not to overflow? If the original were 9 and you incremented to get
> > 10, you would need one extra byte.
>
> Even if it is enough, I'd ask to please use xsnprintf(). In the off
> chance that there's a programming error, we'd get a nice die("BUG")
> instead of a buffer overflow (and it makes the code base easier to audit
> for other overflows).
I ended up with more verbose, easier-to-read code that does not try to do
things in-place, in favor of being slightly more wasteful with strbufs.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
From: Johannes Schindelin @ 2016-12-19 16:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqlgvhuj82.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 15 Dec 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/sequencer.c b/sequencer.c
> > index f6e20b142a..271c21581d 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -45,6 +45,35 @@ static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
> > */
> > static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done")
> > /*
> > + * The commit message that is planned to be used for any changes that
> > + * need to be committed following a user interaction.
> > + */
> > +static GIT_PATH_FUNC(rebase_path_message, "rebase-merge/message")
> > +/*
> > + * The file into which is accumulated the suggested commit message for
> > + * squash/fixup commands. When the first of a series of squash/fixups
>
> The same comment as 03/34 applies here, regarding blank line to
> separate logical unit.
Same rationale here: the path functions are one big continuous block, with
comments obviously applying to the immediately following line only.
> > +static int update_squash_messages(enum todo_command command,
> > + struct commit *commit, struct replay_opts *opts)
> > +{
> > + struct strbuf buf = STRBUF_INIT;
> > + int count, res;
> > + const char *message, *body;
> > +
> > + if (file_exists(rebase_path_squash_msg())) {
> > + char *p, *p2;
> > +
> > + if (strbuf_read_file(&buf, rebase_path_squash_msg(), 2048) <= 0)
> > + return error(_("could not read '%s'"),
> > + rebase_path_squash_msg());
> > +
> > + if (buf.buf[0] != comment_line_char ||
> > + !skip_prefix(buf.buf + 1, " This is a combination of ",
> > + (const char **)&p))
> > + return error(_("unexpected 1st line of squash message:"
> > + "\n\n\t%.*s"),
> > + (int)(strchrnul(buf.buf, '\n') - buf.buf),
> > + buf.buf);
> > + count = strtol(p, &p2, 10);
> > +
> > + if (count < 1 || *p2 != ' ')
> > + return error(_("invalid 1st line of squash message:\n"
> > + "\n\t%.*s"),
> > + (int)(strchrnul(buf.buf, '\n') - buf.buf),
> > + buf.buf);
> > +
> > + sprintf((char *)p, "%d", ++count);
>
> Do we know the area pointed at p (which is inside buf) long enough
> not to overflow?
Yes, we know that: p2 points to the byte after the parsed number, and said
byte is a space (ASCII 0x20), as verified by the if() above.
> If the original were 9 and you incremented to get 10, you would need one
> extra byte.
Exactly. That extra byte (if needed) is 0x20, as verified above, and can
be overwritten.
If that extra byte (to which p2 points) is *not* overwritten, i.e. if the
new count requires the same amount of space in decimal representation as
the previous count, it is now NUL, as tested here:
> > + if (!*p2)
> > + *p2 = ' ';
> > + else {
> > + *(++p2) = 'c';
>
> p2 points into buf; do we know this increment does not step beyond
> its end? What is the meaning of a letter 'c' here (I do not see a
> corresponding one in the scripted update_squash_messages)?
This clause is entered only when the space needed by the previous count
was not sufficient to hold the new count, at which point we know that not
only the space after the old count was overwritten, but also the 'c' of
the "commits" string.
Therefore, this clause reinstates the 'c' and inserts the space, so that
everything is groovy again:
> > + strbuf_insert(&buf, p2 - buf.buf, " ", 1);
> > + }
Having said that, I just realized that this code was only safe as long as
the squash messages were not localized.
I changed the code to imitate more closely what the shell script does. It
made the code a little more verbose, but it should work better as a
consequence, and I am pretty certain you will find it easier to verify
that it is correct.
> > + }
> > + else {
>
> Style: "} else {" (I won't repeat this, as it will become too noisy).
It is not necessary to repeat this, either, as I took the first such
comment as a strong hint to look at the entire patch series and fix it
appropriately.
>
> > + unsigned char head[20];
> > + struct commit *head_commit;
> > + const char *head_message, *body;
> > +
> > + if (get_sha1("HEAD", head))
> > + return error(_("need a HEAD to fixup"));
> > + if (!(head_commit = lookup_commit_reference(head)))
> > + return error(_("could not read HEAD"));
> > + if (!(head_message = get_commit_buffer(head_commit, NULL)))
> > + return error(_("could not read HEAD's commit message"));
> > +
> > + body = strstr(head_message, "\n\n");
> > + if (!body)
> > + body = "";
> > + else
> > + body = skip_blank_lines(body + 2);
>
> I think I saw you used a helper function find_commit_subject() to do
> the above in an earlier patch for "edit" in this series. Would it
> make this part (and another one for "commit" we have after this
> if/else) shorter?
Right, and by using the helper function, I also fixed a bug handling funny
commit objects that had more than one empty line separating header from
the message.
Of course, there is a third location in sequencer.c (predating my patches)
that uses the very same idiom. Yet another patch added to this growing
patch series...
> > static int do_pick_commit(enum todo_command command, struct commit *commit,
> > - struct replay_opts *opts)
> > + struct replay_opts *opts, int final_fixup)
> > {
> > + int edit = opts->edit, cleanup_commit_message = 0;
> > + const char *msg_file = edit ? NULL : git_path_merge_msg();
> > unsigned char head[20];
> > struct commit *base, *next, *parent;
> > const char *base_label, *next_label;
> > struct commit_message msg = { NULL, NULL, NULL, NULL };
> > struct strbuf msgbuf = STRBUF_INIT;
> > - int res, unborn = 0, allow;
> > + int res, unborn = 0, amend = 0, allow;
> >
> > if (opts->no_commit) {
> > /*
> > @@ -749,7 +885,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> > else
> > parent = commit->parents->item;
> >
> > - if (opts->allow_ff &&
> > + if (opts->allow_ff && !is_fixup(command) &&
> > ((parent && !hashcmp(parent->object.oid.hash, head)) ||
> > (!parent && unborn)))
> > return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
> > @@ -813,6 +949,28 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> > }
> > }
> >
> > + if (is_fixup(command)) {
> > + if (update_squash_messages(command, commit, opts))
> > + return -1;
> > + amend = 1;
> > + if (!final_fixup)
> > + msg_file = rebase_path_squash_msg();
> > + else if (file_exists(rebase_path_fixup_msg())) {
> > + cleanup_commit_message = 1;
> > + msg_file = rebase_path_fixup_msg();
> > + }
> > + else {
> > + const char *dest = git_path("SQUASH_MSG");
> > + unlink(dest);
> > + if (copy_file(dest, rebase_path_squash_msg(), 0666))
> > + return error(_("could not rename '%s' to '%s'"),
> > + rebase_path_squash_msg(), dest);
>
> Perhaps an error from unlink(dest) before copy_file() should also
> result in an error return?
No, because the most likely cause for that `unlink()` to fail is that the
destination does not exist, and it is fine if it does not exist yet.
No worries, copy_file() will fail if we could not remove any existing file
and we still error out in that case.
> > + unlink(git_path("MERGE_MSG"));
>
> Errors from this and other unlink() that emulates "rm -f" were
> unchecked in the scripted original, so not checking for errors is
> not a regression. I would check for an error if I were writing
> this, however, because I know I would forget updating these after I
> am done with the series.
The problem is that these files do not necessarily exist. We only unlink()
them to make sure that they do not exist afterwards.
In any case, I am still more interested in a faithful translation than
already starting to improve the code at this stage.
> > @@ -1475,6 +1660,21 @@ static int do_exec(const char *command_line)
> > return status;
> > }
> >
> > +static int is_final_fixup(struct todo_list *todo_list)
> > +{
> > + int i = todo_list->current;
> > +
> > + if (!is_fixup(todo_list->items[i].command))
> > + return 0;
> > +
> > + while (++i < todo_list->nr)
> > + if (is_fixup(todo_list->items[i].command))
> > + return 0;
> > + else if (todo_list->items[i].command < TODO_NOOP)
> > + break;
>
> What follows NOOP are comment and "drop" which is another comment in
> disguise, so this one is excluding all the no-op commands in various
> shapes, which makes sense but is clear only to a reader who bothered
> to go back to "enum todo_command" and checked that fact. If a check
> for "is it one of the no-op commands?" appears only here, a single
> liner comment may be sufficient (but necessary) to help readers.
> Otherwise a single-liner helper function (similar to is_fixup() you
> have) with a descriptive name would be better than a single liner
> comment.
True. I introduced is_noop().
Ciao,
Dscho
^ permalink raw reply
* [PATCH 03/13] gitk: use a type "tags" to indicate abbreviated tags
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
This replaces the functionality of the old `singletag` variable.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/gitk b/gitk
index 7c830c3..502f0aa 100755
--- a/gitk
+++ b/gitk
@@ -6561,7 +6561,6 @@ proc drawtags {id x xt y1} {
set marks {}
set types {}
- set singletag 0
set maxtags 3
set maxtagpct 25
set maxwidth [expr {[graph_pane_width] * $maxtagpct / 100}]
@@ -6574,8 +6573,7 @@ proc drawtags {id x xt y1} {
if {$ntags > $maxtags ||
[totalwidth $tags mainfont $extra] > $maxwidth} {
# show just a single "n tags..." tag
- set singletag 1
- lappend types tag
+ lappend types tags
if {$ntags == 1} {
lappend marks "tag..."
} else {
@@ -6626,13 +6624,13 @@ proc drawtags {id x xt y1} {
set xl [expr {$x + $delta}]
set xr [expr {$x + $delta + $wid + $lthickness}]
set font mainfont
- if {$type eq "tag"} {
+ if {$type eq "tag" || $type eq "tags"} {
# draw a tag
set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
$xr $yt $xr $yb $xl $yb $x [expr {$yb - $delta}] \
-width 1 -outline $tagoutlinecolor -fill $tagbgcolor \
-tags tag.$id]
- if {$singletag} {
+ if {$type eq "tags"} {
set tagclick [list showtags $id 1]
} else {
set tagclick [list showtag $tag_quoted 1]
@@ -6663,7 +6661,7 @@ proc drawtags {id x xt y1} {
}
set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
-font $font -tags [list tag.$id text]]
- if {$type eq "tag"} {
+ if {$type eq "tag" || $type eq "tags"} {
$canv bind $t <1> $tagclick
} elseif {$type eq "head"} {
$canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
--
2.9.3
^ permalink raw reply related
* [PATCH 13/13] gitk: change the default colors for remote-tracking references
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
Instead of coloring the reference name part of remote-tracking
references the same bright green as used for local branches, change it
to a subtler brown. Change the remote name color to go better with the
brown.
I chose these colors because
* Remote-tracking references are less important than local branches, so
it is appropriate that their coloring be subtler.
* Brown isn't used elsewhere.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gitk b/gitk
index cb5c715..be97640 100755
--- a/gitk
+++ b/gitk
@@ -12363,8 +12363,8 @@ set markbgcolor "#e0e0ff"
set headbgcolor "#00ff00"
set headfgcolor black
set headoutlinecolor black
-set remotebgcolor #ffddaa
-set remoterefbgcolor #00ff00
+set remotebgcolor #d9d4b2
+set remoterefbgcolor #c1a677
set otherrefbgcolor #ddddff
set tagbgcolor yellow
set tagfgcolor black
--
2.9.3
^ permalink raw reply related
* [PATCH 11/13] gitk: introduce a constant otherrefbgcolor
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
This is the value used for references other than tags, heads, and
remote-tracking references (e.g., `refs/stash`).
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index 44f4d70..0bd4aa5 100755
--- a/gitk
+++ b/gitk
@@ -6573,7 +6573,8 @@ proc drawtags {id x xt y1} {
global idtags idheads idotherrefs mainhead
global linespc lthickness
global canv rowtextx curview fgcolor bgcolor ctxbut
- global headbgcolor headfgcolor headoutlinecolor remotebgcolor
+ global headbgcolor headfgcolor headoutlinecolor
+ global remotebgcolor otherrefbgcolor
global tagbgcolor tagfgcolor tagoutlinecolor
global reflinecolor
@@ -6671,7 +6672,7 @@ proc drawtags {id x xt y1} {
} elseif {$type eq "head" || $type eq "remote"} {
set col $headbgcolor
} else {
- set col "#ddddff"
+ set col $otherrefbgcolor
}
set xl [expr {$xl - $delta/2}]
$canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
@@ -12361,6 +12362,7 @@ set headbgcolor "#00ff00"
set headfgcolor black
set headoutlinecolor black
set remotebgcolor #ffddaa
+set otherrefbgcolor #ddddff
set tagbgcolor yellow
set tagfgcolor black
set tagoutlinecolor black
@@ -12418,7 +12420,8 @@ set config_variables {
bgcolor fgcolor uifgcolor uifgdisabledcolor colors diffcolors mergecolors
markbgcolor diffcontext selectbgcolor foundbgcolor currentsearchhitbgcolor
extdifftool perfile_attrs headbgcolor headfgcolor headoutlinecolor
- remotebgcolor tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
+ remotebgcolor otherrefbgcolor
+ tagbgcolor tagfgcolor tagoutlinecolor reflinecolor
filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor
linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor
indexcirclecolor circlecolors linkfgcolor circleoutlinecolor
--
2.9.3
^ permalink raw reply related
* [PATCH 10/13] gitk: use type "remote" for remote-tracking references
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
This is clearer, and also lets us avoid calling `remotereftext` a second
time for normal branches.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index d22ce5f..44f4d70 100755
--- a/gitk
+++ b/gitk
@@ -6613,9 +6613,11 @@ proc drawtags {id x xt y1} {
if {$head eq $mainhead} {
lappend types mainhead
lappend wvals [font measure mainfontbold $head]
+ } elseif {[remotereftext $head text remoteprefix]} {
+ lappend types remote
+ lappend wvals [font measure mainfont $text]
} else {
lappend types head
- remotereftext $head text remoteprefix
lappend wvals [font measure mainfont $text]
}
lappend marks $head
@@ -6666,7 +6668,7 @@ proc drawtags {id x xt y1} {
if {$type eq "mainhead"} {
set col $headbgcolor
set font mainfontbold
- } elseif {$type eq "head"} {
+ } elseif {$type eq "head" || $type eq "remote"} {
set col $headbgcolor
} else {
set col "#ddddff"
@@ -6674,7 +6676,8 @@ 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 {[remotereftext $tag text remoteprefix]} {
+ if {$type eq "remote"} {
+ remotereftext $tag text remoteprefix
set rwid [font measure mainfont $remoteprefix]
set xi [expr {$x + 1}]
set yti [expr {$yt + 1}]
--
2.9.3
^ permalink raw reply related
* [PATCH 09/13] gitk: shorten labels displayed for modern remote-tracking refs
From: Michael Haggerty @ 2016-12-19 16:45 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Michael Haggerty
In-Reply-To: <cover.1482164633.git.mhagger@alum.mit.edu>
If a reference has the form
refs/remotes/origin/foo
, then use the label
origin/foo
^^^^^^^
, where the indicated part is displayed in `$remotebgcolor`. The old
code would have displayed such a reference as
remotes/origin/foo
^^^^^^^^^^^^^^^
, which wastes horizontal space displaying `remote/` for every remote
reference. However, if a remote-tracking reference has only two slashes,
like
refs/remotes/baz
, render the label the same as before, namely
remotes/baz
^^^^^^^^
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
gitk | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index c146a15..d22ce5f 100755
--- a/gitk
+++ b/gitk
@@ -6558,7 +6558,9 @@ proc remotereftext {head textName prefixName} {
upvar $textName text
upvar $prefixName prefix
- if {[regexp {^((remotes/([^/]+/|)).*)} $head match text prefix]} {
+ if {[regexp {^remotes/(([^/]+/).*)} $head match text prefix]} {
+ return 1
+ } elseif {[regexp {^((remotes/).*)} $head match text prefix]} {
return 1
} else {
set text $head
@@ -6613,7 +6615,8 @@ proc drawtags {id x xt y1} {
lappend wvals [font measure mainfontbold $head]
} else {
lappend types head
- lappend wvals [font measure mainfont $head]
+ remotereftext $head text remoteprefix
+ lappend wvals [font measure mainfont $text]
}
lappend marks $head
}
@@ -6644,6 +6647,7 @@ proc drawtags {id x xt y1} {
set xl [expr {$x + $delta}]
set xr [expr {$x + $delta + $wid + $lthickness}]
set font mainfont
+ set text $tag
if {$type eq "tag" || $type eq "tags"} {
# draw a tag
set t [$canv create polygon $x [expr {$yt + $delta}] $xl $yt \
@@ -6679,7 +6683,7 @@ proc drawtags {id x xt y1} {
-width 0 -fill $remotebgcolor -tags tag.$id
}
}
- set t [$canv create text $xl $y1 -anchor w -text $tag -fill $headfgcolor \
+ set t [$canv create text $xl $y1 -anchor w -text $text -fill $headfgcolor \
-font $font -tags [list tag.$id text]]
if {$type eq "tag" || $type eq "tags"} {
$canv bind $t <1> $tagclick
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox