fast-import should not care about core.ignorecase

fast-import should not care about core.ignorecase

[Mike Hommey]

Hi,

As you now know, I'm working on a mercurial remote helper for git. As
such, it uses fast-import.

In the mercurial history of mozilla-central, there have been various
renames of files with only case changes, and it so happens that my
remote helper blows things up on case insensitive file systems. The
reason is git clone probing the file system and setting core.ignorecase
appropriately.

While it makes sense for checkouts and local commits, it doesn't make
sense to me that using git fast-import with the same import script would
have a different behavior depending on whether the file system is
case-sensitive or not.

Reduced testcase:

$ git init
$ git fast-import <<EOF
blob
mark :1
data 2
a

commit refs/FOO
committer <foo@foo> 0 +0000
data 0

M 644 :1 a

commit refs/FOO
committer <foo@foo> 0 +0000
data 0

R a A
EOF

This is what you get on a case sensitive FS:

$ git log refs/FOO -p -M
commit be1497308f30f883343eefd0da7ddf1e747133f8
Author:  <foo@foo>
Date:   Thu Jan 1 00:00:00 1970 +0000

diff --git a/a b/A
similarity index 100%
rename from a
rename to A

commit 8d37f958cfc0702c577b918c86769a902fe109f8
Author:  <foo@foo>
Date:   Thu Jan 1 00:00:00 1970 +0000

diff --git a/a b/a
new file mode 100644
index 0000000..7898192
--- /dev/null
+++ b/a
@@ -0,0 +1 @@
+a

This is what you get on a case insensitive FS:

$ git log refs/FOO -p -M
commit 208c0c4cf58cd54512301e0de33ccb8a78d6b226
Author:  <foo@foo>
Date:   Thu Jan 1 00:00:00 1970 +0000

commit 8d37f958cfc0702c577b918c86769a902fe109f8
Author:  <foo@foo>
Date:   Thu Jan 1 00:00:00 1970 +0000

diff --git a/a b/a
new file mode 100644
index 0000000..7898192
--- /dev/null
+++ b/a
@@ -0,0 +1 @@
+a

Note, this applies equally to filerename commands or filedelete +
filemodify combinations.

Mike

Re: fast-import should not care about core.ignorecase

[Mike Hommey]

On Tue, Dec 09, 2014 at 09:12:11AM +0900, Mike Hommey wrote:
> Hi,
> 
> As you now know, I'm working on a mercurial remote helper for git. As
> such, it uses fast-import.
> 
> In the mercurial history of mozilla-central, there have been various
> renames of files with only case changes, and it so happens that my
> remote helper blows things up on case insensitive file systems. The
> reason is git clone probing the file system and setting core.ignorecase
> appropriately.
> 
> While it makes sense for checkouts and local commits, it doesn't make
> sense to me that using git fast-import with the same import script would
> have a different behavior depending on whether the file system is
> case-sensitive or not.

Heh, I just found this thread:
http://marc.info/?t=139134708700001&r=1&w=2

It doesn't seem to have led to something actually being committed,
though.

Mike

Re: fast-import should not care about core.ignorecase

[Joshua Jensen]

Mike Hommey wrote on 12/8/2014 5:12 PM:
> While it makes sense for checkouts and local commits, it doesn't make
> sense to me that using git fast-import with the same import script would
> have a different behavior depending on whether the file system is
> case-sensitive or not.
I have used fast-import with Perforce inputs.  When you run a Windows 
Perforce server, filenames can be submitted with ANY case, but given the 
case insensitive nature of the file system, a synced Perforce file will 
end up using whatever case happens to be on the file system at that point.

That may not be clear, so here goes:

Revision 1: abc/DEF/ghi/FILE.dat

Revision 2: ABC/def/GHI/file.dat

^^ Yes, Perforce stores the filename internally in that manner and does 
not fold the case.

If you happen to sync Revision 2 on an empty directory tree, you'll get 
ABC/def/GHI/file.dat.  If you then sync Revision 1, the filename case 
remains ABC/def/GHI/file.dat.

Likewise, if you happen to sync Revision 1 into an empty directory tree, 
you'll get abc/DEF/ghi/FILE.dat.  If you then sync Revision 2, the 
filename case remains as abc/DEF/ghi/FILE.dat.

