FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

CI: Temporarily skip paths with spaces to avoid error by hugovk · Pull Request #105110 · python/cpython · GitHub

/ cpython Public

CI: Temporarily skip paths with spaces to avoid error - #105110

Merged
hugovk merged 3 commits into
python:mainfrom
hugovk:ci-workaround-path-spaces-error
May 30, 2023
Merged

CI: Temporarily skip paths with spaces to avoid error#105110
hugovk merged 3 commits into
python:mainfrom
hugovk:ci-workaround-path-spaces-error

Conversation

hugovk commented May 30, 2023

Copy link
Copy Markdown
Member

PRs like these:

Have NEWS files in subdirectories containing spaces:

  • Misc/NEWS.d/next/C API/2023-05-30-10-15-13.gh-issue-105071.dPtp7c.rst
  • Misc/NEWS.d/next/C API/2023-05-30-19-11-09.gh-issue-105107.YQwMnm.rst

But the https://github.com/Ana06/get-changed-files action (added in #103914) fails for such files with:

Error: One of your files includes a space. Consider using a different output format or removing spaces from your filenames. Please submit an issue on this action's GitHub repo.

We ran into this before and worked around it in #103019 by skipping Misc and its subdirs.

Let's do something similar here, by replacing Misc with a list of its subdirs that are absent a space.


See also python/core-workflow#186 to rename the C API and Core and Builtins directories to C-API and Core-and-Builtins to avoid this altogether.

hugovk commented May 30, 2023
edited
Loading

Copy link
Copy Markdown
Member Author

✅ No error with a dummy file at Misc/NEWS.d/next/C API/2023-05-30-21-34-29.gh-issue-105107.1mmbfA.rst (05cd028):

Run Ana06/get-changed-files@v2.2.0
Base commit: 457[1](https://github.com/python/cpython/actions/runs/5125091879/jobs/9217804839#step:5:1)eedca2f70768ddc4a2fd4fba2ae11b4c8037
Head commit: 05cd028fd4c56e3173b134b10dc947c3af471d82
All: 
Added: 
Modified: 
Removed: 
Renamed: 
Added or modified: 
Added, modified or renamed:

https://github.com/python/cpython/actions/runs/5125091879/jobs/9217804839

hugovk marked this pull request as ready for review May 30, 2023 19:06
hugovk requested a review from ezio-melotti as a code owner May 30, 2023 19:06
hugovk merged commit 4c77061 into python:main May 30, 2023
hugovk deleted the ci-workaround-path-spaces-error branch May 30, 2023 20:01
carljm added a commit to carljm/cpython that referenced this pull request May 30, 2023
* main:
  CI: Temporarily skip paths with spaces to avoid error (python#105110)
  pythongh-105071: add missing versionadded directive (python#105097)
  pythongh-80064: Fix is_valid_wide_char() return type (python#105099)
  Small speedup for dataclass __eq__ and __repr__ (python#104904)
  pythongh-103921: Minor PEP-695 fixes to the `ast` module docs (python#105093)
  pythongh-105091: stable_abi.py: Remove "Unixy" check from --all on other platforms (pythonGH-105092)

Copy link
Copy Markdown
Member

But the https://github.com/Ana06/get-changed-files action (added in #103914) fails for such files with

Maybe you should report the bug there?

hugovk commented May 31, 2023

Copy link
Copy Markdown
Member Author

It's a feature: https://github.com/Ana06/get-changed-files#usage says the default behaviour is to return files separated by spaces (format: 'space-delimited'), and will fail if any files contain spaces. They suggest using one of the other formats in such case (CSV, JSON).

(Let's continue discussion in #103914).

webknjaz added a commit to webknjaz/cpython that referenced this pull request May 31, 2023

Copy link
Copy Markdown
Member

@vstinner I've just submitted a proper fix: #105151

hugovk pushed a commit to hugovk/cpython that referenced this pull request Jun 1, 2023
hugovk pushed a commit to hugovk/cpython that referenced this pull request Jun 1, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL