Git development
 help / color / mirror / Atom feed
* Re: [PATCH] convert: mark a file-local symbol static
From: Ramsay Jones @ 2016-10-17 17:21 UTC (permalink / raw)
  To: Jeff King, Johannes Schindelin
  Cc: Lars Schneider, Junio C Hamano, GIT Mailing-list
In-Reply-To: <20161017093754.wah5d6cg4qgtw7ln@sigill.intra.peff.net>



On 17/10/16 10:37, Jeff King wrote:
> On Mon, Oct 17, 2016 at 11:04:19AM +0200, Johannes Schindelin wrote:
> 
>>> Gross. I would not be opposed to a Makefile rule that outputs the
>>> correct set of OBJECTS so this (or other) scripts could build on it.
>>
>> You could also use the method I use in Git for Windows to "extend" the
>> Makefile:
>>
>> -- snipsnap --
>> cat >dummy.mak <<EOF
>> include Makefile
>>
>> blub: $(OBJECTS)
>> 	do-something-with $^
>> EOF
>>
>> make -f dummy.mak blub
> 
> Hacky but clever. I like it.
> 
> In the particular case of git, I think I've cheated similarly before by
> putting things in config.mak, though of course an arbitrary script can't
> assume it can overwrite that file.

Heh, I actually have the following in my config.mak already:

extra-clean: clean
	find . -iname '*.o' -exec rm {} \;

But for some reason I _always_ type 'make clean' and then, to top
it off, I _always_ type the 'find' command by hand (I have no idea
why) :-D

ATB,
Ramsay Jones


^ permalink raw reply

* Re: [PATCH v3 15/25] sequencer: allow editing the commit message on a case-by-case basis
From: Junio C Hamano @ 2016-10-17 17:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <0577f66a108dced07775e10b87292c0eabfcc514.1476120229.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> In the upcoming commits, we will implement more and more of rebase -i's
> functionality inside the sequencer. One particular feature of the
> commands to come is that some of them allow editing the commit message
> while others don't, i.e. we cannot define in the replay_opts whether the
> commit message should be edited or not.
>
> Let's add a new parameter to the run_git_commit() function. Previously,
> it was the duty of the caller to ensure that the opts->edit setting
> indicates whether to let the user edit the commit message or not,
> indicating that it is an "all or nothing" setting, i.e. that the
> sequencer wants to let the user edit *all* commit message, or none at
> all. In the upcoming rebase -i mode, it will depend on the particular
> command that is currently executed, though.

Makes tons of sense.

^ permalink raw reply

* Re: [PATCH v3 14/25] sequencer: introduce a helper to read files written by scripts
From: Junio C Hamano @ 2016-10-17 17:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt
In-Reply-To: <b554346c600fafa71d2a3461fda8402e377b596e.1476120229.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +/*
> + * Reads a file that was presumably written by a shell script, i.e.
> + * with an end-of-line marker that needs to be stripped.
> + *
> + * Returns 1 if the file was read, 0 if it could not be read or does not exist.
> + */
> +static int read_oneliner(struct strbuf *buf,
> +	const char *path, int skip_if_empty)
> +...
> +	if (strbuf_read_file(buf, path, 0) < 0) {
> +		warning_errno(_("could not read '%s'"), path);
> +		return 0;
> +	}
> +	if (buf->len > orig_len && buf->buf[buf->len - 1] == '\n') {
> +		if (--buf->len > orig_len && buf->buf[buf->len - 1] == '\r')
> +			--buf->len;
> +		buf->buf[buf->len] = '\0';
> +	}

The name says "oneliner" but this reads the whole thing and trims
only the last line of the input.  Which is correct?

Do we want to error out if we got more than one line?  That makes it
more strict.  Going in the other direction, do we want to just read
the first line and ignore the remainder?  That allows users to leave
cruft after what matters.  I _think_ the existing code is closer to
the latter, i.e. something along the lines of ...

	struct strbuf oneline = STRBUF_INIT;
	FILE *fp = fopen(path, "r");
	if (!fp) {
		warning_errno(_("could not open '%s'"), path);
		return 0;
	}
	if (strbuf_getline(&oneline, fp) < 0)
		; /* EOF - empty */
	else {
		strbuf_addbuf(buf, &oneline);
	}

^ permalink raw reply

* Re: [PATCH] convert: mark a file-local symbol static
From: Ramsay Jones @ 2016-10-17 17:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Lars Schneider, Junio C Hamano, GIT Mailing-list
In-Reply-To: <20161017021825.jgpsew7qqfjrirhw@sigill.intra.peff.net>



