Git development
 help / color / mirror / Atom feed
* Re: [PATCHv2 4/4] submodule: add embed-git-dir function
From: Stefan Beller @ 2016-11-30 20:48 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Brandon Williams, Junio C Hamano, Git Mailing List,
	Jonathan Nieder, Jens Lehmann, Heiko Voigt
In-Reply-To: <CAGZ79kbvey_f8+R16yYT_qsF0RErOh8own8n-RRApTM0dS-+ag@mail.gmail.com>

On Wed, Nov 30, 2016 at 10:04 AM, Stefan Beller <sbeller@google.com> wrote:

>> Submodules and worktrees seem to have many things in common.
>
> Yes. :)

I moved the code to dir.{c,h} and renamed it to a more generic "move_gitdir",
but then I am thinking further about how to align worktrees and submodules here,
specifically the recursive part.

Whenever submodules and recursion is involved so far we spawned off a child
process to take care of the issue in the submodule itself. Here we followed
the same pattern and called the submodule--helper to embed the git dirs
in the submodule recursively.

As this function is not supposed to be submodule specific anymore, I'd
rather not want to have the recursive childrens code live inside the submodule
helper, so we also need a neutral helper for moving git directories?

So there are a couple ways forward:
* We declare the "recursive" flag to be submodule specific and keep
the recursion
  in the submodule helper
* the recursive flag is generic enough and we invent a plumbing helper.
  Analogous to the submodule--helper, that was originally invented as a C aid
  for the git-submodule shell script, we could have a "git--helper" or just
  "git plumbing <subcmd>", though that sounds like reinventing the wheel as
  traditionally we'd just have a top level command to do a specific thing.
  (side note: Some parts of git-rev-parse could go into such a plumbing
  command)

For now I'd think to declare recursion in this function to be
submodule specific.

Stefan

^ permalink raw reply

* Re: [PATCHv2 4/4] submodule: add embed-git-dir function
From: Stefan Beller @ 2016-11-30 21:00 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Duy Nguyen, Brandon Williams, Git Mailing List, Jonathan Nieder,
	Jens Lehmann, Heiko Voigt
In-Reply-To: <xmqqpolcd73b.fsf@gitster.mtv.corp.google.com>

On Wed, Nov 30, 2016 at 12:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote:
>>> +/*
>>> + * Migrate the given submodule (and all its submodules recursively) from
>>> + * having its git directory within the working tree to the git dir nested
>>> + * in its superprojects git dir under modules/.
>>> + */
>>> +void migrate_submodule_gitdir(const char *prefix, const char *path,
>>> +                             int recursive)
>>
>> Submodules and worktrees seem to have many things in common. The first
>> one is this. "git worktree move" on a worktree that contains
>> submodules .git also benefits from something like this [1]. I suggest
>> you move this function to some neutral place and maybe rename it to
>> relocate_gitdir() or something.
>
> Yeah, good suggestion (including name; first round used "intern" I
> had trouble with, then "embed" which was OK-ish, but probably
> "relocate" is better choice.  If anything, what Stefan's series adds
> is a command to un-embed embedded one).

Heh, good counter perspective. I thought about embedding it
"into the superprojects git directory" whereas your un-embed sounds
like you'd assume embedding is targetd towards the working dir.

relocate as a fancy name for move sounds like it expects 2 arguments
(source and destination), but maybe we could go with:

    git relocate-git-dir (--into-workingtree|--into-gitdir) \
      [--recurse-submodules] \
      [--only-fix-gitfile-links-and-core-setting-as-I-moved-it-myself-already] \
      [--dryrun] [--verbose] [--unsafe-move]

No need to have a pathspec here IMHO. Later it could have another flag
to specify the "main" worktree or such as well.

^ permalink raw reply

* Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Junio C Hamano @ 2016-11-30 20:40 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Johannes Schindelin, GIT Mailing-list
In-Reply-To: <29abc89b-9ca5-930f-8e90-ca446ac2b96a@ramsayjones.plus.com>

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> [I have fixed my config.mak file now, so I don't see the warning
> anymore! Having -Wno-format-zero-length in DEVELOPER_CFLAGS, or
> not, is a separate matter.]

I suspect that 658df95a4a ("add DEVELOPER makefile knob to check for
acknowledged warnings", 2016-02-25) took it from me (namely, Make
script in my 'todo' branch).  In turn, I added it to my set of flags
in order to squelch this exact warning, so...



^ permalink raw reply

* "git add -p ." raises an unexpected "warning: empty strings as pathspecs will be made invalid in upcoming releases. please use . instead if you meant to match all paths"
From: Peter Urda @ 2016-11-30 20:31 UTC (permalink / raw)
  To: git

After upgrading to version 2.11.0 I am getting a warning about empty
strings as pathspecs while using 'patch'

- Ran 'git add -p .' from the root of my git repository.

- I was able to normally stage my changes, but was presented with a
"warning: empty strings as pathspecs will be made invalid in upcoming
releases. please use . instead if you meant to match all paths"
message.

- I expected no warning message since I included a "." with my original command.

I believe that I should not be seeing this warning message as I
included the requested "." pathspec.

~ Peter Urda

http://urda.cc

^ permalink raw reply

* Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Ramsay Jones @ 2016-11-30 19:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <alpine.DEB.2.20.1611301204020.117539@virtualbox>



