Consistently terminate commit messages with LF

[thaliaarchi]

When the length logic for fast-import 'data' commands was updated in 4c10270 (Fix data handling, 2023-03-02), one branch was missed, so commit messages now do not have a final LF appended in most cases. This changed the longtime behavior, which had been consistent since the first commit of hg2git, 9832035 (Initial import, 2007-03-06), and is expected by some applications which compare against old conversions from Mercurial[1].

[thaliaarchi]

This changes the expected output of the tests. Note, though, that t/ was created after this bug was introduced, so they have never reflected this behavior. I would have updated them myself here, but tests currently fail on master for reasons I don't understand.

[frej]

This changed the longtime behavior, which had been consistent since the first commit of hg2git

Good catch! That should be fixed, no argument there. But we should have the test cases updated in the same commit to stay bisectable.

I would have updated them myself here, but tests currently fail on master for reasons I don't understand.

I can't reproduce any breakage on master locally. I did #334 to rule out that it was just a fluke, but it's not broken in the CI either. If you are switching between branches, have you done git clean -fxd to be sure that there's not any old files laying around? Running make TEST_OPTS='--debug --verbose' -C t will give you more information. The documentation for the test harness is here.

[thaliaarchi]

Thanks for the pointer about --debug and --verbose. With that, this error is printed, which was hidden before:

Error: The option core.ignoreCase is set to true in the git
repository. This will produce empty changesets for renames that just
change the case of the file name.
Use --force to skip this check or change the option with
git config core.ignoreCase false

I have core.ignoreCase=true globally configured, because I unfortunately setup my filesystem as case-insensitive long ago. The problem is simply that the tests don't override this. Rather than setting it in the tests' repos as I have done in my own usage of hg-fast-export, I've pushed a patch to not rely on the user's configuration and instead run git fast-import with ignoreCase disabled inline. Now users have one less error to worry about. I don't think there's a single use case for wanting path collisions with hg-fast-export, that wouldn't be more correctly solved with a git-filter-repo pass afterwards.

I'm surprised that git fast-import has this issue in the first place, since I'm familiar with the code from contributing patches to it, but, sure enough, it uses variably case-sensitive path comparisons:

int fspathncmp(const char *a, const char *b, size_t count)
{
	return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
}

[thaliaarchi]

Should I have left the core.ignoreCase change out of this PR? Tests pass for me locally with those changes and should remain working with CI (though I think you need to approve the workflow, for some reason, for the tests to run).

[frej]

f947189 looks good, but I don't think we should force the user to use core.ignoreCase=false. As we leave it to the user to run git init they should be free to configure such things without fast-export overriding it.

[thaliaarchi]

Thanks for the review. I've reverted the changes to core.ignoreCase behavior and updated only tests.

[frej]

Thank you for your contribution @thaliaarchi.