On 17/10/16 03:18, Jeff King wrote:
> On Mon, Oct 17, 2016 at 02:37:58AM +0100, Ramsay Jones wrote:
> 
>> Hmm, well, you have to remember that 'make clean' sometimes
>> doesn't make clean. Ever since the Makefile was changed to only
>> remove $(OBJECTS), rather than *.o xdiff/*.o etc., you have to
>> remember to 'make clean' _before_ you switch branches. Otherwise,
>> you risk leaving some objects laying around. Since the script
>> runs 'nm' on all objects it finds, any stale ones can cause problems.
>> (Of course, I almost always forget, so I frequently have to manually
>> check for and remove stale objects!)
> 
> Gross. I would not be opposed to a Makefile rule that outputs the
> correct set of OBJECTS so this (or other) scripts could build on it.
> 
> IIRC, BSD make has an option to do this "make -V OBJECTS" or something,
> but I don't thnk there's an easy way to do so.

Hmm, I would go in the opposite direction and take a leaf out of
Ævar's book (see commit bc548efe) and this one-liner:

diff --git a/Makefile b/Makefile
index ee89c06..c08c25e 100644
--- a/Makefile
+++ b/Makefile
@@ -2506,7 +2506,7 @@ profile-clean:
 
 clean: profile-clean coverage-clean
        $(RM) *.res
-       $(RM) $(OBJECTS)
+       $(RM) $(addsuffix *.o,$(object_dirs))
        $(RM) $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
        $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
        $(RM) $(TEST_PROGRAMS) $(NO_INSTALL)

This would actually solve my problem, but it actually isn't a
_complete_ solution. (Hint: think about what isn't in $(OBJECTS),
depending on the configuration). ;-)

> Or, since it seems to find useful results quite frequently, maybe it
> would be worth including the script inside git (and triggering it with
> an optional Makefile rule). It sounds like we'd need a way to annotate
> known false positives, but if it were in common use, it would be easier
> to get people to keep that list up to date.

Hmm, I suspect that wouldn't happen, which would reduce it usefulness
and ultimately lead to it not being used. (Updating the 'stop list' would
fast become a burden.)

I find it useful to flag these issues automatically, but I still need
to look at each symbol and decide what to do (you may not agree with
some of my choices either - take a look at the output on the master
branch!).

The way I use it, I effectively ignore the 'stop list' maintenance issues.

ATB,
Ramsay Jones



^ permalink raw reply related

* Re: What's cooking in git.git (Oct 2016, #03; Tue, 11)
From: Junio C Hamano @ 2016-10-17 17:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <alpine.DEB.2.20.1610151020480.197091@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> I'll mark it as "wait for follow-up fix" in whats-cooking.txt (on
>> 'todo' branch) to remind myself not to merge it yet.
>
> May I request your guidance as to your preference how to proceed?
> ...

I guess I didn't see this before I sent my response to the review
thread, which was in my pile of "these need more thought than others
before responding" topics.  

> Here are the options I see:
>
> A) remove the tests in question
>
> B) mark them as !MINGW instead
>
> C) change just those two tests from using `$PWD` (pseudo-Unix path) to
>   `$(pwd)` (native path)
>
> I would like to hear your feedback about your preference, but not without
> priming you a little bit by detailing my current opinion on the matter:
>
> While I think B) would be the easiest to read, C) would document the
> expected behavior better. A) would feel to me like shrugging, i.e. the
> lazy, wrong thing to do.
>
> What do you think?

As to my preference on tests, I guess what I suggested was a cross
between your B and C below, and I can go with either one as an
abbreviated version of my preference ;-) 

I am still wondering if the test is expecting the right behaviour,
though.  If some codepaths rely on a question "please resolve '../.'
relative to 'path/to/dir/.'" being answered as "that's path/to/dir
itself", it smells to me that the downstream of the dataflow that
expects such an answer, as well as the machinery that produces such
an answer, are acting as two wrongs that happen to cancel each
other.  Am I grossly misunderstanding what that test is doing?


^ permalink raw reply

* Re: [PATCH v10 13/14] convert: add filter.<driver>.process option
From: Junio C Hamano @ 2016-10-17 17:05 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Lars Schneider, git, Jakub Narębski, peff
In-Reply-To: <20161011100946.GA13745@tb-raspi>

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

>> +test_cmp_count () {
>> +	expect=$1 actual=$2
>
> That could be 
> expect="$1"
> actual="$2"

Yes, but it does not have to ;-).

^ permalink raw reply

* Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
From: Junio C Hamano @ 2016-10-17 16:54 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Jakub Narębski, Git Mailing List
In-Reply-To: <CACBZZX4FK+zrVyBTpbJAbSAxQ9LuCTXcExeEqZE6D3nwHbNxZA@mail.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> As far as I can tell the only outstanding "change this" is your
> s/SHA1/SHA-1/ in <xmqq37k9jm86.fsf@gitster.mtv.corp.google.com>, do
> you want to fix that up or should I submit another series?

I think I did that already myself while queuing.  Could you fetch
what I queued on 'pu' to double check?

I think the diff between what was posted and what is queued (I just
checked) looks like this:

-gitweb: Link to 7-char+ SHA1s, not only 8-char+
+gitweb: link to 7-char+ SHA-1s, not only 8-char+

 Change the minimum length of an abbreviated object identifier in the
 commit message gitweb tries to turn into link from 8 hexchars to 7.
 
@@ -5,16 +12,18 @@
 SHA-1 in commit log message links to "object" view", 2006-12-10), but
 the default abbreviation length is 7, and has been for a long time.
 
-It's still possible to reference SHA1s down to 4 characters in length,
+It's still possible to reference SHA-1s down to 4 characters in length,
 see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make
 git actually produce that, so I doubt anyone is putting that into log
-messages in practice, but people definitely do put 7 character SHA1s
+messages in practice, but people definitely do put 7 character SHA-1s
 into log messages.
 
 I think it's fairly dubious to link to things matching [0-9a-fA-F]
 here as opposed to just [0-9a-f], that dates back to the initial
 version of gitweb from 161332a ("first working version",
-2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce
+2005-08-07). Git will accept all-caps SHA-1s, but didn't ever produce
 them as far as I can tell.
 
 Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
+Acked-by: Jakub Narębski <jnareb@gmail.com>
+Signed-off-by: Junio C Hamano <gitster@pobox.com>

^ permalink raw reply

* Re: Merge conflicts in .gitattributes can cause trouble
From: Johannes Schindelin @ 2016-10-17 16:11 UTC (permalink / raw)
  To: Lars Schneider; +Cc: git, Jeff King, me
In-Reply-To: <248A6E81-8D5C-4183-9756-51A0D5193E3E@gmail.com>

Hi Lars,

On Tue, 4 Oct 2016, Lars Schneider wrote:

> If there is a conflict in the .gitattributes during a merge then it looks 
> like as if the attributes are not applied

I tried to replicate this behavior, to the point where I wrote a patch
that demonstrates the breakage so I could single-step in a debugger to
find out where things go wrong, and fix them.

Alas, I found out that the .gitattributes are read *before* any merge
conflict arises in the case you demonstrated. Which kind of makes sense,
because the gitattributes decide over which merge driver to use, among
other things.

So in your example:

> Consider this script on Windows:
> 
> $ git init .
> $ touch first.commit
> $ git add .
> $ git commit -m "first commit"
> 
> $ git checkout -b branch
> $ printf "*.bin binary\n" >> .gitattributes
> $ git add .
> $ git commit -m "tracking *.bin files"
> 
> $ git checkout master
> $ printf "binary\ndata\n" > file.dat # <-- Unix line ending!
> $ printf "*.dat binary\n" >> .gitattributes # <-- Tell Git to keep Unix line ending!
> $ git add .
> $ git commit -m "tracking *.dat files"
> $ git cat-file -p :file.dat | od -c
> 0000000   b   i   n   a   r   y  \n   d   a   t   a  \n 
>                                 ^^^^                ^^^^  <-- Correct!
> $ git checkout branch

At this point, the .gitattributes list only .bin files as binary. That is
the revision of the .gitattributes used by this command:

> $ git merge master # <-- Causes merge conflict!

And as a consequence, the .gitattributes do not tell Git that it should
handle .dat files as binary. Which means that...

> $ printf "*.bin binary\n*.dat binary\n" > .gitattributes # <-- Fix merge conflict!
> $ git add .
> $ git commit -m "merged"
> $ git cat-file -p :file.dat | od -c
> 0000000   b   i   n   a   r   y  \r  \n   d   a   t   a  \r  \n
>                                 ^^^^^^^^                ^^^^^^^^  <-- Wrong!

... this is actually expected! Why? Because the .gitattributes that were
in effect when the user asked to perform a merge said so.

If you adjust .gitattributes *before* merging `master`, it works as you
would expect: the line endings are not changed.

The reason to do it this way: we want to respect the .gitattributes as per
the current worktree. We go even so far that we respect uncommitted
changes to said file...

> Possible solutions:
> 
> 1. We could print an appropriate warning if we detect a merge conflict 
>    in .gitattributes
> 
> 2. We could disable all line ending conversions in case of a merge conflict
>    (I am not exactly sure about all the implications, though)
> 
> 3. We could salvage what we could of the .gitattributes file, 
>    perhaps by using the version from HEAD (or more likely, the ours stage of
>    the index) -- suggested by Peff on the related GitHub issue mentioned below

I would vote for:

4. We keep letting Git read in the *current* version of .gitattributes
   *before* the merge, and apply those attributes while performing the
   merge.

Ciao,
Dscho

^ permalink raw reply

* RE: Uninitialized submodules as symlinks
From: David Turner @ 2016-10-17 15:12 UTC (permalink / raw)
  To: 'Duy Nguyen'; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <CACsJy8BAB4sqnv1GBk=K9yUuqVhW4Y=CHdON7mnM9PY08HwdGw@mail.gmail.com>

> -----Original Message-----
> From: Duy Nguyen [mailto:pclouds@gmail.com]
> Sent: Monday, October 17, 2016 5:46 AM
> To: David Turner
> Cc: Stefan Beller; git@vger.kernel.org
> Subject: Re: Uninitialized submodules as symlinks
> 
> On Sat, Oct 8, 2016 at 2:59 AM, David Turner <David.Turner@twosigma.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stefan Beller [mailto:sbeller@google.com]
> >> Sent: Friday, October 07, 2016 2:56 PM
> >> To: David Turner
> >> Cc: git@vger.kernel.org
> >> Subject: Re: Uninitialized submodules as symlinks
> >>
> >> On Fri, Oct 7, 2016 at 11:17 AM, David Turner
> >> <David.Turner@twosigma.com>
> >> wrote:
> >> > Presently, uninitialized submodules are materialized in the working
> >> > tree as empty directories.
> >>
> >> Right, there has to be something, to hint at the user that creating a
> >> file with that path is probably not what they want.
> >>
> >> >  We would like to consider having them be symlinks.  Specifically,
> >> > we'd like them to be symlinks into a FUSE filesystem which
> >> > retrieves files on demand.
> >> >
> >> > We've actually already got a FUSE filesystem written, but we use a
> >> > different (semi-manual) means to connect it to the initialized
> submodules.
> >>
> >> So you currently do a
> >>
> >>     git submodule init <pathspec>
> >>     custom-submodule make-symlink <pathspec>
> >>
> >> ?
> >
> > We do something like
> >
> > For each initialized submodule: symlink it into the right place in
> > .../somedir For each uninitialized submodule: symlink from the FUSE
> > into the right place in .../somedir
> >
> > So .../somedir has the structure of the git main repo, but is all
> symlinks -- some into FUSE, some into the git repo.
> >
> > This means that when we initialize (or deinitialize) a submodule, we
> need to re-run the linking script.
> 
> Do .git files work? If .git files point to somewhere in fuse, I guess you
> still have file retrieval on demand. It depends on what files to retrieve
> I guess. If you want worktree files, not object database then .git files
> won't work because worktree remains in the same filesystem as the super
> repo.

Yes, we want worktree files (or even worktree files + built artifacts).

^ permalink raw reply

* RE: Uninitialized submodules as symlinks
From: David Turner @ 2016-10-17 15:11 UTC (permalink / raw)
  To: 'Heiko Voigt'; +Cc: git@vger.kernel.org
In-Reply-To: <20161013161017.GC29710@book.hvoigt.net>



> -----Original Message-----
> From: Heiko Voigt [mailto:hvoigt@hvoigt.net]
> Sent: Thursday, October 13, 2016 12:10 PM
> To: David Turner
> Cc: git@vger.kernel.org
> Subject: Re: Uninitialized submodules as symlinks
> 
> On Fri, Oct 07, 2016 at 06:17:05PM +0000, David Turner wrote:
> > Presently, uninitialized submodules are materialized in the working
> > tree as empty directories.  We would like to consider having them be
> > symlinks.  Specifically, we'd like them to be symlinks into a FUSE
> > filesystem which retrieves files on demand.
> 
> How about portability? This feature would only work on Unix like operating
> systems. You have to be careful to not break Windows since they do not
> have symlinks.

Windows doesn't support FUSE either IIRC.  Since this would be an alternate mode of operation, Windows would still work fine on the old model.

^ permalink raw reply

* [PATCH 1/4] i18n: apply: mark error message for translation
From: Vasco Almeida @ 2016-10-17 13:15 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA

Update test to reflect changes.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 apply.c               | 4 ++--
 t/t4254-am-corrupt.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/apply.c b/apply.c
index 8215874..705cf56 100644
--- a/apply.c
+++ b/apply.c
@@ -1586,8 +1586,8 @@ static int find_header(struct apply_state *state,
 				patch->new_name = xstrdup(patch->def_name);
 			}
 			if (!patch->is_delete && !patch->new_name) {
-				error("git diff header lacks filename information "
-					     "(line %d)", state->linenr);
+				error(_("git diff header lacks filename information "
+					     "(line %d)"), state->linenr);
 				return -128;
 			}
 			patch->is_toplevel_relative = 1;
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
index 9bd7dd2..168739c 100755
--- a/t/t4254-am-corrupt.sh
+++ b/t/t4254-am-corrupt.sh
@@ -31,7 +31,7 @@ test_expect_success 'try to apply corrupted patch' '
 test_expect_success 'compare diagnostic; ensure file is still here' '
 	echo "error: git diff header lacks filename information (line 4)" >expected &&
 	test_path_is_file f &&
-	test_cmp expected actual
+	test_i18ncmp expected actual
 '
 
 test_done
-- 
2.10.1.459.g5fd885d


^ permalink raw reply related

* [PATCH 3/4] i18n: credential-cache--daemon: mark advice for translation
From: Vasco Almeida @ 2016-10-17 13:15 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <20161017131529.27856-1-vascomalmeida@sapo.pt>

Mark permissions_advice for translation.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 credential-cache--daemon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index 1e5f16a..46c5937 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -219,11 +219,11 @@ static void serve_cache(const char *socket_path, int debug)
 	close(fd);
 }
 
-static const char permissions_advice[] =
+static const char permissions_advice[] = N_(
 "The permissions on your socket directory are too loose; other\n"
 "users may be able to read your cached credentials. Consider running:\n"
 "\n"
-"	chmod 0700 %s";
+"	chmod 0700 %s");
 static void init_socket_directory(const char *path)
 {
 	struct stat st;
@@ -232,7 +232,7 @@ static void init_socket_directory(const char *path)
 
 	if (!stat(dir, &st)) {
 		if (st.st_mode & 077)
-			die(permissions_advice, dir);
+			die(_(permissions_advice), dir);
 	} else {
 		/*
 		 * We must be sure to create the directory with the correct mode,
-- 
2.10.1.459.g5fd885d


^ permalink raw reply related

* [PATCH 2/4] i18n: convert mark error messages for translation
From: Vasco Almeida @ 2016-10-17 13:15 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <20161017131529.27856-1-vascomalmeida@sapo.pt>

Mark error messages about CRLF for translation.

Update test to reflect changes.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 convert.c       | 12 ++++++++----
 t/t0020-crlf.sh |  6 +++++-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/convert.c b/convert.c
index 077f5e6..0ad39b1 100644
--- a/convert.c
+++ b/convert.c
@@ -197,17 +197,21 @@ static void check_safe_crlf(const char *path, enum crlf_action crlf_action,
 		 * CRLFs would not be restored by checkout
 		 */
 		if (checksafe == SAFE_CRLF_WARN)
-			warning("CRLF will be replaced by LF in %s.\nThe file will have its original line endings in your working directory.", path);
+			warning(_("CRLF will be replaced by LF in %s.\n"
+				  "The file will have its original line"
+				  " endings in your working directory."), path);
 		else /* i.e. SAFE_CRLF_FAIL */
-			die("CRLF would be replaced by LF in %s.", path);
+			die(_("CRLF would be replaced by LF in %s."), path);
 	} else if (old_stats->lonelf && !new_stats->lonelf ) {
 		/*
 		 * CRLFs would be added by checkout
 		 */
 		if (checksafe == SAFE_CRLF_WARN)
-			warning("LF will be replaced by CRLF in %s.\nThe file will have its original line endings in your working directory.", path);
+			warning(_("LF will be replaced by CRLF in %s.\n"
+				  "The file will have its original line"
+				  " endings in your working directory."), path);
 		else /* i.e. SAFE_CRLF_FAIL */
-			die("LF would be replaced by CRLF in %s", path);
+			die(_("LF would be replaced by CRLF in %s"), path);
 	}
 }
 
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index f94120a..71350e0 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -83,7 +83,11 @@ test_expect_success 'safecrlf: print warning only once' '
 	git add doublewarn &&
 	git commit -m "nowarn" &&
 	for w in Oh here is CRLFQ in text; do echo $w; done | q_to_cr >doublewarn &&
-	test $(git add doublewarn 2>&1 | grep "CRLF will be replaced by LF" | wc -l) = 1
+	git add doublewarn 2>err &&
+	if test_have_prereq C_LOCALE_OUTPUT
+	then
+		test $(grep "CRLF will be replaced by LF" err | wc -l) = 1
+	fi
 '
 
 
-- 
2.10.1.459.g5fd885d


^ permalink raw reply related

* [PATCH 4/4] i18n: diff: mark warnings for translation
From: Vasco Almeida @ 2016-10-17 13:15 UTC (permalink / raw)
  To: git
  Cc: Vasco Almeida, Jiang Xin, Ævar Arnfjörð Bjarmason,
	Jean-Noël AVILA
In-Reply-To: <20161017131529.27856-1-vascomalmeida@sapo.pt>

Mark rename_limit_warning and degrade_cc_to_c_warning and
rename_limit_warning for translation.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
---
 diff.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index 1d304e0..1687317 100644
--- a/diff.c
+++ b/diff.c
@@ -4638,25 +4638,25 @@ static int is_summary_empty(const struct diff_queue_struct *q)
 }
 
 static const char rename_limit_warning[] =
-"inexact rename detection was skipped due to too many files.";
+N_("inexact rename detection was skipped due to too many files.");
 
 static const char degrade_cc_to_c_warning[] =
