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

Fix shebang and executable paths in gitbash #1364

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
253702f
Add pathing options to shells
amorphousWaste Aug 20, 2022
666d55f
Add pathing options for cmd
amorphousWaste Aug 20, 2022
3d2e429
Add pathing options to gitbash
amorphousWaste Aug 20, 2022
7fd92e7
Add slash expansion
amorphousWaste Aug 20, 2022
cfafdbe
Conform drive start letters
amorphousWaste Aug 20, 2022
9c7caa0
Quote executable
amorphousWaste Aug 20, 2022
6bf148b
Use new pathing option for shell paths
amorphousWaste Aug 20, 2022
956ecb3
Add pathing options to rex
amorphousWaste Aug 20, 2022
ab0bdab
Use new path option
amorphousWaste Aug 20, 2022
d1b071d
Remove linting fixes
amorphousWaste Aug 21, 2022
4376a38
Add shell_pathed_env_vars to config
amorphousWaste Sep 22, 2022
24dbc7c
Add classmethod to check for shell pathed keys
amorphousWaste Sep 22, 2022
9c00baf
Add is_shell_path arg to escape_string
amorphousWaste Sep 22, 2022
3677b00
Add is_shell_path arg to escape_string overrides
amorphousWaste Sep 22, 2022
57123b1
Update call to escape_string with is_shell_path arg
amorphousWaste Sep 22, 2022
8aae074
Capitalize drive letter on windows path conversion
amorphousWaste Sep 22, 2022
250338f
Change "cygwin" to "cygpath"
amorphousWaste Sep 24, 2022
de91e23
Change "cygwin" to "cygpath"
amorphousWaste Sep 24, 2022
6c9c2ec
Fix docstring
amorphousWaste Sep 24, 2022
4bb599a
Merge branch 'fixes_#1302' of https://github.com/amorphousWaste/rez i…
amorphousWaste Sep 24, 2022
abe10f6
Fix call
amorphousWaste Oct 3, 2022
b633dc7
Add description to shell_pathed_env_vars
amorphousWaste Oct 3, 2022
024708b
Add convert_path wrapper
amorphousWaste Oct 3, 2022
2dae12b
Update to_posix_path
amorphousWaste Oct 3, 2022
314efe7
Update to_windows_path
amorphousWaste Oct 3, 2022
0ac5820
Use new convert_path function
amorphousWaste Oct 3, 2022
55c52e8
Replace as_shell_path with as_path
amorphousWaste Oct 4, 2022
fefa176
Add to_windows_path function
amorphousWaste Oct 4, 2022
21f3184
Revert erroneous change
amorphousWaste Oct 4, 2022
7559fc3
Swap if statement for better readability
amorphousWaste Oct 14, 2022
420ba83
Update normalize_paths replacement
amorphousWaste Oct 14, 2022
3b9a0c5
Correct capitalization
amorphousWaste Oct 14, 2022
e50404b
Change `as_shell_path` with `as_path`
amorphousWaste Oct 14, 2022
ef4a427
Update implicit package value expansion
amorphousWaste Oct 14, 2022
b56f64d
Add special regex for mixed drive letters
amorphousWaste Oct 25, 2022
9e5226a
Fix path slashes
amorphousWaste Oct 25, 2022
8c7c3d2
Conform strings
amorphousWaste Oct 25, 2022
78abeab
Fix path conversions
amorphousWaste Oct 25, 2022
2883f73
Add special logic for implicit keys and values
amorphousWaste Oct 25, 2022
c3caada
Merge with 2.112.0 changes
amorphousWaste Nov 18, 2022
8dfde24
Add disable_normalization flag to config
amorphousWaste Nov 24, 2022
e81756c
Add disable normalization test
amorphousWaste Nov 24, 2022
efc95f4
Create shell utils tests
amorphousWaste Nov 24, 2022
8844f69
Add logic for skipping path normalization
amorphousWaste Nov 24, 2022
6a4edb1
Add debugging output to path conversion
amorphousWaste Nov 24, 2022
ecdb3cb
Merge branch 'master' into fixes_#1302
JeanChristopheMorinPerso Feb 12, 2023
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
46 changes: 40 additions & 6 deletions src/rezplugins/shell/_utils/powershell_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from rez.system import system
from rez.utils.platform_ import platform_
from rez.utils.execution import Popen
from rez.utils.logging_ import print_debug
from rez.util import shlex_join
from .windows import convert_path

