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.

Sign up for GitHub

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

Improve handling of commands in Git.pm on native Windows #1604

Closed
wants to merge 3 commits into from
Closed

Improve handling of commands in Git.pm on native Windows #1604

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fff0040
Perl: Support spaces in command arguments on MSWin32
ColMelvin Mar 31, 2018
5d4934a
Perl: Support piping commands on MSWin32
ColMelvin Mar 31, 2018
f6033be
Make MSWin32 Perl available to all tests
ColMelvin Oct 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2475,6 +2475,19 @@ LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))

# Set MSWIN32_PERL_PATH=false to disable search
ifndef MSWIN32_PERL_PATH
MSWIN32_PERL_PATH = $(shell \
for perl in $$(type -ap perl); do \
if "$$perl" -e 'exit 1 if $$^O ne q{MSWin32}'; then \
echo "$$perl"; \
exit 0; \
fi; \
done \
)
endif
MSWIN32_PERL_PATH_SQ = $(subst ','\'',$(MSWIN32_PERL_PATH))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure that this is a good change. By default, the Git for Windows SDK comes with a perl.exe and we know exactly which one it is: the one that is shipped with (and therefore, supported by) Git for Windows.

In other words, if you really want this, I would want to make it an opt-in rather than an opt-out.

Besides, would it not make things substantially cleaner if you simply prepended the path to the known MSWin32 Perl (if configured via MSWIN32_PERL_PATH explicitly) to PATH? That way, perl would already find the Perl interpreter you want, no need to play extra games with any mswin32_perl shell function.


ifndef NO_PERL
all:: $(LIB_PERL_GEN)
ifndef NO_PERL_CPAN_FALLBACKS
Expand Down Expand Up @@ -2573,6 +2586,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+
@echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+
@echo X=\'$(X)\' >>$@+
@echo MSWIN32_PERL_PATH=\''$(subst ','\'',$(MSWIN32_PERL_PATH_SQ))'\' >>$@+
ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+
endif
Expand Down
19 changes: 3 additions & 16 deletions t/t9701-perl-git-MSWin32.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,12 @@
test_description='perl interface (Git.pm) on MSWin32'
. ./test-lib.sh

find_MSWin32_perl() {
local perl
for perl in $(type -ap perl); do
if "$perl" -e 'exit 1 if $^O ne q{MSWin32}'; then
echo "$perl"
return 0
fi
done
return 1
}

MSWin32_Perl="$(find_MSWin32_perl)"
if [ $? -ne 0 ]; then
if ! test_have_prereq MSWIN32_PERL; then
skip_all='skipping perl on MSWin32 interface tests, MSWin32 perl not available'
test_done
fi

"$MSWin32_Perl" -MTest::More -e 0 2>/dev/null || {
mswin32_perl -MTest::More -e 0 2>/dev/null || {
skip_all="MSWin32 Perl Test::More unavailable, skipping test"
test_done
}
Expand All @@ -35,11 +23,10 @@ test_external_has_tap=1
perl_test_path="$TEST_DIRECTORY"/t9701/test.pl
if test_have_prereq CYGWIN || test_have_prereq MINGW; then
perl_test_path="$(cygpath -w "$perl_test_path")"
GITPERLLIB="$(cygpath -w -p "$GITPERLLIB")"
fi

test_external_without_stderr \
'Windows command line (Perl API)' \
"$MSWin32_Perl" "$perl_test_path"
mswin32_perl "$perl_test_path"

test_done
8 changes: 8 additions & 0 deletions t/test-lib-functions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,14 @@ perl () {
command "$PERL_PATH" "$@" 2>&7
} 7>&2 2>&4

mswin32_perl () {
local GITPERLLIB="$GITPERLLIB"
if test_have_prereq CYGWIN || test_have_prereq MINGW; then
GITPERLLIB="$(cygpath -w -p "$GITPERLLIB")"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that really work? There are native .dll files that some of the Perl modules want to load, and I am rather certain that they can be used only by /usr/bin/perl. It has to be an MSYS Perl. It even has to match the precise version, otherwise it won't load.

fi
command "$MSWIN32_PERL_PATH" "$@" 2>&7
} 7>&2 2>&4

# Is the value one of the various ways to spell a boolean true/false?
test_normalize_bool () {
git -c magic.variable="$1" config --bool magic.variable 2>/dev/null
Expand Down
10 changes: 9 additions & 1 deletion t/test-lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ then
exit 1
fi
. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
export PERL_PATH SHELL_PATH
export MSWIN32_PERL_PATH PERL_PATH SHELL_PATH

################################################################
# It appears that people try to run tests without building...
Expand Down Expand Up @@ -1268,3 +1268,11 @@ test_lazy_prereq CURL '
test_lazy_prereq SHA1 '
test $(git hash-object /dev/null) = e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
'

# Some tests require a native Win32 Perl interpreter, such as Strawberry Perl
# or ActiveState Perl, which is not distributed with Git for Windows. These
# tests only run if an appropriate Perl is specified (via MSWIN32_PERL_PATH).
test_lazy_prereq MSWIN32_PERL '
test -n "$MSWIN32_PERL_PATH" &&
$MSWIN32_PERL_PATH -e "exit 1 if \$^O ne q{MSWin32}"
'