-"only found copies from modified paths due to too many files.";
+N_("only found copies from modified paths due to too many files.");
 
 static const char rename_limit_advice[] =
-"you may want to set your %s variable to at least "
-"%d and retry the command.";
+N_("you may want to set your %s variable to at least "
+   "%d and retry the command.");
 
 void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
 {
 	if (degraded_cc)
-		warning(degrade_cc_to_c_warning);
+		warning(_(degrade_cc_to_c_warning));
 	else if (needed)
-		warning(rename_limit_warning);
+		warning(_(rename_limit_warning));
 	else
 		return;
 	if (0 < needed && needed < 32767)
-		warning(rename_limit_advice, varname, needed);
+		warning(_(rename_limit_advice), varname, needed);
 }
 
 void diff_flush(struct diff_options *options)
-- 
2.10.1.459.g5fd885d


^ permalink raw reply related

* Re: Automagic `git checkout branchname` mysteriously fails
From: Duy Nguyen @ 2016-10-17 11:39 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Kevin Daudt, Git
In-Reply-To: <CACPiFCJFXGmsJX97kQuA2h3trVX7L3SacwKrD4mpEU5SPxkLAA@mail.gmail.com>

On Sat, Oct 15, 2016 at 4:06 AM, Martin Langhoff
<martin.langhoff@gmail.com> wrote:
> On Fri, Oct 14, 2016 at 4:58 PM, Kevin Daudt <me@ikke.info> wrote:
>> Correct, this only works when it's unambiguous what branch you actually
>> mean.
>
> That's not surprising, but there isn't a warning. IMHO, finding
> several branch matches is a strong indication that it'll be worth
> reporting to the user that the DWIM machinery got hits, but couldn't
> work it out.
>
> I get that process is not geared towards making a friendly msg easy,
> but we could print to stderr something like:
>
>  "branch" matches more than one candidate ref, cannot choose automatically.
>  If you mean to check out a branch, try git branch command.
>  If you mean to check out a file, use -- before the pathname to
>  disambiguate.

Or even better, list all ambiguous candidates like Jeff did for
ambiguous short SHA-1 in 1ffa26c (get_short_sha1: list ambiguous
objects on error - 2016-09-26).There were a few occasions I was
confused by ambiguous refs and displaying them all would help me see
what problem was much faster.
-- 
Duy

^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Duy Nguyen @ 2016-10-17 11:02 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <alpine.DEB.2.20.1610171241240.197091@virtualbox>

On Mon, Oct 17, 2016 at 5:46 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Duy,
>
> On Mon, 17 Oct 2016, Duy Nguyen wrote:
>
>> On Mon, Oct 17, 2016 at 3:57 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Hi Stefan,
>> >
>> > On Sun, 16 Oct 2016, Stefan Beller wrote:
>> >
>> >> Conceptually I would prefer if we had a single switch that indicates a
>> >> case insensitive FS.
>> >
>> > AFAIU Lars' use case is where the FS is *case sensitive*, but he still
>> > needs the .gitattributes to be *case insensitive* because that file
>> > originates from a developer with such a file system.
>> >
>> > Otherwise he would simply tack onto the core.ignoreCase flag.
>>
>> That sounds to me like setting core.ignoreCase to true (on all devs'
>> repo) would "solve" this.
>
> It is good that you quoted this verb, because it does not solve things.
> Instead, it would try to use the flag for two slightly incompatible
> purposes at the same time.
>
> The first (and so far, only) purpose is to tell Git that the current file
> system is case insensitive.
>
> The new purpose you described would be to tell Git that the *user* does
> not care about the file names' case, even if the file system does.
>
> I do not think that this leads to a better situation than before. Instead,
> I am convinced that it will cause new and sometimes "entertaining"
> problems because you can no longer discern between those two purposes
> based on core.ignoreCase, you would have to teach Git to test every single
> time whether the file system is case-sensitive or not.

I agree. Which is why I wrote "we probably want something in the same
spirit but limited to .gitattributes and .gitignore only". In other
words we could have core.someName that makes .gitattributes and
.gitignore patterns case-insensitive (or core-sensitive). If it's
present, it overrides core.ignoreCase. If it's not present,
core.ignoreCase decides. I'm just not sure if the new config should
cover everything involving filename's case in git. That's too big to
fit in my head.

> Needless to say, I'd rather not see that happening. Many users, including
> my colleagues and myself, rely on Git being a rock solid piece of
> software, and that change would make it less so.
-- 
Duy

^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Johannes Schindelin @ 2016-10-17 10:46 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <CACsJy8BBLcZvB1FswcEKS1KgvjMjo_uaVqOTgjmMJkjnmoye+w@mail.gmail.com>

Hi Duy,

On Mon, 17 Oct 2016, Duy Nguyen wrote:

> On Mon, Oct 17, 2016 at 3:57 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi Stefan,
> >
> > On Sun, 16 Oct 2016, Stefan Beller wrote:
> >
> >> Conceptually I would prefer if we had a single switch that indicates a
> >> case insensitive FS.
> >
> > AFAIU Lars' use case is where the FS is *case sensitive*, but he still
> > needs the .gitattributes to be *case insensitive* because that file
> > originates from a developer with such a file system.
> >
> > Otherwise he would simply tack onto the core.ignoreCase flag.
> 
> That sounds to me like setting core.ignoreCase to true (on all devs'
> repo) would "solve" this.

It is good that you quoted this verb, because it does not solve things.
Instead, it would try to use the flag for two slightly incompatible
purposes at the same time.

The first (and so far, only) purpose is to tell Git that the current file
system is case insensitive.

The new purpose you described would be to tell Git that the *user* does
not care about the file names' case, even if the file system does.

I do not think that this leads to a better situation than before. Instead,
I am convinced that it will cause new and sometimes "entertaining"
problems because you can no longer discern between those two purposes
based on core.ignoreCase, you would have to teach Git to test every single
time whether the file system is case-sensitive or not.

Needless to say, I'd rather not see that happening. Many users, including
my colleagues and myself, rely on Git being a rock solid piece of
software, and that change would make it less so.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v1 1/2] config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11
From: Jeff King @ 2016-10-17  9:50 UTC (permalink / raw)
  To: larsxschneider; +Cc: git, tboegi, gitster
In-Reply-To: <20161017002550.88782-2-larsxschneider@gmail.com>

On Sun, Oct 16, 2016 at 05:25:49PM -0700, larsxschneider@gmail.com wrote:

> From: Lars Schneider <larsxschneider@gmail.com>
> 
> Apple removed the OpenSSL header files in macOS 10.11 and above. OpenSSL
> was deprecated since macOS 10.7.
> 
> Set `NO_OPENSSL` and `APPLE_COMMON_CRYPTO` to `YesPlease` as default for
> macOS. Make it possible to override this and use OpenSSL by defining
> `DARWIN_OPENSSL`.

I like that you gave an override, but I don't think it works in all
cases:

> diff --git a/config.mak.uname b/config.mak.uname
> index b232908..f0c94a9 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -108,6 +108,12 @@ ifeq ($(uname_S),Darwin)
>  	ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 11 && echo 1),1)
>  		HAVE_GETDELIM = YesPlease
>  	endif
> +	ifeq ($(shell test "`expr "$(uname_R)" : '\([0-9][0-9]*\)\.'`" -ge 15 && echo 1),1)
> +		ifndef DARWIN_OPENSSL
> +			NO_OPENSSL = YesPlease
> +			APPLE_COMMON_CRYPTO=YesPlease
> +		endif
> +	endif

This is in config.mak.uname, which gets sourced before config.mak (and
ifndef is evaluated at the time of parsing). So it would work to do:

  make DARWIN_OPENSSL=Yep

but not:

  echo DARWIN_OPENSSL=Yep >>config.mak
  make

I think you'd have to set a flag in config.mak.uname, and then resolve
it in the Makefile proper like:

  ifdef DARWIN_OPENSSL
	# Overrides AUTO_AVOID_OPENSSL, do nothing.
  else ifdef AUTO_AVOID_OPENSSL
	NO_OPENSSL = YesPlease
	APPLE_COMMON_CRYPTO = YesPlease
  endif

but that's totally untested.

-Peff

^ permalink raw reply

* Re: Uninitialized submodules as symlinks
From: Duy Nguyen @ 2016-10-17  9:45 UTC (permalink / raw)
  To: David Turner; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <f0c11de1bd4d414d8a9464e6ccc1d58a@exmbdft7.ad.twosigma.com>

On Sat, Oct 8, 2016 at 2:59 AM, David Turner <David.Turner@twosigma.com> wrote:
>
>
>> -----Original Message-----
>> From: Stefan Beller [mailto:sbeller@google.com]
>> Sent: Friday, October 07, 2016 2:56 PM
>> To: David Turner
>> Cc: git@vger.kernel.org
>> Subject: Re: Uninitialized submodules as symlinks
>>
>> On Fri, Oct 7, 2016 at 11:17 AM, David Turner <David.Turner@twosigma.com>
>> wrote:
>> > Presently, uninitialized submodules are materialized in the working tree
>> > as empty directories.
>>
>> Right, there has to be something, to hint at the user that creating a file
>> with that path is probably not what they want.
>>
>> >  We would like to consider having them be symlinks.  Specifically, we'd
>> > like them to be symlinks into a FUSE filesystem which retrieves files on
>> > demand.
>> >
>> > We've actually already got a FUSE filesystem written, but we use a
>> > different (semi-manual) means to connect it to the initialized submodules.
>>
>> So you currently do a
>>
>>     git submodule init <pathspec>
>>     custom-submodule make-symlink <pathspec>
>>
>> ?
>
> We do something like
>
> For each initialized submodule: symlink it into the right place in .../somedir
> For each uninitialized submodule: symlink from the FUSE into the right place in .../somedir
>
> So .../somedir has the structure of the git main repo, but is all symlinks -- some into FUSE, some into the git repo.
>
> This means that when we initialize (or deinitialize) a submodule, we need to re-run the linking script.