Expand Down Expand Up @@ -250,7 +251,16 @@ def normalize_path(self, path):
return path

if platform_.name == "windows":
return convert_path(path, 'windows')
converted_path = convert_path(path, 'windows')
if path != converted_path:
print_debug(
'Path converted: {} -> {}'.format(path, converted_path)

Choose a reason for hiding this comment

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

How about Path normalized? Also, both converted and changed are used. I think we should make sure the message is consistent. Lastly, I'd use {!r} to automatically quote the paths. This usually helps with debugging whitespace issues, etc.

)
self._addline(
'# Path converted: {} -> {}'.format(path, converted_path)
)
return converted_path

else:
return path

Expand All @@ -261,21 +271,45 @@ def shebang(self):
pass

def setenv(self, key, value):
value = self.escape_string(value, is_path=self._is_pathed_key(key))
self._addline('Set-Item -Path "Env:{0}" -Value "{1}"'.format(key, value))
is_path = self._is_pathed_key(key)
new_value = self.escape_string(value, is_path=is_path)

if is_path and value != new_value:
print_debug(
'Path changed: {} -> {}'.format(value, new_value)
)
self._addline(
'# Path value changed: {} -> {}'.format(value, new_value)
)

self._addline(
'Set-Item -Path "Env:{0}" -Value "{1}"'.format(key, new_value)
)

def prependenv(self, key, value):
value = self.escape_string(value, is_path=self._is_pathed_key(key))
is_path = self._is_pathed_key(key)
new_value = self.escape_string(value, is_path=is_path)

if is_path and value != new_value:
self._addline(
'# Path value changed: {} -> {}'.format(value, new_value)
)

# Be careful about ambiguous case in pwsh on Linux where pathsep is :
# so that the ${ENV:VAR} form has to be used to not collide.
self._addline(
'Set-Item -Path "Env:{0}" -Value ("{1}{2}" + (Get-ChildItem "Env:{0}").Value)'.format(
key, value, self.pathsep)
key, new_value, self.pathsep)
)

def appendenv(self, key, value):
value = self.escape_string(value, is_path=self._is_pathed_key(key))
is_path = self._is_pathed_key(key)
new_value = self.escape_string(value, is_path=is_path)

if is_path and value != new_value:
self._addline(
'# Path value changed: {} -> {}'.format(value, new_value)
)

# Be careful about ambiguous case in pwsh on Linux where pathsep is :
# so that the ${ENV:VAR} form has to be used to not collide.
Expand Down
15 changes: 9 additions & 6 deletions src/rezplugins/shell/_utils/windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import re
import subprocess
from rez.utils.execution import Popen

from rez.utils.logging_ import print_debug

_drive_start_regex = re.compile(r"^([A-Za-z]):\\")
_drive_regex_mixed = re.compile(r"([a-z]):/")
Expand Down Expand Up @@ -38,20 +38,23 @@ def _repl(m):

# Convert the path based on mode.
if mode == 'mixed':
path = to_mixed_path(path)
new_path = to_mixed_path(path)
elif mode == 'windows':
path = to_windows_path(path)
new_path = to_windows_path(path)
else:
path = to_posix_path(path)
new_path = to_posix_path(path)

# NOTE: This would be normal cygpath behavior, but the broader
# implications of enabling it need extensive testing.
# Leaving it up to the user for now.
if force_fwdslash:
# Backslash -> fwdslash
path = path.replace('\\', '/')
new_path = new_path.replace('\\', '/')

return path
if path != new_path:
print_debug('Path converted: {} -> {}'.format(path, new_path))

Choose a reason for hiding this comment

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

Won't this cause duplicate logs? Should the responsibility to log be of the caller or the callee? In general the responsibility is delegated to the caller, but we could say that in this case it's the callee.


return new_path