I was the one who originally submitted the patch for this some 4 years 
ago.  It was commit 50906e04e8f48215b0b09841686709b92a2ab2e4. 'git 
fast-import' with core.ignorecase=true will fold the case of the 
filename specified in Revision 2 to the case currently stored in the Git 
repository from Revision 1.

If it does not do this, then Git internally stores FILE.dat and 
file.dat, and bad things happen on case-insensitive file systems.

(Further, there are still a few paths into Git where 
core.ignorecase=true does not fold the case of the filename, and this 
can cause 'repository corruptions' on case-insensitive file systems.  
One such place is in 'git update-index' directly used by 'git gui'.  I 
really need to get this submitted, as we've been beating on it for a 
long time now, but here is the partial patch for informational purposes 
only.

I think it has been discussed before, but maybe Git needs a 
core.casefold in addition to core.ignorecase.)

-Josh

---------------------------- builtin/update-index.c 
----------------------------
index aaa6f78..4cfedc1 100644
@@ -99,6 +99,7 @@ static int add_one_path(const struct cache_entry *old, 
const char *path, int len
      memcpy(ce->name, path, len);
      ce->ce_flags = create_ce_flags(0);
      ce->ce_namelen = len;
+    fold_ce_name_case(&the_index, ce);
      fill_stat_cache_info(ce, st);
      ce->ce_mode = ce_mode_from_stat(old, st->st_mode);

@@ -234,6 +235,7 @@ static int add_cacheinfo(unsigned int mode, const 
unsigned char *sha1,
      memcpy(ce->name, path, len);
      ce->ce_flags = create_ce_flags(stage);
      ce->ce_namelen = len;
+    fold_ce_name_case(&the_index, ce);
      ce->ce_mode = create_ce_mode(mode);
      if (assume_unchanged)
          ce->ce_flags |= CE_VALID;

Re: fast-import should not care about core.ignorecase

[Jonathan Nieder]

Joshua Jensen wrote:

> I think it has been discussed before, but maybe Git needs a
> core.casefold in addition to core.ignorecase.)

Would it work for --casefold to be a commandline flag to fast-import,
instead of a global option affecting multiple Git commands?

Curious,
Jonathan

Re: fast-import should not care about core.ignorecase

[Joshua Jensen]

Jonathan Nieder wrote on 12/8/2014 6:31 PM:
> Joshua Jensen wrote:
>> I think it has been discussed before, but maybe Git needs a
>> core.casefold in addition to core.ignorecase.)
> Would it work for --casefold to be a commandline flag to fast-import,
> instead of a global option affecting multiple Git commands?
Given that core.ignorecase=true means to fold filename case in quite a 
number of places within Git right now, I would expect the same behavior 
within a repository where fast-import is being run against 
core.ignorecase=true.

So, I don't know what core.ignorecase should mean, but I'm pretty sure I 
know what core.foldcase should mean.

Would --casefold work?  Sure, but it would be a special case against the 
existing core.ignorecase behavior that I don't think makes much sense.

Josh

Re: fast-import should not care about core.ignorecase

[Junio C Hamano]

Joshua Jensen <jjensen@workspacewhiz.com> writes:

> Jonathan Nieder wrote on 12/8/2014 6:31 PM:
>> Joshua Jensen wrote:
>>> I think it has been discussed before, but maybe Git needs a
>>> core.casefold in addition to core.ignorecase.)
>> Would it work for --casefold to be a commandline flag to fast-import,
>> instead of a global option affecting multiple Git commands?
> Given that core.ignorecase=true means to fold filename case in quite a
> number of places within Git right now, I would expect the same
> behavior within a repository where fast-import is being run against
> core.ignorecase=true.
>
> So, I don't know what core.ignorecase should mean, but I'm pretty sure
> I know what core.foldcase should mean.
>
> Would --casefold work?  Sure, but it would be a special case against
> the existing core.ignorecase behavior that I don't think makes much
> sense.

I would recommend doing this:

 - Add file-scope static variable fast_import_casefold to fast-import.c

 - Patch fast-import.c where it calls strncmp_icase(a, b) to read

    fast_import_casefold ? strcasecmp(a, b) : strcmp(a, b);

 - Initialize fast_import_casefold to the same value as ignore_case.

 - Add a new command line option "--[no-]casefold" to modify the
   value of fast_import_casefold.