Do .git files work? If .git files point to somewhere in fuse, I guess
you still have file retrieval on demand. It depends on what files to
retrieve I guess. If you want worktree files, not object database then
.git files won't work because worktree remains in the same filesystem
as the super repo.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] convert: mark a file-local symbol static
From: Jeff King @ 2016-10-17  9:37 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Ramsay Jones, Lars Schneider, Junio C Hamano, GIT Mailing-list
In-Reply-To: <alpine.DEB.2.20.1610171058580.197091@virtualbox>

On Mon, Oct 17, 2016 at 11:04:19AM +0200, Johannes Schindelin wrote:

> > Gross. I would not be opposed to a Makefile rule that outputs the
> > correct set of OBJECTS so this (or other) scripts could build on it.
> 
> You could also use the method I use in Git for Windows to "extend" the
> Makefile:
> 
> -- snipsnap --
> cat >dummy.mak <<EOF
> include Makefile
> 
> blub: $(OBJECTS)
> 	do-something-with $^
> EOF
> 
> make -f dummy.mak blub

Hacky but clever. I like it.

In the particular case of git, I think I've cheated similarly before by
putting things in config.mak, though of course an arbitrary script can't
assume it can overwrite that file.

-Peff

^ permalink raw reply

* Re: [PATCH v3 07/25] sequencer: completely revamp the "todo" script parsing
From: Jeff King @ 2016-10-17  9:36 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Torsten Bögershausen, git, Jakub Narębski,
	Johannes Sixt
In-Reply-To: <alpine.DEB.2.20.1610171025470.197091@virtualbox>

[tl;dr: the version in your repo is fine, and there's a trivial fix
 below if we want to silence the warning in the meantime]

On Mon, Oct 17, 2016 at 10:37:52AM +0200, Johannes Schindelin wrote:

> > I'm not sure I agree. IIRC, Assigning values outside the range of an enum has
> > always been fishy according to the standard, and a compiler really is
> > allowed to allocate a single bit for storage for this enum.
> 
> Really? I did see my share of code that completely violated this freedom,
> by assuming that it was really okay to cast a -1 to an enum and then test
> for that value later, when -1 was not a legal enum value.

I poked around a bit, and it seems we're both half-wrong. C99 says:

  6.7.2.2 Enumeration specifiers
  [...]
  The expression that defines the value of an enumeration constant shall
  be an integer constant expression that has a value representable as an
  int.
  [...]
  Each enumerated type shall be compatible with char, a signed integer
  type, or an unsigned integer type. The choice of type is
  implementation-defined, but shall be capable of representing the
  values of all the members of the enumeration.

My reading is that it can't be a single-bit bitfield as I claimed, but
it also isn't necessarily interchangeable with an int. But you get at
least a "char", and you can use all of those integer values even if they
aren't explicitly part of the set. And I'd assume that goes for values
even beyond the largest tag as long as you don't need more bits, so
that:

  enum { A = 1, B = 2, C = 4 } x = A | B | C;

is OK (though I didn't see anything particularly about that in the
standard).

Assigning "-1" works in the same way that normal "unsigned x = -1" works
(and is defined by the standard), though of course it may unexpectedly
conflict with an actual enum value if the compiler chooses a smaller
type (e.g., it may literally be 255 in many cases).