def to_posix_path(path):
Expand Down
18 changes: 14 additions & 4 deletions src/rezplugins/shell/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"""
Windows Command Prompt (DOS) shell.
"""
from lib2to3.pytree import convert
from rez.config import config
from rez.rex import RexExecutor, expandable, OutputStyle, EscapedString
from rez.shells import Shell
Expand Down Expand Up @@ -296,24 +295,35 @@ def normalize_path(self, path):
Returns:
(str): Normalized file path.
"""
return convert_path(path, 'windows')
# Prevent path conversion if normalization is disabled in the config.
if config.disable_normalization:
return path

converted_path = convert_path(path, 'windows')

if path != converted_path:
self._addline(
'REM Path converted: {} -> {}'.format(

Choose a reason for hiding this comment

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

Again, converted vs changed.

path, converted_path
)
)

return converted_path

def _saferefenv(self, key):
pass

def shebang(self):
pass

def setenv(self, key, value):
value = self.escape_string(
new_value = self.escape_string(
value,
is_path=self._is_pathed_key(key),
is_shell_path=self._is_shell_pathed_key(key),
)
self._addline('set %s=%s' % (key, value))

self._addline('set %s=%s' % (key, new_value))

def unsetenv(self, key):
self._addline("set %s=" % key)
Expand Down
15 changes: 13 additions & 2 deletions src/rezplugins/shell/csh.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from rez.util import shlex_join
from rez.utils.execution import Popen
from rez.utils.platform_ import platform_
from rez.utils.logging_ import print_debug
from rez.shells import UnixShell
from rez.rex import EscapedString

Expand Down Expand Up @@ -159,8 +160,18 @@ def _saferefenv(self, key):
self._addline("if (!($?%s)) setenv %s" % (key, key))

def setenv(self, key, value):
value = self.escape_string(value, is_path=self._is_pathed_key(key))
self._addline('setenv %s %s' % (key, value))
is_path = self._is_pathed_key(key) or self._is_shell_pathed_key(key)
new_value = self.escape_string(value, is_path=self._is_pathed_key(key))

if is_path and value != new_value:
print_debug(
'Path changed: {} -> {}'.format(value, new_value)
)
self._addline(
'# Path value changed: {} -> {}'.format(value, new_value)

Choose a reason for hiding this comment

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

Inconsistency in message.

)

self._addline('setenv %s %s' % (key, new_value))

Choose a reason for hiding this comment

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

Should we also normalize the source command? (I can't comment on non modified code...)


def unsetenv(self, key):
self._addline("unsetenv %s" % key)
Expand Down
16 changes: 14 additions & 2 deletions src/rezplugins/shell/gitbash.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,17 @@ def as_shell_path(self, path):
Returns:
(str): Transformed file path.
"""
return convert_path(path, mode='mixed', force_fwdslash=True)
# Prevent path conversion if normalization is disabled in the config.
if config.disable_normalization:
return path

converted_path = convert_path(path, mode='mixed', force_fwdslash=True)
if path != converted_path:
self._addline(
'# Path converted: {} -> {}'.format(path, converted_path)
)
return converted_path

def normalize_path(self, path):
"""Normalize the path to fit the environment.
For example, POSIX paths, Windows path, etc. If no transformation is
Expand All @@ -128,11 +134,17 @@ def normalize_path(self, path):
Returns:
(str): Normalized file path.
"""
return convert_path(path, mode='unix', force_fwdslash=True)
# Prevent path conversion if normalization is disabled in the config.
if config.disable_normalization:
return path

converted_path = convert_path(path, mode='unix', force_fwdslash=True)
if path != converted_path:
self._addline(
'# Path converted: {} -> {}'.format(path, converted_path)
)
return converted_path

def normalize_paths(self, value):
"""
This is a bit tricky in the case of gitbash. The problem we hit is that
Expand Down
14 changes: 12 additions & 2 deletions src/rezplugins/shell/sh.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from rez.config import config
from rez.utils.execution import Popen
from rez.utils.platform_ import platform_
from rez.utils.logging_ import print_debug
from rez.shells import UnixShell
from rez.rex import EscapedString

Expand Down Expand Up @@ -105,15 +106,24 @@ def _bind_interactive_rez(self):

def setenv(self, key, value):
is_implicit = key == 'REZ_USED_IMPLICIT_PACKAGES'

Choose a reason for hiding this comment

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

I'm struggling to understand why this is needed...

is_path = self._is_pathed_key(key) or self._is_shell_pathed_key(key)

value = self.escape_string(
new_value = self.escape_string(
value,
is_path=self._is_pathed_key(key),
is_shell_path=self._is_shell_pathed_key(key),
is_implicit=is_implicit,
)

self._addline('export %s=%s' % (key, value))
if is_path and value != new_value:
print_debug(
'Path value changed: {} -> {}'.format(value, new_value)
)
self._addline(
'# Path value changed: {} -> {}'.format(value, new_value)
)

self._addline('export %s=%s' % (key, new_value))

def unsetenv(self, key):
self._addline("unset %s" % key)
Expand Down