Once all of the above is in place, optionally do this:

 - Add fastImport.casefold configuration variable.  The
   initialization logic for fast_import_casefold would then become:

    1. If the configuration is set, use that value; otherwise
    2. Use the same value as ignore_case.

 - Start warning when fast_import_casefold is set to true only
   because ignore_case is set (i.e. no "--[no-casefold]" is given
   from the command line or from the config), saying that this is
   true for now only as a backward compatibility measure and in a
   later version of Git we will flip the default _not_ to fold on
   any system.

 - Wait a bit and then flip the default.

[PATCH] fast-import: add options to enable/disable case folding

[Mike Hommey]

Currently, fast-import does case folding depending on `core.ignorecase`.
`core.ignorecase` depends on the file system where the working tree is.
However, different kind of imports require different kinds of semantics,
and they usually aren't tied with the file system, but with the data being
imported.

Add command line options to enable or disable case folding. Also expose
them as features in the fast-import stream. Features instead of options,
because a stream that needs case folding enabled or disabled won't work
as expected if fast-import doesn't support the case folding options.
---
 Documentation/git-fast-import.txt | 11 ++++++
 fast-import.c                     | 19 ++++++++--
 t/t9300-fast-import.sh            | 79 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 690fed3..22eba87 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -50,6 +50,13 @@ OPTIONS
 	memory used by fast-import during this run.  Showing this output
 	is currently the default, but can be disabled with \--quiet.
 
+--[no-]fold-case::
+	When files/directories with the same name but a different case
+	are detected, they are treated as the same (--fold-case) or as
+	being different (--no-fold-case). The default is --fold-case
+	when `core.ignorecase` is set to `true`, and --no-fold-case when
+	it is `false`.
+
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~
 
@@ -1027,6 +1034,8 @@ date-format::
 export-marks::
 relative-marks::
 no-relative-marks::
+fold-case::
+no-fold-case::
 force::
 	Act as though the corresponding command-line option with
 	a leading '--' was passed on the command line
@@ -1091,6 +1100,8 @@ not be passed as option:
 * import-marks
 * export-marks
 * cat-blob-fd
+* fold-case
+* no-fold-case
 * force
 
 `done`
diff --git a/fast-import.c b/fast-import.c
index 6378726..958f3da 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -371,10 +371,18 @@ static volatile sig_atomic_t checkpoint_requested;
 /* Where to write output of cat-blob commands */
 static int cat_blob_fd = STDOUT_FILENO;
 
+/* Whether to enable case folding */
+static int fold_case;
+
 static void parse_argv(void);
 static void parse_cat_blob(const char *p);
 static void parse_ls(const char *p, struct branch *b);
 