Anyway. Enough language lawyering. It seems like clang is being overly
strict in its interpretation of the standard (it should be giving us at
least a char's worth of values). But it matters less what the standard
says and more what real compilers do, and we have to deal with clang's
behavior.

> In any case, the fact that even one compiler used to build Git *may*
> violate that standard, and that we therefore need such safety guards as
> the one under discussion, still makes me think that this warning, while
> certainly well-intentioned, is poison for cross-platform projects.

Oh, I agree that the warning is annoying, and the code should not go
away. We just need to figure out how to silence clang.

> > I'm happy to test the TODO_NOOP version against clang (and prepare a
> > patch on top if it still complains), but that doesn't seem to have
> > Junio's tree at all yet.
> 
> Junio chose to pick up only one patch series out of the rebase--helper
> thicket at a time, it seems. I did send out at least one revision per
> patch series prior to integrating them into Git for Windows v2.10.0,
> though. Plus, I kept updating the `interactive-rebase` branch in my
> repository on GitHub (https://github.com/dscho/git).

Thanks, I was able to test that branch. It looks like clang is happy
with it because you compare against the max value. Unlike the
ARRAY_SIZE() check, this does mean if somebody modifies the enum without
touching the array, we might go out of bounds. But things would be
severely broken enough from the mismatch that I don't think it's worth
worrying about too much (and I see you have a nice comment warning
people about this).

If the rest of your interactive-rebase branch is coming soon, I think we
can probably ignore it for now. Otherwise something like:

diff --git a/sequencer.c b/sequencer.c
index d662c6b..1fdc35e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -620,7 +620,8 @@ static int allow_empty(struct replay_opts *opts, struct commit *commit)
 
 enum todo_command {
 	TODO_PICK = 0,
-	TODO_REVERT
+	TODO_REVERT,
+	TODO_MAX
 };
 

is probably the simplest portable fix.

As a more clever change, I wondered if switching the enum values from
(0,1) to (1,2) would silence the warning, and indeed it does. Which I
assume is because using bit-flags, we could now represent "1|2", or "3",
which is larger than the array (well, obviously "2" is, but we'd need to
subtract 1 when indexing the array). I don't think that's a good route,
though, because it loses the 0-indexing, the benefits of
zero-initialization, etc. I was mostly just poking at how clang
perceives the enum values.

> P.S.: I cannot wait for the day when somebody with an artistic touch
> provides .css for the public-inbox.org site so it stops threatening
> causing eye cancer to me.

Heh. I gently hinted something similar to Eric in the past, but I think
he actually likes how it looks. He has invited others to mirror
public-inbox and make their own interface, though. I just lack the
"artistic touch" you mentioned.

-Peff

^ permalink raw reply related

* Re: Uninitialized submodules as symlinks
From: Heiko Voigt @ 2016-10-17  9:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kevin Daudt, David Turner, git@vger.kernel.org
In-Reply-To: <xmqqr37izxkf.fsf@gitster.mtv.corp.google.com>

On Fri, Oct 14, 2016 at 09:48:16AM -0700, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
> 
> > On Thu, Oct 13, 2016 at 06:10:17PM +0200, Heiko Voigt wrote:
> >> On Fri, Oct 07, 2016 at 06:17:05PM +0000, David Turner wrote:
> >> > Presently, uninitialized submodules are materialized in the working
> >> > tree as empty directories.  We would like to consider having them be
> >> > symlinks.  Specifically, we'd like them to be symlinks into a FUSE
> >> > filesystem which retrieves files on demand.
> >> 
> >> How about portability? This feature would only work on Unix like
> >> operating systems. You have to be careful to not break Windows since
> >> they do not have symlinks.
> >
> > NTFS does have symlinks, but you need admin right to create them though
> > (unless you change the policy).
> 
> That sounds like saying "It has, but it practically is not usable by
> Git as a mechanism to achieve this goal" to me.

Yes and that is why Git for Windows does not use them and I simplified
to: "Windows does not have symlinks". For a normal user there is no such
thing as symlinks on Windows, unfortunately.

Cheers Heiko

^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Duy Nguyen @ 2016-10-17  9:12 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <alpine.DEB.2.20.1610171056170.197091@virtualbox>

On Mon, Oct 17, 2016 at 3:57 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Stefan,
>
> On Sun, 16 Oct 2016, Stefan Beller wrote:
>
>> Conceptually I would prefer if we had a single switch that indicates a
>> case insensitive FS.
>
> AFAIU Lars' use case is where the FS is *case sensitive*, but he still
> needs the .gitattributes to be *case insensitive* because that file
> originates from a developer with such a file system.
>
> Otherwise he would simply tack onto the core.ignoreCase flag.

That sounds to me like setting core.ignoreCase to true (on all devs'
repo) would "solve" this. Yes core.ignoreCase may introduce some side
effects when used on case-sensitive filesystems, so we probably want
something in the same spirit but limited to .gitattributes and
.gitignore only.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] convert: mark a file-local symbol static
From: Johannes Schindelin @ 2016-10-17  9:04 UTC (permalink / raw)
  To: Jeff King; +Cc: Ramsay Jones, Lars Schneider, Junio C Hamano, GIT Mailing-list
In-Reply-To: <20161017021825.jgpsew7qqfjrirhw@sigill.intra.peff.net>

Hi Ramsay & Peff,

On Sun, 16 Oct 2016, Jeff King wrote:

> On Mon, Oct 17, 2016 at 02:37:58AM +0100, Ramsay Jones wrote:
> 
> > Hmm, well, you have to remember that 'make clean' sometimes doesn't
> > make clean. Ever since the Makefile was changed to only remove
> > $(OBJECTS), rather than *.o xdiff/*.o etc., you have to remember to
> > 'make clean' _before_ you switch branches. Otherwise, you risk leaving
> > some objects laying around. Since the script runs 'nm' on all objects
> > it finds, any stale ones can cause problems.  (Of course, I almost
> > always forget, so I frequently have to manually check for and remove
> > stale objects!)
> 
> Gross. I would not be opposed to a Makefile rule that outputs the
> correct set of OBJECTS so this (or other) scripts could build on it.

You could also use the method I use in Git for Windows to "extend" the
Makefile:

-- snipsnap --
cat >dummy.mak <<EOF
include Makefile

blub: $(OBJECTS)
	do-something-with $^
EOF

make -f dummy.mak blub

^ permalink raw reply

* Re: [RFC] Case insensitive Git attributes
From: Johannes Schindelin @ 2016-10-17  8:57 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Lars Schneider, git
In-Reply-To: <CAGZ79kZ6KaQ5gjGiEFQ-pRJCDAyS0oH=_4dK0nCU9hx8wZwdfw@mail.gmail.com>

Hi Stefan,

On Sun, 16 Oct 2016, Stefan Beller wrote:

> Conceptually I would prefer if we had a single switch that indicates a
> case insensitive FS.

AFAIU Lars' use case is where the FS is *case sensitive*, but he still
needs the .gitattributes to be *case insensitive* because that file
originates from a developer with such a file system.

Otherwise he would simply tack onto the core.ignoreCase flag.

Ciao,
Dscho

^ 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