Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Work around Cygwin CI failure #2004, except fortest_installation#2007

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Byron merged 6 commits intogitpython-developers:mainfromEliahKagan:cygwin-py39-venv
Feb 25, 2025

Conversation

EliahKagan
Copy link
Member

@EliahKaganEliahKagan commentedFeb 25, 2025
edited
Loading

I don't have a complete fix for#2004 yet, but since it's confirmed to fail on the main branch of this upstream repository (mentioned in#1988 (comment), though not due to#1988), I figured my partial fix might be worth putting into place. With these changes, the problem in the Cygwin test job turns from being unable to run any tests to being able to run all the tests but with one new failure compared to the way it was before#2004 (i.e. with one non-xfail failure).

This uses the officialpip bootstrap scripthttps://bootstrap.pypa.io/get-pip.py to installpip in the virtual environment. This allows the actual installation of GitPython and its dependencies, including all test dependencies, to complete without problems. Aside from tests that are expected to fail and are already marked xfail, all tests passexcept fortest_installation, which fails for the same reason. It presumably would also pass if the bootstrap script were used there too, but I believe the current intent of that test is to verify that installation succeeds when done in an ordinary way.

I am likewise reluctant to marktest_installation as xfail on Cygwin, unless that turns out to be truly necessary, because none of this happens on my local Cygwin environment on a Windows 10 development machine. It seems like it is triggered by some combination of changing packages and something wrong or otherwise specific about the Cygwin workflow.

This PR includes some changes to the workflow that are not actually necessary for the workaround it confers. The changes thatare needed are:

  • Refraining from havingvenv automatically attempt to installpip.
  • Installingpip in the virtual environment explicitly using the bootstrap script.
  • Installing thewget Cygwin package to download the bootstrap script without extra complexity or risking encoding/line-ending mismatches.

All the rest is to make the workflow run a little faster, andmaybe be slightly more robust in ways that are not relevant to#2004 and not very important. The most significant of them is switching back to the Cygwin installation action we had previously been using, which is less configurable (it does not support installing Cygwin packages at specific versions, which we had needed to do at one time), but which is faster because it does not first have to install Chocolatey to use it as an intermediary for installing Cygwin.

I kept those changes here because I think it's easier to iterate on this if it is left in. But if you'd like I'd be pleased to undo the nonessential changes, which can always be brought back later and separately. I have tested in7fd5f15 to make sure that this really works, so undoing those changes could be as simple as adding that one commit.

@Byron
Copy link
Member

Thanks so much for your help with this!

Despite the test failure, I think this already is a great improvement to what was there before so I'd merge it on that merit alone.
I don't know what the plan is with the failing test, my preference certainly would be to mark it as failing on CI for now.

@ByronByron merged commitfe7533e intogitpython-developers:mainFeb 25, 2025
23 of 24 checks passed
@EliahKagan
Copy link
MemberAuthor

I don't know what the plan is with the failing test, my preference certainly would be to mark it as failing on CI for now.

My thinking is that it could be marked conditionally xfail when the platform is Cygwin and theCI environment variable is set, if I do not manage to find a fix (or better workaround) in the next couple of days.

Byron reacted with thumbs up emoji

@EliahKaganEliahKagan deleted the cygwin-py39-venv branchFebruary 25, 2025 08:07
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 6, 2025
The previous commit pinned the python39-pip package, but it maystill be upgraded when installed automatically in the`python -m venv` step, since that step runs something like`python3.9 -Im ensurepip --upgrade --default-pip`.This replays4605dd6 to install pip in a separate step. Thisdiffers from the solution ingitpython-developers#2007 in that this does not use thebootstrap script.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 6, 2025
Together withgitpython-developers#2007, this works aroundgitpython-developers#2004, allowing all tests topass on Cygwin CI.Ingitpython-developers#2007, installation of the environment in which tests run wasfixed by downloading and running the `get-pip.py` bootstrap script.If we were to modify our helper that sets up the (separate) virtualenvironment in `test_installation` so that it does the same thing(or conditionally does so on CI, since the problem does not seem tohappen in local installations), that would likely "fix" this morethoroughly, allowing the test to pass.But part of the goal of the installation test is to test thatinstallation works in a typical environment on the platform it runson. So it is not obivous that making it pass in that way would bean improvement compared to marking it `xfail` with the exceptiontype that occurs due togitpython-developers#2004. So this just does that, for now.
@EliahKagan
Copy link
MemberAuthor

EliahKagan commentedMar 6, 2025
edited
Loading

I tried a few more things, but I did not find a good fix or better workaround, so I've opened#2009 to mark that one test conditionallyxfail.

Byron reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@EliahKagan@Byron

[8]ページ先頭

©2009-2025 Movatter.jp