+static int strncmp_foldcase(const char *a, const char *b, size_t count)
+{
+	return fold_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
+}
+
 static void write_branch_report(FILE *rpt, struct branch *b)
 {
 	fprintf(rpt, "%s:\n", b->name);
@@ -1507,7 +1515,7 @@ static int tree_content_set(
 	t = root->tree;
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
-		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
+		if (e->name->str_len == n && !strncmp_foldcase(p, e->name->str_dat, n)) {
 			if (!*slash1) {
 				if (!S_ISDIR(mode)
 						&& e->versions[1].mode == mode
@@ -1597,7 +1605,7 @@ static int tree_content_remove(
 	t = root->tree;
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
-		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
+		if (e->name->str_len == n && !strncmp_foldcase(p, e->name->str_dat, n)) {
 			if (*slash1 && !S_ISDIR(e->versions[1].mode))
 				/*
 				 * If p names a file in some subdirectory, and a
@@ -1664,7 +1672,7 @@ static int tree_content_get(
 	t = root->tree;
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
-		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
+		if (e->name->str_len == n && !strncmp_foldcase(p, e->name->str_dat, n)) {
 			if (!*slash1)
 				goto found_entry;
 			if (!S_ISDIR(e->versions[1].mode))
@@ -3246,6 +3254,10 @@ static int parse_one_feature(const char *feature, int from_stream)
 		relative_marks_paths = 1;
 	} else if (!strcmp(feature, "no-relative-marks")) {
 		relative_marks_paths = 0;
+	} else if (!strcmp(feature, "fold-case")) {
+		fold_case = 1;
+	} else if (!strcmp(feature, "no-fold-case")) {
+		fold_case = 0;
 	} else if (!strcmp(feature, "done")) {
 		require_explicit_termination = 1;
 	} else if (!strcmp(feature, "force")) {
@@ -3372,6 +3384,7 @@ int main(int argc, char **argv)
 	avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));
 	marks = pool_calloc(1, sizeof(struct mark_set));
 
+	fold_case = ignore_case;
 	global_argc = argc;
 	global_argv = argv;
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index aac126f..7057c26 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -3088,4 +3088,83 @@ test_expect_success 'U: validate root delete result' '
 	compare_diff_raw expect actual
 '
 
+cat >input <<INPUT_END
+blob
+mark :1
+data 2
+a
+
+commit refs/heads/V
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data 0
+
+M 644 :1 a
+
+commit refs/heads/V
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data 0
+
+R a A
+INPUT_END
+
+test_expect_success 'V: default case folding with ignorecase=true' '
+	git config core.ignorecase true &&
+	git fast-import <input &&
+	git ls-tree refs/heads/V >actual &&
+	git update-ref -d refs/heads/V &&
+	cat >expected <<\EOF &&
+100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	a
+EOF
+	test_cmp expected actual'
+
+test_expect_success 'V: default case folding with ignorecase=false' '
+	git config core.ignorecase false &&
+	git fast-import <input &&
+	git ls-tree refs/heads/V >actual &&
+	git update-ref -d refs/heads/V &&
+	cat >expected <<\EOF &&
+100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	A
+EOF
+	test_cmp expected actual'
+
+test_expect_success 'V: forced case folding with ignorecase=true' '
+	git config core.ignorecase true &&
+	git fast-import --fold-case <input &&
+	git ls-tree refs/heads/V >actual &&
+	git update-ref -d refs/heads/V &&
+	cat >expected <<\EOF &&
+100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	a
+EOF
+	test_cmp expected actual'
+
+test_expect_success 'V: forced case folding with ignorecase=false' '
+	git config core.ignorecase false &&
+	git fast-import --fold-case <input &&
+	git ls-tree refs/heads/V >actual &&
+	git update-ref -d refs/heads/V &&
+	cat >expected <<\EOF &&
+100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	a
+EOF
+	test_cmp expected actual'
+
+test_expect_success 'V: forced no case folding with ignorecase=true' '
+	git config core.ignorecase true &&
+	git fast-import --no-fold-case <input &&
+	git ls-tree refs/heads/V >actual &&
+	git update-ref -d refs/heads/V &&
+	cat >expected <<\EOF &&
+100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	A
+EOF
+	test_cmp expected actual'
+
+test_expect_success 'V: forced no case folding with ignorecase=false' '
+	git config core.ignorecase false &&
+	git fast-import --no-fold-case <input &&
+	git ls-tree refs/heads/V >actual &&
+	git update-ref -d refs/heads/V &&
+	cat >expected <<\EOF &&
+100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	A
+EOF
+	test_cmp expected actual'
+
 test_done
-- 
2.3.5.1.g2355df5

Re: [PATCH] fast-import: add options to enable/disable case folding

[Torsten Bögershausen]

On 04/17/2015 01:52 PM, Mike Hommey wrote:
> Currently, fast-import does case folding depending on `core.ignorecase`.
> `core.ignorecase` depends on the file system where the working tree is.
> However, different kind of imports require different kinds of semantics,
> and they usually aren't tied with the file system, but with the data being
> imported.
Good that you take up this issue, thanks for the patch
More comments inline.
> Add command line options to enable or disable case folding. Also expose
> them as features in the fast-import stream. Features instead of options,
> because a stream that needs case folding enabled or disabled won't work
> as expected if fast-import doesn't support the case folding options.
> ---
>  Documentation/git-fast-import.txt | 11 ++++++
>  fast-import.c                     | 19 ++++++++--
>  t/t9300-fast-import.sh            | 79 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index 690fed3..22eba87 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -50,6 +50,13 @@ OPTIONS
>  	memory used by fast-import during this run.  Showing this output
>  	is currently the default, but can be disabled with \--quiet.
>  
> +--[no-]fold-case::
> +	When files/directories with the same name but a different case
> +	are detected, they are treated as the same (--fold-case) or as
> +	being different (--no-fold-case). The default is --fold-case
> +	when `core.ignorecase` is set to `true`, and --no-fold-case when
> +	it is `false`.
> +
Most often the we use the term "ignore-case", could that be a better name ?
Other opinions, pros/cons  ?

>  Options for Frontends
>  ~~~~~~~~~~~~~~~~~~~~~
>  
> @@ -1027,6 +1034,8 @@ date-format::
>  export-marks::
>  relative-marks::
>  no-relative-marks::
> +fold-case::
> +no-fold-case::
>  force::
>  	Act as though the corresponding command-line option with
>  	a leading '--' was passed on the command line
> @@ -1091,6 +1100,8 @@ not be passed as option:
>  * import-marks
>  * export-marks
>  * cat-blob-fd
> +* fold-case
> +* no-fold-case
>  * force
>  
>  `done`
> diff --git a/fast-import.c b/fast-import.c
> index 6378726..958f3da 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -371,10 +371,18 @@ static volatile sig_atomic_t checkpoint_requested;
>  /* Where to write output of cat-blob commands */
>  static int cat_blob_fd = STDOUT_FILENO;
>  
> +/* Whether to enable case folding */
> +static int fold_case;
> +
>  static void parse_argv(void);
>  static void parse_cat_blob(const char *p);
>  static void parse_ls(const char *p, struct branch *b);
>  
> +static int strncmp_foldcase(const char *a, const char *b, size_t count)
> +{
> +	return fold_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
> +}
> +
>  static void write_branch_report(FILE *rpt, struct branch *b)
>  {
>  	fprintf(rpt, "%s:\n", b->name);
> @@ -1507,7 +1515,7 @@ static int tree_content_set(
>  	t = root->tree;
>  	for (i = 0; i < t->entry_count; i++) {
>  		e = t->entries[i];
> -		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
> +		if (e->name->str_len == n && !strncmp_foldcase(p, e->name->str_dat, n)) {
>  			if (!*slash1) {
>  				if (!S_ISDIR(mode)
>  						&& e->versions[1].mode == mode
> @@ -1597,7 +1605,7 @@ static int tree_content_remove(
>  	t = root->tree;
>  	for (i = 0; i < t->entry_count; i++) {
>  		e = t->entries[i];
> -		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
> +		if (e->name->str_len == n && !strncmp_foldcase(p, e->name->str_dat, n)) {
>  			if (*slash1 && !S_ISDIR(e->versions[1].mode))
>  				/*
>  				 * If p names a file in some subdirectory, and a
> @@ -1664,7 +1672,7 @@ static int tree_content_get(
>  	t = root->tree;
>  	for (i = 0; i < t->entry_count; i++) {
>  		e = t->entries[i];
> -		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
> +		if (e->name->str_len == n && !strncmp_foldcase(p, e->name->str_dat, n)) {
>  			if (!*slash1)
>  				goto found_entry;
>  			if (!S_ISDIR(e->versions[1].mode))
> @@ -3246,6 +3254,10 @@ static int parse_one_feature(const char *feature, int from_stream)
>  		relative_marks_paths = 1;
>  	} else if (!strcmp(feature, "no-relative-marks")) {
>  		relative_marks_paths = 0;
> +	} else if (!strcmp(feature, "fold-case")) {
> +		fold_case = 1;
> +	} else if (!strcmp(feature, "no-fold-case")) {
> +		fold_case = 0;
>  	} else if (!strcmp(feature, "done")) {
>  		require_explicit_termination = 1;
>  	} else if (!strcmp(feature, "force")) {
> @@ -3372,6 +3384,7 @@ int main(int argc, char **argv)
>  	avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));
>  	marks = pool_calloc(1, sizeof(struct mark_set));
>  
> +	fold_case = ignore_case;
A complete different question:
According to my understanding,

"git -c core.ignorecase=false fast-import"

should already do what you want to do.
(I haven't tested it, but it should work, otherwise there is probably a bug somewhere)

But that option is probably "hidden" under the general git options :
http://git-htmldocs.googlecode.com/git/git.html



>  	global_argc = argc;
>  	global_argv = argv;
>  
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index aac126f..7057c26 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -3088,4 +3088,83 @@ test_expect_success 'U: validate root delete result' '
>  	compare_diff_raw expect actual
>  '
>  
> +cat >input <<INPUT_END
> +blob
> +mark :1
> +data 2
> +a
> +
> +commit refs/heads/V
> +committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
> +data 0
> +
> +M 644 :1 a
> +
> +commit refs/heads/V
> +committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
> +data 0
> +
> +R a A
> +INPUT_END
> +
> +test_expect_success 'V: default case folding with ignorecase=true' '
> +	git config core.ignorecase true &&
> +	git fast-import <input &&
> +	git ls-tree refs/heads/V >actual &&
> +	git update-ref -d refs/heads/V &&
> +	cat >expected <<\EOF &&
> +100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	a
> +EOF
> +	test_cmp expected actual'
> +
> +test_expect_success 'V: default case folding with ignorecase=false' '
> +	git config core.ignorecase false &&
> +	git fast-import <input &&
> +	git ls-tree refs/heads/V >actual &&
> +	git update-ref -d refs/heads/V &&
> +	cat >expected <<\EOF &&
> +100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	A
> +EOF
> +	test_cmp expected actual'
> +
> +test_expect_success 'V: forced case folding with ignorecase=true' '
> +	git config core.ignorecase true &&
> +	git fast-import --fold-case <input &&
> +	git ls-tree refs/heads/V >actual &&
> +	git update-ref -d refs/heads/V &&
> +	cat >expected <<\EOF &&
> +100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	a
> +EOF
> +	test_cmp expected actual'
> +
If you want to make it shorter (and try to avoid repetition):
The forced true cases could be collected in a loop.
(and the same for forced=false)
[snip]

Re: [PATCH] fast-import: add options to enable/disable case folding

[Junio C Hamano]

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

>> +--[no-]fold-case::
>> +	When files/directories with the same name but a different case
>> +	are detected, they are treated as the same (--fold-case) or as
>> +	being different (--no-fold-case). The default is --fold-case
>> +	when `core.ignorecase` is set to `true`, and --no-fold-case when
>> +	it is `false`.
>> +
> Most often the we use the term "ignore-case", could that be a better name ?
> Other opinions, pros/cons  ?

Yeah, --[no-]ignore-case sounds more in line with how other
commands' options are spelled.

But I somehow thought this "case-folding" was deliberately done as
an improvement against the original that did not have a way to do
the "ignore-case"?

http://thread.gmane.org/gmane.comp.version-control.git/200597/focus=200625

I am not sure why not until now I did not find the original
justification dubious, but I think fast-export should never do case
folding---Joshua talks about working trees on a file system that is
incapable of expressing different cases, but "export" is about
reading in-repository histories, whose trees are fully capable of
expressing paths in different cases just fine, and spitting out a
file that can be processed by fast-import.  I do not see why it
should collapse two different paths that differ in case at export
time.

If the original history is broken by Perforce or whatever and
recording the history of the same path in different case
combinations in different commits, perhaps the right thing to do is
to fix the original history in Git repository before exporting in
the first place.

I do not see how such a corruption is related to the characteristics
of the filesystem where "export" is run.  Perhaps a case-insensitive
filesystem may helped Perforce to corrupt the history when initial
import of the history into Git was done, but core.ignorecase of the
current repository does not help us decide if that was actually the
case---the import may have been done on a completely different
machine.

So perhaps we should rip the case folding out altogether instead?
The entry for the change in the Release Notes may say:

 * "git fast-import" incorrectly case-folded the paths recorded in
   the history when core.ignorease is set (i.e. the repository's
   working tree is incapable of expressing paths that differ only in
   their cases); this old bug was reported in 2012 and was finally
   corrected.

or something like that?

Re: [PATCH] fast-import: add options to enable/disable case folding

[Eric Sunshine]

On Fri, Apr 17, 2015 at 06:56:43PM +0200, Torsten Bögershausen wrote:
> On 04/17/2015 01:52 PM, Mike Hommey wrote:
> > +test_expect_success 'V: default case folding with ignorecase=true' '
> > +	git config core.ignorecase true &&
> > +	git fast-import <input &&
> > +	git ls-tree refs/heads/V >actual &&
> > +	git update-ref -d refs/heads/V &&
> > +	cat >expected <<\EOF &&
> > +100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	a
> > +EOF
> > +	test_cmp expected actual'
> > +
> > +test_expect_success 'V: default case folding with ignorecase=false' '
> > +	git config core.ignorecase false &&
> > +	git fast-import <input &&
> > +	git ls-tree refs/heads/V >actual &&
> > +	git update-ref -d refs/heads/V &&
> > +	cat >expected <<\EOF &&
> > +100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	A
> > +EOF
> > +	test_cmp expected actual'
> > +
> > +test_expect_success 'V: forced case folding with ignorecase=true' '
> > +	git config core.ignorecase true &&
> > +	git fast-import --fold-case <input &&
> > +	git ls-tree refs/heads/V >actual &&
> > +	git update-ref -d refs/heads/V &&
> > +	cat >expected <<\EOF &&
> > +100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	a
> > +EOF
> > +	test_cmp expected actual'
>
> If you want to make it shorter (and try to avoid repetition):
> The forced true cases could be collected in a loop.
> (and the same for forced=false)

I was also going to suggest squashing the repetition. Here's what I
had in mind:

--- >8 ---
test_foldcase() {
	ignore=$1 &&
	case "$2" in
	true) fold=--fold-case folded=true ;;
	false) fold=--no-fold-case folded=false ;;
	*) fold= folded=$ignore ;;
	esac &&
	case $folded in true) folded=a ;; false) folded=A ;; esac &&

	test_expect_success "V: case folding: ignorecase=$ignore${fold:+ $fold}" "
		git -c core.ignorecase=$ignore fast-import $fold <input &&
		git ls-tree refs/heads/V >actual &&
		git update-ref -d refs/heads/V &&
		cat >expect <<-EOF &&
		100644 blob 78981922613b2afb6025042ff6bd878ac1994e85	$folded
		EOF
		test_cmp expect actual
	"
}

for o in '' true false
do
	for c in true false
	do
		test_foldcase $c "$o"
	done
done
--- >8 ---

which outputs:

--- >8 ---
ok 176 - V: case folding: ignorecase=true
ok 177 - V: case folding: ignorecase=false
ok 178 - V: case folding: ignorecase=true --fold-case
ok 179 - V: case folding: ignorecase=false --fold-case
ok 180 - V: case folding: ignorecase=true --no-fold-case
ok 181 - V: case folding: ignorecase=false --no-fold-case
--- >8 ---

Re: [PATCH] fast-import: add options to enable/disable case folding

[Mike Hommey]

On Fri, Apr 17, 2015 at 11:44:00AM -0700, Junio C Hamano wrote:
> So perhaps we should rip the case folding out altogether instead?
> The entry for the change in the Release Notes may say:
> 
>  * "git fast-import" incorrectly case-folded the paths recorded in
>    the history when core.ignorease is set (i.e. the repository's
>    working tree is incapable of expressing paths that differ only in
>    their cases); this old bug was reported in 2012 and was finally
>    corrected.
> 
> or something like that?

Is anything else then git-p4 known to rely on case folding? If not, I
guess that's a reasonable plan. We could even add an option to
fast-import that would allow to turn case folding back on, and make
git-p4 use it, so that its expectations are fulfilled. Although at some
point, it could (should?) do case folding itself(?)

Mike

Re: [PATCH] fast-import: add options to enable/disable case folding

[Luke Diamand]

On 18/04/15 08:36, Mike Hommey wrote:
> On Fri, Apr 17, 2015 at 11:44:00AM -0700, Junio C Hamano wrote:
>> So perhaps we should rip the case folding out altogether instead?
>> The entry for the change in the Release Notes may say:
>>
>>   * "git fast-import" incorrectly case-folded the paths recorded in
>>     the history when core.ignorease is set (i.e. the repository's
>>     working tree is incapable of expressing paths that differ only in
>>     their cases); this old bug was reported in 2012 and was finally
>>     corrected.
>>
>> or something like that?
>
> Is anything else then git-p4 known to rely on case folding? If not, I
> guess that's a reasonable plan. We could even add an option to
> fast-import that would allow to turn case folding back on, and make
> git-p4 use it, so that its expectations are fulfilled. Although at some
> point, it could (should?) do case folding itself(?)

git-p4 has a single line of code that checks if core.ignorecase is 
turned on, and uses this to decide whether to skip files that are 
outside the depot being tracked and I *think* is not really related to 
fast-import.

I don't know to what extent though git-p4 relies on the current 
behaviour of git fast-import to fold case for it.

There's a 'p4 info' command which tells you what the server thinks it's 
doing:

$ p4 info | grep Case
Case Handling: sensitive

I don't know how long that support has been present (it might not work 
on older servers that some people are still using).

It's also possible to force the server to be case-insensitive on the 
Linux version. That's useful, as it we could construct some test cases 
to see what we're likely to break without having to force people to 
install a case-insensitive OS in order to run the git regression tests.

Luke


>
> Mike
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>