GeezerVison is showing github.com — Run Cygwin CI workflow commands in login shells by EliahKagan · Pull Request #1709 · gitpython-developers/GitPython
Conversation
EliahKagan
commented
Oct 14, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
This passes --login to the bash shell used to run commands in the Cygwin environment on CI. This eliminates the need to work around a partly broken environment, and the extra code what was used to do that is accordingly removed. There are two benefits of this change:
-
The PATH is correct: Cygwin's /usr/local/bin and /usr/bin are present at the beginning of PATH. Otherwise, it is easy to get /usr/bin at the front, but rather involved to get /usr/local/bin to precede it. Because Python on Cygwin puts scripts/executables such as the upgraded pip and the pytest command in /usr/local/bin, it is valuable to have that directory in the PATH and best to have it before /usr/bin. (I have set CYGWIN_NOWINPATH to omit other directories, since finding any of the commands to be run in the Cygwin environment outside that environment is unintended.)
-
Every step automatically has correct temporary directories: When Cygwin commands were not being run in login shells, they didn't automatically get correct values for TMP and TEMP for their environment. To work around this, those environment variables were set globally, for every step. But that caused them to refer to nonexistent locations for steps such as actions/checkout. Most likely this would not cause any errors, but it did cause copious warnings about a nonexistent temporary directory, which risked obscuring other potentially important output. Now that Cygwin commands run in login shells, both the few non-Cygwin steps, and the steps run in the Cygwin environment, all get correct temporary directories (with TMP and TEMP set in the prewritten startup script the login shell uses).
A theoretical disadvantage of this is that login shells take slightly longer to start up, but that delay is insignificant in this application. A more significant disadvantage is that setting the -x shell option the way it was done before would produce a lot of noise at the beginning of the output for every command-running step. To work around that, -x is omitted from the value of shell and set -x is added at the end of the startup script for login shells, so it runs before each step's "payload" command, but without applying to the commands run in the startup script itself.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This passes --login to the bash shell used to run commands in the
Cygwin environment on CI. This eliminates the need to work around a
partly broken environment, and the extra code what was used to do
that is accordingly removed. There are two benefits of this change:
- The PATH is correct: Cygwin's /usr/local/bin and /usr/bin are
present at the beginning of PATH. Otherwise, it is easy to get
/usr/bin at the front, but rather involved to get /usr/local/bin
to precede it. Because Python on Cygwin puts scripts/executables
such as the upgraded "pip" and the "pytest" command in
/usr/local/bin, it is valuable to have that directory in the PATH
and best to have it before /usr/bin. (I have set CYGWIN_NOWINPATH
to omit other directories, since finding any of the commands to
be run in the Cygwin environment outside that environment is
unintended.)
- Every step automatically has correct temporary directories: When
Cygwin commands were not being run in login shells, they didn't
automatically get correct values for TMP and TEMP for their
environment. To work around this, those environment variables
were set globally, for every step. But that caused them to refer
to nonexistent locations for steps such as actions/checkout. Most
likely this would not cause any errors, but it did cause copious
warnings about a nonexistent temporary directory, which risked
obscuring other potentially important output. Now that Cygwin
commands run in login shells, both the few non-Cygwin steps, and
the steps run in the Cygwin enviroment, all get correct temporary
directories (with TMP and TEMP set in the prewritten startup
script the login shell uses).
A theoretical disadvantage of this is that login shells take
slightly longer to start up, but that delay is insigificant in
this application. A more significant disadvantage is that setting
the -x shell option the way it was done before would produce a lot
of noise at the beginning of the output for every command-running
step. To work around that, -x is omitted from the value of "shell"
and "set -x" is added at the end of the startup script for login
shells, so it runs before each step's "payload" command, but
without applying to the commands run in the startup script itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's so awesome! Great discovery!
I can sense that native Windows support is just around the corner :).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
EliahKagan reacted with thumbs up emoji
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
Dec 23, 2023
This makes a pip -> pip3 symlink in /usr/bin in a new step prior to
the first step that runs the pip command. Using "pip3", "pip3.9",
or a command like "python -m pip" would work, but this allows the
Cygwin workflow to continue using the same installation commands as
the main testing workflow.
Adding this fixes two problems:
1. When the pip version installed by the python39-pip package is
current, so that upgrading pip doesn't install any new commmand,
no "pip" command is created in /usr/local. This has happened not
to be the case for a long time, which is why the Cygwin workflow
was able to pass. (That the recent failures started at the merge
of gitpython-developers#1783 turns out to be a coincidence: rerunning jobs on prior
commits has the failure, as does experimentally reverting it.)
2. Even when the pip version installed by python39-pip is behind
the latest available version, pip is still used before being
upgraded to check if setuptools is installed, to decide whether
to upgrade it. This is to keep similar steps in the two testing
workflows similar, since the Cygwin workflow only uses Python
3.9, which always has setuptools. Because pip was never in $PATH
in that step, the Cygwin workflow wrongly refrained from trying
to upgrade setuptools.
When the "Update PyPA packages" step does find a newer version of
pip to upgrade to, it installs it in /usr/local/bin, which we have
in $PATH before /usr/bin, so the upgraded version, when present,
will still be preferred in subsequent commands, as before.
Running "pip" on Cygwin when it may not be in $PATH -- and, for one
step, never is -- was a bug introduced in e8956e5 (gitpython-developers#1709). Before
that, "pip" still was not always available, but it was not used.
This change fixes the bug by making sure "pip" is always available.
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
Dec 23, 2023
This makes a pip -> pip3 symlink in /usr/bin in a new step prior to
the first step that runs the pip command. Using "pip3", "pip3.9",
or a command like "python -m pip" would work, but this allows the
Cygwin workflow to continue using the same installation commands as
the main testing workflow.
Adding this fixes two problems:
1. When the pip version installed by the python39-pip package is
current, so that upgrading pip doesn't install any new commmand,
no "pip" command is created in /usr/local. This has happened not
to be the case for a long time, which is why the Cygwin workflow
was able to pass. (That the recent failures started at the merge
of gitpython-developers#1783 turns out to be a coincidence: rerunning jobs on prior
commits has the failure, as does experimentally reverting it.)
2. Even when the pip version installed by python39-pip is behind
the latest available version, pip is still used before being
upgraded to check if setuptools is installed, to decide whether
to upgrade it. This is to keep similar steps in the two testing
workflows similar, since the Cygwin workflow only uses Python
3.9, which always has setuptools. Because pip was never in $PATH
in that step, the Cygwin workflow wrongly refrained from trying
to upgrade setuptools.
When the "Update PyPA packages" step does find a newer version of
pip to upgrade to, it installs it in /usr/local/bin, which we have
in $PATH before /usr/bin, so the upgraded version, when present,
will still be preferred in subsequent commands, as before.
Running "pip" on Cygwin when it may not be in $PATH -- and, for one
step, never is -- was a bug introduced in e8956e5 (gitpython-developers#1709). Before
that, "pip" still was not always available, but it was not used.
This change fixes the bug by making sure "pip" is always available.
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
Dec 23, 2023
This makes a pip -> pip3 symlink in /usr/bin in a new step prior to
the first step that runs the pip command. Using "pip3", "pip3.9",
or a command like "python -m pip" would work, but this allows the
Cygwin workflow to continue using the same installation commands as
the main testing workflow.
Adding this fixes two problems:
1. When the pip version installed by the python39-pip Cygwin
package is current, so that upgrading pip doesn't install any
new commmand, no "pip" command is created in /usr/local. This
has happened not to be the case for a long time, which is why
the Cygwin workflow was able to pass. (That the recent failures
started at the merge of gitpython-developers#1783 turns out to be a coincidence:
rerunning jobs on prior commits has the failure, as does
experimentally reverting it.)
2. Even when the pip version installed by python39-pip is behind
the latest available version, pip is still used before being
upgraded to check if setuptools is installed, to decide whether
to upgrade it. This is to keep similar steps in the two testing
workflows similar, since the Cygwin workflow only uses Python
3.9, which always has setuptools. Because pip was never in $PATH
in that step, the Cygwin workflow wrongly refrained from trying
to upgrade setuptools.
When the "Update PyPA packages" step does find a newer version of
pip to upgrade to, it installs it in /usr/local/bin, which we have
in $PATH before /usr/bin, so the upgraded version, when present,
will still be preferred in subsequent commands, as before.
Running "pip" on Cygwin when it may not be in $PATH -- and, for one
step, never is -- was a bug introduced in e8956e5 (gitpython-developers#1709). Before
that, "pip" still was not always available, but it was not used.
This change fixes the bug by making sure "pip" is always available.
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
Dec 23, 2023
This makes a pip -> pip3 symlink in /usr/bin in a new step prior to
the first step that runs the pip command. Using "pip3", "pip3.9",
or a command like "python -m pip" would work, but this allows the
Cygwin workflow to continue using the same installation commands as
the main testing workflow.
Adding this fixes two problems:
1. When the pip version installed by the python39-pip Cygwin
package is current, so that upgrading pip doesn't install any
new commmand, no "pip" command is created in /usr/local. This
has happened not to be the case for a long time, which is why
the Cygwin workflow was able to pass. (That the recent failures
started at the merge of gitpython-developers#1783 turns out to be a coincidence:
rerunning jobs on prior commits has the failure, as does
experimentally reverting it.)
2. Even when the pip version installed by python39-pip is behind
the latest available version, pip is still used before being
upgraded to check if setuptools is installed, to decide whether
to upgrade it. This is to keep similar steps in the two testing
workflows similar, since the Cygwin workflow only uses Python
3.9, which always has setuptools. Because pip was never in $PATH
in that step, the Cygwin workflow wrongly refrained from trying
to upgrade setuptools.
When the "Update PyPA packages" step does find a newer version of
pip to upgrade to, it installs it in /usr/local/bin, which we have
in $PATH before /usr/bin, so the upgraded version, when present,
will still be preferred in subsequent commands, as before.
Running "pip" on Cygwin when it may not be in $PATH -- and, for one
step, never is -- was a bug introduced in e8956e5 (gitpython-developers#1709). Before
that, "pip" still was not always available, but it was not used.
This change fixes the bug by making sure "pip" is always available.
Reviewers
Byron
Byron approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|
%5D(https%3A%2F%2Frenovatebot.com){kind=link}