Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows fixes #231

Closed
wants to merge 6 commits into from
Closed

Windows fixes #231

wants to merge 6 commits into from

Conversation

dscho
Copy link
Contributor

@dscho dscho commented Mar 30, 2021

I finally got Git Bash to accept the Windows Store version of Python, so naturally I wanted to verify that the test suite works, but that was not so, funnily enough.

With these two fixes, I can run t9391 successfully in Git for Windows v2.31.1.

newren and others added 6 commits October 21, 2020 16:10
Add a GitHub workflow for continuous testing
--use-mailmap was defined as `--mailmap .mailmap` except that it would
set args.mailmap to ".mailmap" rather than b".mailmap" (in other words,
it accidentally set it to a string rather than a bytestring).  Since
the --mailmap parameter is always passed as a bytestring, we ran into
errors with calling unknown functions due to the type mismatch.

Signed-off-by: Elijah Newren <[email protected]>
The --path-rename flag expected an argument with a colon
character (':') in it, which it assumed without checking. If the user
gave an argument with no colon in it, this backtrace would be shown:

  File "/usr/local/bin/git-filter-repo", line 1626, in __call__
    if values[0] and values[1] and not (
IndexError: list index out of range

Add a real error message in place of the backtrace.

Also check that there's exactly one colon; show an error message if
there's more than one, as that syntax has no interpretation that is
obviously the right one.

Signed-off-by: Lassi Kortela <[email protected]>
On Windows, we want to run with a native Python, i.e. the separator is a
semicolon, and the paths should be Windows paths (although they're
allowed to have forward slashes instead of backslashes).

Since we're most likely running this in an MSYS2 Bash, allow for
`$TEST_DIRECTORY` to pretend to be a Unix path, and translate it via
`cygpath` into a Windows path.

Signed-off-by: Johannes Schindelin <[email protected]>
This fixes the "TypeError: a bytes-like object is required, not 'str'"
problem on Windows, letting t9391 pass.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Contributor Author

dscho commented Mar 30, 2021

Hrm. I see this failure:

[...]
2021-03-30T20:56:22.6773660Z ++ git add foobar.c
2021-03-30T20:56:22.7032750Z warning: LF will be replaced by CRLF in foobar.c.
2021-03-30T20:56:22.7033830Z The file will have its original line endings in your working directory
2021-03-30T20:56:22.7104433Z ++ test_tick
2021-03-30T20:56:22.7105665Z ++ test -z set
2021-03-30T20:56:22.7112391Z ++ test_tick=1112912293
2021-03-30T20:56:22.7113145Z ++ GIT_COMMITTER_DATE='1112912293 -0700'
2021-03-30T20:56:22.7113744Z ++ GIT_AUTHOR_DATE='1112912293 -0700'
2021-03-30T20:56:22.7114516Z ++ export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
2021-03-30T20:56:22.7115444Z ++ git commit -m 'Brain damage'
2021-03-30T20:56:22.7538070Z [master 7d3b5dd] Brain damage
2021-03-30T20:56:22.7539248Z  Author: A U Thor <[email protected]>
2021-03-30T20:56:22.7539879Z  1 file changed, 1 insertion(+)
2021-03-30T20:56:22.7540479Z  create mode 100644 foobar.c
2021-03-30T20:56:22.7649100Z ++ cd print_progress
2021-03-30T20:56:22.7729653Z +++ git rev-parse --verify master
2021-03-30T20:56:22.8006045Z ++ MASTER=7d3b5dddd77d25eb401a95ccbe1a90491a6af781
2021-03-30T20:56:22.8008263Z ++ /d/a/git-filter-repo/git-filter-repo/t/t9391/print_progress.py . new
2021-03-30T20:56:22.9830727Z b'.' does not appear to be a valid git repository
2021-03-30T20:56:22.9990926Z error: last command exited with $?=1
2021-03-30T20:56:22.9991907Z not ok 3 - print_progress.py
2021-03-30T20:56:23.0281824Z ++ setup rename_master_to_develop
2021-03-30T20:56:23.0282198Z #	
2021-03-30T20:56:23.0284876Z #		setup print_progress &&
2021-03-30T20:56:23.0285599Z ++ git init rename_master_to_develop
2021-03-30T20:56:23.0286086Z #		(
2021-03-30T20:56:23.0286587Z #			cd print_progress &&
2021-03-30T20:56:23.0287322Z #			MASTER=$(git rev-parse --verify master) &&
2021-03-30T20:56:23.0288063Z #			$TEST_DIRECTORY/t9391/print_progress.py . new &&
2021-03-30T20:56:23.0288719Z #			test $MASTER = $(git rev-parse --verify refs/heads/master)
2021-03-30T20:56:23.0289214Z #		)
2021-03-30T20:56:23.0289528Z #	

But then, I see the same failure in main... What gives?

@newren
Copy link
Owner

newren commented Mar 31, 2021

Interesting. Not sure why this never triggered problems before, but is it possible that the program "wc" doesn't exist on some Windows boxes like it does on Linux & Mac? Maybe it exists for you, and existed at some point in the past under GitHub Actions, but doesn't exist within whatever current GitHub Actions environment for Windows provides anymore?

Anyway, the patches look good to me. I'll see if I can play with the Windows tests and that suboptimal error message a bit and get it fixed up, then merge these changes.

@newren
Copy link
Owner

newren commented Apr 1, 2021

Merged...but I'm an evil maintainer and I rewrote history for some commits from a few months ago (I'm hiding behind the excuse that this is the filter-repo project so folks should be able to handle history rewrites, or something like that). So, I had to rebase your patches and GitHub doesn't detect it as merged. But it is merged.

Thanks for the fixes!

@newren newren closed this Apr 1, 2021
@dscho dscho deleted the windows-fixes branch April 1, 2021 19:47
@dscho
Copy link
Contributor Author

dscho commented Apr 1, 2021

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants