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

Jump to bottom

Enable Windows build on GH actions #87

Merged
merged 3 commits into from
Jul 26, 2021
Merged

Enable Windows build on GH actions #87

merged 3 commits into from
Jul 26, 2021

Conversation

justvanrossum
Copy link
Collaborator

Trying to see if this "just works". Perhaps it can fix #86.

@justvanrossum
Copy link
Collaborator Author

So that didn't work. I wonder if it's a pytest issue, similar to pytest-dev/pytest#7951

  =================================== ERRORS ====================================
  ________________________ ERROR collecting test session ________________________
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\runner.py:311: in from_call
      result: Optional[TResult] = func()
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\runner.py:341: in <lambda>
      call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\main.py:690: in collect
      for direntry in visit(str(argpath), self._recurse):
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:606: in visit
      yield from visit(entry.path, recurse)
  users\runneradmin\appdata\local\temp\tmp1rj1ttgu\lib\site-packages\_pytest\pathlib.py:591: in visit
      for entry in os.scandir(path):
  E   OSError: [WinError 1921] The name of the file cannot be resolved by the system: 'c:\\Documents and Settings\\All Users\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data\\Application Data'
  =========================== short test summary info ===========================
  ERROR  - OSError: [WinError 1921] The name of the file cannot be resolved by ...
  !!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
  ======================== 1 error in 598.94s (0:09:58) =========================

@anthrotype
Copy link
Member

hm, but do we have a recursive symlink?

@justvanrossum
Copy link
Collaborator Author

Not that I know. It's crashing in visiting 'c:\Documents and Settings\All Users\Application Data', and I don't know what it's doing there.

@justvanrossum justvanrossum mentioned this pull request May 18, 2021
Closed
@justvanrossum
Copy link
Collaborator Author

So now that #88 fixed #86, this PR is no longer needed, but it is still nice to have, so leaving this open, in case anyone knows how to fix it.

@khaledhosny khaledhosny marked this pull request as draft July 24, 2021 12:54
justvanrossum and others added 2 commits July 26, 2021 01:07
@justvanrossum @khaledhosny
enable Windows build
4d0f2d4
@khaledhosny
Use pushd instead of cd in CIBW_TEST_COMMAND
4adcfb3 
GitHub Actions seems to clone the project on the D: drive, but cd on
Windows does not change the drive (we would need to use /d option).
@khaledhosny
Copy link
Collaborator

This is now working. It was failing because we have CIBW_TEST_COMMAND: "cd {project} && pytest", and on GitHub Actions {project} points to a directory on the D: drive, but cd on Windows does not change drive by default and silently fails, so we we running pytest on the root of C: drive. Using pushd seems to work on the three platforms.

Should we drop AppVeyor use and merge this, or is there any thing else needed?

@khaledhosny khaledhosny marked this pull request as ready for review July 26, 2021 01:38
@justvanrossum
Copy link
Collaborator Author

Wow, fantastic, @khaled! I'd say "yes" to drop AppVeyor, but @anthrotype's opinion is worth more than mine.

(Btw. thank you so much for your recent flood of fixes and improvements!)

anthrotype
anthrotype approved these changes Jul 26, 2021
View reviewed changes
Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

thank you Khaled!

@anthrotype
Copy link
Member

"yes" to drop AppVeyor

+1

@anthrotype
Copy link
Member

before removing appveyor you need to make sure that the wheel uploading still works
i think the GHA workflow uses the TRAVIS_OS_NAME env var to create a unique (per-platform) zip file name and right now there's an if statement that assumes either macosx or linux.

@anthrotype
Copy link
Member

anthrotype commented Jul 26, 2021 •
edited 

-    - name: Set up environment variables
-      run: |
-        if [ "macos-latest" == "${{ matrix.os }}" ]; then
-         echo "TRAVIS_OS_NAME=osx" >> $GITHUB_ENV
-        else
-          echo "TRAVIS_OS_NAME=ubuntu" >> $GITHUB_ENV
-        fi
      - name: Install dependencies
        run: pip install cibuildwheel
      - name: Build Wheels
        run: python -m cibuildwheel --output-dir wheelhouse
      - uses: actions/upload-artifact@v2
        with:
-        name: uharfbuzz-${{ env.TRAVIS_OS_NAME }}
+        name: uharfbuzz-${{ matrix.os }}
        path: wheelhouse/*.whl

@khaledhosny khaledhosny merged commit 4745154 into harfbuzz:master Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anthrotype anthrotype anthrotype approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No win_amd64 wheels were produced for last release (0.16.0)
3 participants
@justvanrossum @anthrotype @khaledhosny