On 30/11/16 11:07, Johannes Schindelin wrote:
> Hi Ramsay,
> 
> On Tue, 29 Nov 2016, Ramsay Jones wrote:
>> Also, due to a problem in my config.mak file on Linux (a commented
>> out line that had a line continuation '\', grrrrr!), gcc issued a
>> warning, thus:
>>
>>   builtin/difftool.c: In function ‘run_dir_diff’:
>>   builtin/difftool.c:568:13: warning: zero-length gnu_printf format string [-Wformat-zero-length]
>>        warning("");
>>                ^
>> I am not sure why -Wno-format-zero-length is set in DEVELOPER_CFLAGS,
>> but do you really need to space the output with an an 'empty'
>> "warning:" line? (Just curious).
> 
> That `warning("");` comes from a straight-forward port of this line (see
> https://github.com/git/git/blob/v2.11.0/git-difftool.perl#L425):
> 
> 	$errmsg .= "warning:\n";

Ah, OK, so it is being used to 'space out' the (possibly multiple)
'Both files modified' warning(s) followed by a 2-line warning
summary. Hmm, I am not sure the 'Working tree file has been left'
(after each pair of files) part of the message is adding much ...

> I could see two possible ways out:
> 
> - warning("%s", ""); (ugly!)
> 
> - do away with the "prefix every line with warning:" convention and simply
>   have a multi-line `warning(_("...\n...\n"), ...)`
> 
> What do you think?

I think, for now, it is probably best to do nothing. ;-)

Until you have replaced the 'legacy' difftool, it would be best
not to alter the output from the tool, so that the tests can be
used unaltered on both. This could be addressed (if necessary)
after you complete your series.

[I have fixed my config.mak file now, so I don't see the warning
anymore! Having -Wno-format-zero-length in DEVELOPER_CFLAGS, or
not, is a separate matter.]

ATB,
Ramsay Jones


^ permalink raw reply

* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Brandon Williams @ 2016-11-30 19:54 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20161129065125.cwlbkctniy7oshj2@sigill.intra.peff.net>

On 11/29, Jeff King wrote:
> On Tue, Nov 29, 2016 at 01:37:59AM -0500, Jeff King wrote:
> 
> >   2. Grep threads doing more complicated stuff that needs to take a
> >      lock. You might try building with -fsanitize=thread to see if it
> >      turns up anything.
> 
> I tried this and it didn't find anything useful. It complains about
> multiple threads calling want_color() at the same time, which you can
> silence with something like:
> 
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2c727ef49..d48846f40 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -207,6 +207,12 @@ static void start_threads(struct grep_opt *opt)
>  {
>  	int i;
>  
> +	/*
> +	 * trigger want_color() for its side effect of caching the result;
> +	 * otherwise the threads will fight over setting the cache
> +	 */
> +	want_color(GIT_COLOR_AUTO);
> +
>  	pthread_mutex_init(&grep_mutex, NULL);
>  	pthread_mutex_init(&grep_read_mutex, NULL);
>  	pthread_mutex_init(&grep_attr_mutex, NULL);
> 
> But the problem persists even with that patch, so it is something else.
> It may still be a threading problem; -fsanitize=thread isn't perfect. I
> also couldn't get the stress-test to fail when compiled with it. But
> that may simply mean that the timing of the resulting binary is changed
> enough not to trigger the issue.
> 
> -Peff

With you're stress script I'm able to see the failures.  The interesting
thing is that the entry missing is always from the non-submodule file.

-- 
Brandon Williams

^ permalink raw reply

* Re: gitconfig includes
From: Eli Barzilay @ 2016-11-30 19:42 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20161130190653.kk5pboas54yen2it@sigill.intra.peff.net>

On Wed, Nov 30, 2016 at 2:06 PM, Jeff King <peff@peff.net> wrote:
>
> I'm not sure what your script does exactly, but in general I think the
> right thing for most scripts is _not_ to use a specific-file option
> like --global.
>
> If the script is looking up a config value on behalf of a user, it
> probably makes sense for it to use the normal config lookup procedure
> (system, global, repo, command-line), which also enables includes by
> default. That would make it consistent with internal git config
> lookups (e.g., user.name probably only ever appears in global config,
> but you _can_ override it at the repo level if you want to).

This is intended for git newbies (and big company => infinite supply of
them), and also allows them to conveniently nuke the repo and start from
a fresh copy, so it makes sense to make the script inspect/tweak the
global settings.  If knowing git "well enough" was an assumed
requirement, I'd definitely do the normal thing.


> I know that's mostly orthogonal to what we're discussing, but I'd feel
> more convinced that enabling "--includes" with "--global" is useful if
> I thought that "--global" was useful in the first place outside of a
> few narrow debugging cases.

Ok.  Perhaps I overestimated the utility of --global anyway, given the
above...

-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

^ permalink raw reply

* Re: gitconfig includes
From: Jeff King @ 2016-11-30 19:06 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: Junio C Hamano, git
In-Reply-To: <CALO-gusHzTaLg=7X=KqYB==Yz_6yH6qkh8GDK54Lacu5ofD2pw@mail.gmail.com>

On Wed, Nov 30, 2016 at 01:54:35PM -0500, Eli Barzilay wrote:

> I don't have any strong opinion, but FWIW, the use case I have for this
> is as follows: I sync my ~/.gitconfig between my own machine and a work
> machine.  On the work machine though, I like people to have work emails,
> and I wrote some scripts that verify that.  For my case, I added an
> include of a ~/.gitconfig.more which is not synced, and has values that
> override the ones in ~/.gitconfig.  Since I'm the one who also wrote
> that script, I just added an "--includes" to the check so it won't barf
> on my setup, but had it not been my script I'd be stuck.

I'm not sure what your script does exactly, but in general I think the
right thing for most scripts is _not_ to use a specific-file option like
--global.

If the script is looking up a config value on behalf of a user, it
probably makes sense for it to use the normal config lookup procedure
(system, global, repo, command-line), which also enables includes by
default. That would make it consistent with internal git config lookups
(e.g., user.name probably only ever appears in global config, but you
_can_ override it at the repo level if you want to).

I know that's mostly orthogonal to what we're discussing, but I'd feel
more convinced that enabling "--includes" with "--global" is useful if I
thought that "--global" was useful in the first place outside of a few
narrow debugging cases.

-Peff

^ permalink raw reply

* Re: gitconfig includes
From: Eli Barzilay @ 2016-11-30 18:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqeg1udkg4.fsf@gitster.mtv.corp.google.com>

On Tue, Nov 29, 2016 at 4:50 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> I think it's arguable whether "--global" should behave the same.
>
> I know you know this and I am writing this message for others.
>
> I admit that I wondered if "a single file" ought to cover these
> short-hand notations like --global and --local while re-reading the
> log message of 9b25a0b52 (config: add include directive,
> 2012-02-06).  In other words, I agree that it used to be arguable
> before we released v1.7.10.
>
> It no longer is arguable simply due to backward compatibilty.  The
> ship has long sailed.

I don't have any strong opinion, but FWIW, the use case I have for this
is as follows: I sync my ~/.gitconfig between my own machine and a work
machine.  On the work machine though, I like people to have work emails,
and I wrote some scripts that verify that.  For my case, I added an
include of a ~/.gitconfig.more which is not synced, and has values that
override the ones in ~/.gitconfig.  Since I'm the one who also wrote
that script, I just added an "--includes" to the check so it won't barf
on my setup, but had it not been my script I'd be stuck.

This is all a "FWIW" -- in case anyone thinks about use cases for a
possible (future) change of the default.

-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

^ permalink raw reply

* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Junio C Hamano @ 2016-11-30 18:39 UTC (permalink / raw)
  To: Jakub Narębski
  Cc: Johannes Schindelin, Jeff King, git, David Aguilar,
	Dennis Kaarsemaker
In-Reply-To: <855b9172-7225-e09e-e46d-87940f9fda75@gmail.com>

Jakub Narębski <jnareb@gmail.com> writes:

>> My original "create a file in libexec/git-core/" was simple, did the job
>> reliably, and worked also for testing.
>> 
>> It is a pity that you two gentlemen shot it down for being inelegant. And
>> ever since, we try to find a solution that is as simple, works as
>> reliably, also for testing, *and* appeases your tastes.
>
> I just would like to note that existence of file is used for both
> git-daemon and gitweb (the latter following the git-daemon example).
>
> So there is a precedent for the use of this mechanism.

I think you are thinking about git-daemon-export-ok (for 'git
daemon') and $GITWEB_EXPORT_OK file (for 'gitweb').

You do realize that it is apples-and-oranges [*1*] to take these as
analogous to what Dscho is trying to do, don't you?

First of all, these are to control access to each repository on the
server side; the presence of the file is checked in each repository.
What Dscho wants is to control the behaviour of an installation of
Git as a whole, no matter which repository is being accessed [*2*,
*3*].

More importantly, did you notice that git-daemon-export-ok predates
the configuration mechanism by a large margin?  The "does the file
exist?" check done in a87e8be2ae ("Add a "git-daemon" that listens
on a TCP port", 2005-07-13) is a relic from the past [*4*], and
32f4aaccaa ("gitweb: export options", 2006-09-17) added
GITWEB_EXPORT_OK to mimic it, also long time ago [*5*].  They are
not something you would want to mimic in new programs these days.

Besides, $GIT_EXEC_PATH is where you place git subcommands.  Who in
the right mind considers it even remotely sane to design a system
where you have to throw in a file that is not a command to /usr/bin
to control the behaviour of your system? [*6*]

So the "precedent" is irrelevant in the first place, and even if it
were relevant, it is a bad piece of advice to mimic it.


[Footnote]

*1* Or is it apples-and-pineapples these days?

*2* Not that I agree with that desire, if I understand him correctly
    from his description against the approach based on an
    environment variable.  If a user has multiple installations and
    not even aware of which one of them s/he is currently using, a
    mechanism that affects only one of them (instead of consistently
    affecting all of them) would lead to more confusion, I would
    think.  

*3* If such hermetically configured independent installations are
    desirable, etc/gitconfig aka "git config --system" is a more
    appropriate thing to use, and you do not need to do repository
    discovery before you can read it.

*4* If we had config mechanism, we would have used it just like we
    use daemon.* variables to control what services are enabled for
    each repository.

*5* By that time, the config mechanism did already exist, so the
    GITWEB_EXPORT_OK could have been a per-repository configuration,
    but "gitweb" had another excuse to deviate from the norm.  "Is
    this repository visible?" was done during repository listing and
    the script did not want to run "git config" in each and every
    repository-like directory it encountered in File::Find::find().

*6* And I do not think $GIT_EXEC_PATH vs /usr/bin is
    apples-and-oranges analogy.

^ permalink raw reply

* Re: [PATCHv2 4/4] submodule: add embed-git-dir function
From: Stefan Beller @ 2016-11-30 18:04 UTC (permalink / raw)
  To: Duy Nguyen
  Cc: Brandon Williams, Junio C Hamano, Git Mailing List,
	Jonathan Nieder, Jens Lehmann, Heiko Voigt
In-Reply-To: <CACsJy8Ce3Oa-xJ4BwgRRy6neM=Jxkfqq7yboHZDXLDG2tu9GzQ@mail.gmail.com>

On Wed, Nov 30, 2016 at 3:14 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote:
>> +/*
>> + * Migrate the given submodule (and all its submodules recursively) from
>> + * having its git directory within the working tree to the git dir nested
>> + * in its superprojects git dir under modules/.
>> + */
>> +void migrate_submodule_gitdir(const char *prefix, const char *path,
>> +                             int recursive)
>
> Submodules and worktrees seem to have many things in common.

Yes. :)

> The first
> one is this. "git worktree move" on a worktree that contains
> submodules .git also benefits from something like this [1].

That patch is a sensible approach. :)
(By checking all files to not be submodules a worktree would not run
into problems like
a127331cd81, mv: allow moving nested submodules)

> I suggest
> you move this function to some neutral place and maybe rename it to
> relocate_gitdir() or something.

ok tell me where this neutral place is found?
(I'd prefer to not clobber it into cache.h *the* most neutral place in git)
Maybe dir.{c,h} ?

>
> It probably should take a bit flag instead of "recursive" here. One
> thing I would need is the ability to tell this function "I have moved
> all these .git dirs already (because I move whole worktree in one
> operation), here are the old and new locations of them, fix them up!".
> In other words, no rename() could be optionally skipped.

In the non-main working trees you'd also have a .git file linking
to the actual git dir and you'd only have to fix them up instead of moving.


>
> [1] https://public-inbox.org/git/20161128094319.16176-11-pclouds@gmail.com/T/#u
>
>> +{
>> +       char *old_git_dir;
>> +       const char *new_git_dir;
>> +       const struct submodule *sub;
>> +
>> +       old_git_dir = xstrfmt("%s/.git", path);
>> +       if (read_gitfile(old_git_dir))
>> +               /* If it is an actual gitfile, it doesn't need migration. */
>> +               goto out;
>> +
>> +       sub = submodule_from_path(null_sha1, path);
>> +       if (!sub)
>> +               die(_("Could not lookup name for submodule '%s'"),
>> +                     path);
>> +
>> +       new_git_dir = git_common_path("modules/%s", sub->name);
>
> Why doesn't git_path() work here? This would make "modules" shared
> between worktrees, even though it's not normally. That inconsistency
> could cause trouble.

I thought that was a long term goal?
(I actually think about reviving the series you sent out a few weeks ago
to make worktree and submodules work well together)

So for that we'd want to have at least the object store shared across all
worktrees.

>
>> +       if (safe_create_leading_directories_const(new_git_dir) < 0)
>> +               die(_("could not create directory '%s'"), new_git_dir);
>> +
>> +       if (!prefix)
>> +               prefix = get_super_prefix();
>> +       printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
>> +               prefix ? prefix : "", path,
>> +               real_path(old_git_dir), 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);
>
> Another thing in common is, both submodules and worktrees use some
> form of textual symlinks. You need to fix up some here. But if this
> submodule has multiple worktreee, there may be some "symlinks" in
> .git/worktrees which would need fixing up as well.

We could signal that via one of the flag bits?
(e.g. FIXUP_WORKTREE_SYMLINKS )

>
> You don't have to do the fix up thing right away, but I think we
> should at least make sure we leave no dangling links behind (by
> die()ing early if we find a .git dir we can't handle yet)
> --
> Duy

^ permalink raw reply

* [PATCH v2 1/1] convert: git cherry-pick -Xrenormalize did not work
From: tboegi @ 2016-11-30 17:02 UTC (permalink / raw)
  To: git, eevee.reply; +Cc: Torsten Bögershausen
In-Reply-To: <6a7e155-f399-c9f8-c69e-8164e0735dfb@veekun.com>

From: Torsten Bögershausen <tboegi@web.de>

Working with a repo that used to be all CRLF. At some point it
was changed to all LF, with `text=auto` in .gitattributes.
Trying to cherry-pick a commit from before the switchover fails:

$ git cherry-pick -Xrenormalize <commit>
    fatal: CRLF would be replaced by LF in [path]

Commit 65237284 "unify the "auto" handling of CRLF" introduced
a regression:

Whenever crlf_action is CRLF_TEXT_XXX and not CRLF_AUTO_XXX,
SAFE_CRLF_RENORMALIZE was feed into check_safe_crlf().
This is wrong because here everything else than SAFE_CRLF_WARN is
treated as SAFE_CRLF_FAIL.

Call check_safe_crlf() only if checksafe is SAFE_CRLF_WARN or SAFE_CRLF_FAIL.

Reported-by: Eevee (Lexy Munroe) <eevee@veekun.com>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 convert.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/convert.c b/convert.c
index be91358..f8e4dfe 100644
--- a/convert.c
+++ b/convert.c
@@ -281,13 +281,13 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 		/*
 		 * If the file in the index has any CR in it, do not convert.
 		 * This is the new safer autocrlf handling.
+		   - unless we want to renormalize in a merge or cherry-pick
 		 */
-		if (checksafe == SAFE_CRLF_RENORMALIZE)
-			checksafe = SAFE_CRLF_FALSE;
-		else if (has_cr_in_index(path))
+		if ((checksafe != SAFE_CRLF_RENORMALIZE) && has_cr_in_index(path))
 			convert_crlf_into_lf = 0;
 	}
-	if (checksafe && len) {
+	if ((checksafe == SAFE_CRLF_WARN ||
+	    (checksafe == SAFE_CRLF_FAIL)) && len) {
 		struct text_stat new_stats;
 		memcpy(&new_stats, &stats, sizeof(new_stats));
 		/* simulate "git add" */
-- 
2.10.0


^ permalink raw reply related

* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Jakub Narębski @ 2016-11-30 16:02 UTC (permalink / raw)
  To: Johannes Schindelin, Junio C Hamano
  Cc: Jeff King, git, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611281830040.117539@virtualbox>

Hello,

W dniu 28.11.2016 o 18:34, Johannes Schindelin pisze:

> My original "create a file in libexec/git-core/" was simple, did the job
> reliably, and worked also for testing.
> 
> It is a pity that you two gentlemen shot it down for being inelegant. And
> ever since, we try to find a solution that is as simple, works as
> reliably, also for testing, *and* appeases your tastes.

I just would like to note that existence of file is used for both
git-daemon and gitweb (the latter following the git-daemon example).

So there is a precedent for the use of this mechanism.

Best,
-- 
Jakub Narębski


^ permalink raw reply

* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Jeff King @ 2016-11-30 12:35 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, git, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611301330100.117539@virtualbox>

On Wed, Nov 30, 2016 at 01:30:47PM +0100, Johannes Schindelin wrote:

> On Tue, 29 Nov 2016, Jeff King wrote:
> 
> > On Tue, Nov 29, 2016 at 09:36:55PM +0100, Johannes Schindelin wrote:
> > 
> > > So the suggestion by both you and Peff, to use an environment variable,
> > > which is either global, or requires the user to set it manually per
> > > session, is simply not a good idea at all.
> > 
> > No, my suggestion was to use config and have the test suite use an
> > environment variable to test both cases (preferably automatically,
> > without the user having to do anything).
> > 
> > I do not see how that fails to cover all of your use cases.
> 
> Oh, so the suggestion is to have *both* a config *and* an environment
> variable. That is not elegant.

No, that is not at all what I said.  I was going to explain myself
again, but I do not see what good it would do, as clearly my point did
not come across in the other three emails. And then you would just
complain that I am making work for you. So whatever. I do not care about
your difftool topic at all. Do whatever you like (which hey, I already
said before, too).

-Peff

^ permalink raw reply

* [PATCH v2] tag, branch, for-each-ref: add --ignore-case for sorting and filtering
From: Nguyễn Thái Ngọc Duy @ 2016-11-30 12:35 UTC (permalink / raw)
  To: git; +Cc: karthik.188, Nguyễn Thái Ngọc Duy
In-Reply-To: <20161117102114.16649-1-pclouds@gmail.com>

This option makes sorting ignore case, which is great when you have
branches named bug-12-do-something, Bug-12-do-some-more and
BUG-12-do-what and want to group them together. Sorting externally may
not be an option because we lose coloring and column layout from
git-branch and git-tag.

The same could be said for filtering, but it's probably less important
because you can always go with the ugly pattern [bB][uU][gG]-* if you're
desperate.

You can't have case-sensitive filtering and case-insensitive sorting (or
the other way around) with this though. But who would want that?

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v2 has a different approach, and I think it's a better one even with
 that unanswered question above.

 Documentation/git-branch.txt       |  4 ++++
 Documentation/git-for-each-ref.txt |  3 +++
 Documentation/git-tag.txt          |  4 ++++
 builtin/branch.c                   | 23 ++++++++++++++---------
 builtin/for-each-ref.c             |  5 ++++-
 builtin/tag.c                      |  4 ++++
 ref-filter.c                       | 28 +++++++++++++++++++++-------
 ref-filter.h                       |  2 ++
 t/t3203-branch-output.sh           | 22 ++++++++++++++++++++++
 t/t7004-tag.sh                     | 20 ++++++++++++++++++++
 10 files changed, 98 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 1fe7344..5516a47 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -118,6 +118,10 @@ OPTIONS
 	default to color output.
 	Same as `--color=never`.
 
+-i::
+--ignore-case::
+	Sorting and filtering branches are case insensitive.
+
 --column[=<options>]::
 --no-column::
 	Display branch listing in columns. See configuration variable
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69b..6d22974 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -79,6 +79,9 @@ OPTIONS
 	Only list refs which contain the specified commit (HEAD if not
 	specified).
 
+--ignore-case::
+	Sorting and filtering refs are case insensitive.
+
 FIELD NAMES
 -----------
 
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 80019c5..76cfe40 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -108,6 +108,10 @@ OPTIONS
 	variable if it exists, or lexicographic order otherwise. See
 	linkgit:git-config[1].
 
+-i::
+--ignore-case::
+	Sorting and filtering tags are case insensitive.
+
 --column[=<options>]::
 --no-column::
 	Display tag listing in columns. See configuration variable
diff --git a/builtin/branch.c b/builtin/branch.c
index 60cc5c8..36e0a21 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -512,15 +512,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 	if (filter->verbose)
 		maxwidth = calc_maxwidth(&array, strlen(remote_prefix));
 
-	/*
-	 * If no sorting parameter is given then we default to sorting
-	 * by 'refname'. This would give us an alphabetically sorted
-	 * array with the 'HEAD' ref at the beginning followed by
-	 * local branches 'refs/heads/...' and finally remote-tacking
-	 * branches 'refs/remotes/...'.
-	 */
-	if (!sorting)
-		sorting = ref_default_sorting();
 	ref_array_sort(sorting, &array);
 
 	for (i = 0; i < array.nr; i++)
@@ -645,6 +636,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	const char *new_upstream = NULL;
 	enum branch_track track;
 	struct ref_filter filter;
+	int icase = 0;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 
 	struct option options[] = {
@@ -686,6 +678,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
 			N_("print only branches of the object"), 0, parse_opt_object_name
 		},
+		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
 
@@ -723,6 +716,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	if (filter.abbrev == -1)
 		filter.abbrev = DEFAULT_ABBREV;
+	filter.ignore_case = icase;
+
 	finalize_colopts(&colopts, -1);
 	if (filter.verbose) {
 		if (explicitly_enable_column(colopts))
@@ -744,6 +739,16 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
 			filter.kind |= FILTER_REFS_DETACHED_HEAD;
 		filter.name_patterns = argv;
+		/*
+		 * If no sorting parameter is given then we default to sorting
+		 * by 'refname'. This would give us an alphabetically sorted
+		 * array with the 'HEAD' ref at the beginning followed by
+		 * local branches 'refs/heads/...' and finally remote-tacking
+		 * branches 'refs/remotes/...'.
+		 */
+		if (!sorting)
+			sorting = ref_default_sorting();
+		sorting->ignore_case = icase;
 		print_ref_list(&filter, sorting);
 		print_columns(&output, colopts, NULL);
 		string_list_clear(&output, 0);
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4e9f6c2..df41fa0 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -18,7 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
-	int maxcount = 0, quote_style = 0;
+	int maxcount = 0, quote_style = 0, icase = 0;
 	struct ref_array array;
 	struct ref_filter filter;
 
@@ -43,6 +43,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_MERGED(&filter, N_("print only refs that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only refs that are not merged")),
 		OPT_CONTAINS(&filter.with_commit, N_("print only refs which contain the commit")),
+		OPT_BOOL(0, "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END(),
 	};
 
@@ -63,6 +64,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 
 	if (!sorting)
 		sorting = ref_default_sorting();
+	sorting->ignore_case = icase;
+	filter.ignore_case = icase;
 
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
diff --git a/builtin/tag.c b/builtin/tag.c
index 50e4ae5..73df728 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -335,6 +335,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_filter filter;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	const char *format = NULL;
+	int icase = 0;
 	struct option options[] = {
 		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
 		{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -370,6 +371,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			N_("print only tags of the object"), 0, parse_opt_object_name
 		},
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
+		OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
 		OPT_END()
 	};
 
@@ -401,6 +403,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	}
 	if (!sorting)
 		sorting = ref_default_sorting();
+	sorting->ignore_case = icase;
+	filter.ignore_case = icase;
 	if (cmdmode == 'l') {
 		int ret;
 		if (column_active(colopts)) {
diff --git a/ref-filter.c b/ref-filter.c
index f5f7a70..bd98010 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1231,8 +1231,14 @@ static int commit_contains(struct ref_filter *filter, struct commit *commit)
  * matches a pattern "refs/heads/mas") or a wildcard (e.g. the same ref
  * matches "refs/heads/mas*", too).
  */
-static int match_pattern(const char **patterns, const char *refname)
+static int match_pattern(const struct ref_filter *filter, const char *refname)
 {
+	const char **patterns = filter->name_patterns;
+	unsigned flags = 0;
+
+	if (filter->ignore_case)
+		flags |= WM_CASEFOLD;
+
 	/*
 	 * When no '--format' option is given we need to skip the prefix
 	 * for matching refs of tags and branches.
@@ -1243,7 +1249,7 @@ static int match_pattern(const char **patterns, const char *refname)
 	       skip_prefix(refname, "refs/", &refname));
 
 	for (; *patterns; patterns++) {
-		if (!wildmatch(*patterns, refname, 0, NULL))
+		if (!wildmatch(*patterns, refname, flags, NULL))
 			return 1;
 	}
 	return 0;
@@ -1255,9 +1261,15 @@ static int match_pattern(const char **patterns, const char *refname)
  * matches a pattern "refs/heads/" but not "refs/heads/m") or a
  * wildcard (e.g. the same ref matches "refs/heads/m*", too).
  */
-static int match_name_as_path(const char **pattern, const char *refname)
+static int match_name_as_path(const struct ref_filter *filter, const char *refname)
 {
+	const char **pattern = filter->name_patterns;
 	int namelen = strlen(refname);
+	unsigned flags = WM_PATHNAME;
+
+	if (filter->ignore_case)
+		flags |= WM_CASEFOLD;
+
 	for (; *pattern; pattern++) {
 		const char *p = *pattern;
 		int plen = strlen(p);
@@ -1280,8 +1292,8 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
 	if (!*filter->name_patterns)
 		return 1; /* No pattern always matches */
 	if (filter->match_as_path)
-		return match_name_as_path(filter->name_patterns, refname);
-	return match_pattern(filter->name_patterns, refname);
+		return match_name_as_path(filter, refname);
+	return match_pattern(filter, refname);
 }
 
 /*
@@ -1536,18 +1548,20 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
 	struct atom_value *va, *vb;
 	int cmp;
 	cmp_type cmp_type = used_atom[s->atom].type;
+	int (*cmp_fn)(const char *, const char *);
 
 	get_ref_atom_value(a, s->atom, &va);
 	get_ref_atom_value(b, s->atom, &vb);
+	cmp_fn = s->ignore_case ? strcasecmp : strcmp;
 	if (s->version)
 		cmp = versioncmp(va->s, vb->s);
 	else if (cmp_type == FIELD_STR)
-		cmp = strcmp(va->s, vb->s);
+		cmp = cmp_fn(va->s, vb->s);
 	else {
 		if (va->ul < vb->ul)
 			cmp = -1;
 		else if (va->ul == vb->ul)
-			cmp = strcmp(a->refname, b->refname);
+			cmp = cmp_fn(a->refname, b->refname);
 		else
 			cmp = 1;
 	}
diff --git a/ref-filter.h b/ref-filter.h
index 14d435e..fc55fa3 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -29,6 +29,7 @@ struct ref_sorting {
 	struct ref_sorting *next;
 	int atom; /* index into used_atom array (internal) */
 	unsigned reverse : 1,
+		ignore_case : 1,
 		version : 1;
 };
 
@@ -62,6 +63,7 @@ struct ref_filter {
 
 	unsigned int with_commit_tag_algo : 1,
 		match_as_path : 1,
+		ignore_case : 1,
 		detached : 1;
 	unsigned int kind,
 		lines;
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index c6a3ccb..fad79e8 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -89,6 +89,11 @@ test_expect_success 'git branch --list -v pattern shows branch summaries' '
 	awk "{print \$NF}" <tmp >actual &&
 	test_cmp expect actual
 '
+test_expect_success 'git branch --ignore-case --list -v pattern shows branch summaries' '
+	git branch --list --ignore-case -v BRANCH* >tmp &&
+	awk "{print \$NF}" <tmp >actual &&
+	test_cmp expect actual
+'
 
 test_expect_success 'git branch -v pattern does not show branch summaries' '
 	test_must_fail git branch -v branch*
@@ -196,4 +201,21 @@ test_expect_success 'local-branch symrefs shortened properly' '
 	test_cmp expect actual
 '
 
+test_expect_success 'sort branches, ignore case' '
+	(
+		git init sort-icase &&
+		cd sort-icase &&
+		test_commit initial &&
+		git branch branch-one &&
+		git branch BRANCH-two &&
+		git branch --list -i | awk "{print \$NF}" >actual &&
+		cat >expected <<-\EOF &&
+		branch-one
+		BRANCH-two
+		master
+		EOF
+		test_cmp expected actual
+	)
+'
+
 test_done
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a..2d9cae3 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -27,6 +27,23 @@ test_expect_success 'listing all tags in an empty tree should output nothing' '
 	test $(git tag | wc -l) -eq 0
 '
 
+test_expect_success 'sort tags, ignore case' '
+	(
+		git init sort &&
+		cd sort &&
+		test_commit initial &&
+		git tag tag-one &&
+		git tag TAG-two &&
+		git tag -l -i >actual &&
+		cat >expected <<-\EOF &&
+		initial
+		tag-one
+		TAG-two
+		EOF
+		test_cmp expected actual
+	)
+'
+
 test_expect_success 'looking for a tag in an empty tree should fail' \
 	'! (tag_exists mytag)'
 
@@ -81,6 +98,9 @@ test_expect_success 'listing all tags if one exists should output that tag' '
 test_expect_success 'listing a tag using a matching pattern should succeed' \
 	'git tag -l mytag'
 
+test_expect_success 'listing a tag using a matching pattern should succeed' \
+	'git tag -l --ignore-case MYTAG'
+
 test_expect_success \
 	'listing a tag using a matching pattern should output that tag' \
 	'test $(git tag -l mytag) = mytag'
-- 
2.8.2.524.g6ff3d78


^ permalink raw reply related

* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Johannes Schindelin @ 2016-11-30 12:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <20161129204915.lo45b2jz57dlfug5@sigill.intra.peff.net>

Hi Peff,

On Tue, 29 Nov 2016, Jeff King wrote:

> On Tue, Nov 29, 2016 at 09:36:55PM +0100, Johannes Schindelin wrote:
> 
> > So the suggestion by both you and Peff, to use an environment variable,
> > which is either global, or requires the user to set it manually per
> > session, is simply not a good idea at all.
> 
> No, my suggestion was to use config and have the test suite use an
> environment variable to test both cases (preferably automatically,
> without the user having to do anything).
> 
> I do not see how that fails to cover all of your use cases.

Oh, so the suggestion is to have *both* a config *and* an environment
variable. That is not elegant.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Johannes Schindelin @ 2016-11-30 12:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <xmqqshqadn0f.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Tue, 29 Nov 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > So the suggestion by both you and Peff, to use an environment variable,
> > which is either global, or requires the user to set it manually per
> > session, is simply not a good idea at all.
> 
> As I already said, I do not have a strong preference between config
> and env.  I raised the env as a possible alternative that you can
> think about its pros and cons, and as I already said, if you thought
> and your concluded that config would work better for your needs,
> that is fine by me.

The env flat out fails, on the grounds of not integrating nicely into a
Git for Windows installer.

The config kinda works now. But for what price. It stole 4 hours I did not
have. When the libexec/git-core/use-builtin-difftool solution took me a
grand total of half an hour to devise, implement and test.

And you know what? I still do not really see what is so bad about it.

And I still see what is bad about the config "solution": it *creates* a
chicken-and-egg problem with the order of config reading vs running
scripts. It *creates* problems for requiring to spawn a `git config` call
because reading the config in-process would mess up the global variables
and environment *beyond repair*, making it *impossible* to even spawn the
git-legacy-difftool Perl script.

In short: the config setting now works. But it is ugly as hell. I wish I
never had listened to you.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCHv2 4/4] submodule: add embed-git-dir function
From: Duy Nguyen @ 2016-11-30 11:14 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Brandon Williams, Junio C Hamano, Git Mailing List,
	Jonathan Nieder, Jens Lehmann, Heiko Voigt
In-Reply-To: <20161122192235.6055-5-sbeller@google.com>

On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote:
> +/*
> + * Migrate the given submodule (and all its submodules recursively) from
> + * having its git directory within the working tree to the git dir nested
> + * in its superprojects git dir under modules/.
> + */
> +void migrate_submodule_gitdir(const char *prefix, const char *path,
> +                             int recursive)

Submodules and worktrees seem to have many things in common. The first
one is this. "git worktree move" on a worktree that contains
submodules .git also benefits from something like this [1]. I suggest
you move this function to some neutral place and maybe rename it to
relocate_gitdir() or something.

It probably should take a bit flag instead of "recursive" here. One
thing I would need is the ability to tell this function "I have moved
all these .git dirs already (because I move whole worktree in one
operation), here are the old and new locations of them, fix them up!".
In other words, no rename() could be optionally skipped.

[1] https://public-inbox.org/git/20161128094319.16176-11-pclouds@gmail.com/T/#u

> +{
> +       char *old_git_dir;
> +       const char *new_git_dir;
> +       const struct submodule *sub;
> +
> +       old_git_dir = xstrfmt("%s/.git", path);
> +       if (read_gitfile(old_git_dir))
> +               /* If it is an actual gitfile, it doesn't need migration. */
> +               goto out;
> +
> +       sub = submodule_from_path(null_sha1, path);
> +       if (!sub)
> +               die(_("Could not lookup name for submodule '%s'"),
> +                     path);
> +
> +       new_git_dir = git_common_path("modules/%s", sub->name);

Why doesn't git_path() work here? This would make "modules" shared
between worktrees, even though it's not normally. That inconsistency
could cause trouble.

> +       if (safe_create_leading_directories_const(new_git_dir) < 0)
> +               die(_("could not create directory '%s'"), new_git_dir);
> +
> +       if (!prefix)
> +               prefix = get_super_prefix();
> +       printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
> +               prefix ? prefix : "", path,
> +               real_path(old_git_dir), 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);

Another thing in common is, both submodules and worktrees use some
form of textual symlinks. You need to fix up some here. But if this
submodule has multiple worktreee, there may be some "symlinks" in
.git/worktrees which would need fixing up as well.

You don't have to do the fix up thing right away, but I think we
should at least make sure we leave no dangling links behind (by
die()ing early if we find a .git dir we can't handle yet)
-- 
Duy

^ permalink raw reply

* Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Johannes Schindelin @ 2016-11-30 11:07 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <4ddad7ea-5ac8-20b2-da9e-5843c486878a@ramsayjones.plus.com>

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

Hi Ramsay,

On Tue, 29 Nov 2016, Ramsay Jones wrote:

> If you need to re-roll your 'js/difftool-builtin' branch, could
> you please squash this into the relevant patch.

Fixed. Thanks!

> Also, due to a problem in my config.mak file on Linux (a commented
> out line that had a line continuation '\', grrrrr!), gcc issued a
> warning, thus:
> 
>   builtin/difftool.c: In function ‘run_dir_diff’:
>   builtin/difftool.c:568:13: warning: zero-length gnu_printf format string [-Wformat-zero-length]
>        warning("");
>                ^
> I am not sure why -Wno-format-zero-length is set in DEVELOPER_CFLAGS,
> but do you really need to space the output with an an 'empty'
> "warning:" line? (Just curious).

That `warning("");` comes from a straight-forward port of this line (see
https://github.com/git/git/blob/v2.11.0/git-difftool.perl#L425):

	$errmsg .= "warning:\n";

I could see two possible ways out:

- warning("%s", ""); (ugly!)

- do away with the "prefix every line with warning:" convention and simply
  have a multi-line `warning(_("...\n...\n"), ...)`

What do you think?
Dscho

^ permalink raw reply

* Re: Partial fetch?
From: Dāvis Mosāns @ 2016-11-30  0:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Jonathan Tan, git
In-Reply-To: <20161129215538.vhgmxpq4rasri4vm@sigill.intra.peff.net>

2016-11-29 23:55 GMT+02:00 Jeff King <peff@peff.net>:
> On Mon, Nov 28, 2016 at 10:34:51PM +0200, Dāvis Mosāns wrote:
>
>> I'm trying to fetch a remote repository over https but sadly it
>> timeouts too soon.
>>
>> $ git fetch -v upstream
>> POST git-upload-pack (gzip 1148 to 641 bytes)
>> POST git-upload-pack (gzip 1148 to 644 bytes)
>> [...]
>> Is there some way to fetch partially by smaller chunks and then repeat
>> that again till everything is fetched?
>
> Not an easy one. The series of POSTs is an indication that the fetch
> negotiation is going on for a long time, which probably means you have a
> lot of commits in your local repository that aren't in the remote, or
> vice versa.
>
> Here are the things I might try:
>
>   - git v2.10.2 has commit 06b3d386e (fetch-pack: do not reset in_vain
>     on non-novel acks, 2016-09-23), which may help with this.
>

That output and this is already with git v2.10.2

>   - HTTP, because the server is stateless, performs less well than other
>     protocols. If you can fetch over ssh or git://, it will probably
>     just work.
>

It's only available under https://git.replicant.us/replicant/frameworks_base.git

>   - If this is a one-time thing to fetch unrelated history from another
>     repository, you can "clone --mirror" instead of fetching,
>     then fetch from the mirror locally. Subsequent fetches should be
>     fast.
>

Looks like something is wrong with their server/setup since currently
even clone doesn't work for me.

 Cloning into bare repository 'frameworks_base.git'...
remote: Counting objects: 739930, done.
remote: Compressing objects: 100% (196567/196567), done.
error: RPC failed; curl 18 transfer closed with outstanding read data remaining
fatal: The remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed

But someone have made a mirror for it and there clone works only still
same issue with fetch.
Once I will have this cloned I will try local fetch.

Thanks!

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)
From: Stefan Beller @ 2016-11-30  0:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kZbBufaKoJyTQa_DseE5GofMAwb=ujtEYn35r9VEwdJ2g@mail.gmail.com>

On Tue, Nov 29, 2016 at 11:21 AM, Stefan Beller <sbeller@google.com> wrote:
>
>>
>> * dt/empty-submodule-in-merge (2016-11-17) 1 commit
>>  - submodules: allow empty working-tree dirs in merge/cherry-pick
>>
>>  Waiting for review
>
> That slipped by me. Will review.
>

I reviewed what you have queued and it still looks good to me.

^ permalink raw reply

* Re: [PATCH v2 00/11] git worktree (re)move
From: Stefan Beller @ 2016-11-30  0:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Duy Nguyen, Johannes Schindelin, Git Mailing List, Johannes Sixt
In-Reply-To: <xmqqfumaf5lf.fsf@gitster.mtv.corp.google.com>

On Tue, Nov 29, 2016 at 11:28 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Another way, as pointed out by j6t, is go with "move within filesystem
>> only", at least at the first step. Which is probably a good idea
>> anyway so we can concentrate on git-specific stuff before going to
>> minor and complicated copy/move details.
>
> Yup, that is a very sensible approach.

In case you decided to go another way than a plain rename, please
make it easy to reuse (e.g. move_directory_safely exposed to the
rest of the codebase). I'd find use for that in the series currently
queued as:

  sb/submodule-intern-gitdir

which moves git directories of submodules around (from inside the
submodule/.git to the superprojects .git/modules/<name>)

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH v2 00/11] git worktree (re)move
From: Duy Nguyen @ 2016-11-30  0:09 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, Johannes Schindelin, Git Mailing List
In-Reply-To: <c73ec2ee-eed0-414c-63a1-a1bd5c864cef@kdbg.org>

On Wed, Nov 30, 2016 at 4:14 AM, Johannes Sixt <j6t@kdbg.org> wrote:
>> diff --git a/copy.c b/copy.c
>> index 4de6a11..b232aec 100644
>> --- a/copy.c
>> +++ b/copy.c
>> @@ -65,3 +65,9 @@ int copy_file_with_time(const char *dst, const char
>> *src, int mode)
>>                 return copy_times(dst, src);
>>         return status;
>>  }
>> +
>> +int copy_dir_recursively(const char *dst, const char *src)
>> +{
>> +       errno = ENOSYS;
>> +       return -1;
>> +}
>
>
> An error message "cannot move directories across devices" or something would
> be preferable over "Function not implemented", of course. Or did you mean to
> set errno = EXDEV?

The exact message is not super important right now. Though I'm
thinking of adding move_directory() that is a wrapper of rename(). We
can die("cannot move directories across devices") then and hopefully
be able to move across devices at some point.
-- 
Duy

^ permalink raw reply

* Re: [ANNOUNCE] Git v2.11.0
From: Junio C Hamano @ 2016-11-29 21:55 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20161129214243.uunmdc5omlogipso@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Tue, Nov 29, 2016 at 01:21:51PM -0800, Junio C Hamano wrote:
>
>> The latest feature release Git v2.11.0 is now available at the
>> usual places.  It is comprised of 673 non-merge commits since
>> v2.10.0, contributed by 74 people, 15 of which are new faces.
>> [...]
>> Jeff King (117):
>> [...]
>>       common-main: stop munging argv[0] path
>
> Oh, I didn't expect this to go in at the last minute. The regression was
> actually in 2.10.0, so I figured it would just end up as part of 2.11.1.

To be honest, inclusion of this was a screw-up ;-)  I couldn't count
between 2.10 and 2.11-rc.

^ permalink raw reply

* Re: Partial fetch?
From: Jeff King @ 2016-11-29 21:55 UTC (permalink / raw)
  To: Dāvis Mosāns; +Cc: Jonathan Tan, git
In-Reply-To: <CAOE4rSzTq6DVR2ch+as9Pbo35NjKP5b1+Ub1XZWEnwJTahqEfg@mail.gmail.com>

On Mon, Nov 28, 2016 at 10:34:51PM +0200, Dāvis Mosāns wrote:

> I'm trying to fetch a remote repository over https but sadly it
> timeouts too soon.
> 
> $ git fetch -v upstream
> POST git-upload-pack (gzip 1148 to 641 bytes)
> POST git-upload-pack (gzip 1148 to 644 bytes)
> [...]
> Is there some way to fetch partially by smaller chunks and then repeat
> that again till everything is fetched?

Not an easy one. The series of POSTs is an indication that the fetch
negotiation is going on for a long time, which probably means you have a
lot of commits in your local repository that aren't in the remote, or
vice versa.

Here are the things I might try:

  - git v2.10.2 has commit 06b3d386e (fetch-pack: do not reset in_vain
    on non-novel acks, 2016-09-23), which may help with this.

  - HTTP, because the server is stateless, performs less well than other
    protocols. If you can fetch over ssh or git://, it will probably
    just work.

  - If this is a one-time thing to fetch unrelated history from another
    repository, you can "clone --mirror" instead of fetching,
    then fetch from the mirror locally. Subsequent fetches should be
    fast.

If you do try v2.10.2 and it improves things, I'd be interested to hear
about it as a data point.

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox