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

CI fails when changed files have parens in file path #23605

Closed
1 of 2 tasks
bsmth opened this issue Jan 13, 2023 · 8 comments
Closed
1 of 2 tasks

CI fails when changed files have parens in file path #23605

bsmth opened this issue Jan 13, 2023 · 8 comments
Assignees
Labels
infra Infrastructure issues (npm, GitHub Actions, linting) for this repo

Comments

@bsmth
Copy link
Member

bsmth commented Jan 13, 2023

Update:

We have agreed to ban parentheses from file names and renamed the offending paths:

CI checks are being put in place to ban new files from being added with parentheses:


Failing workflow:
https://github.com/mdn/content/actions/runs/3908866554/jobs/6679383140

Changed files:

  • files/en-us/glossary/descriptor_(css)/index.md
  • files/en-us/glossary/round_trip_time_(rtt)/index.md
  • files/en-us/glossary/xhr_(xmlhttprequest)/index.md

Refers to line 5 in the script, which looks like:

run: |
# The reason this script isn't in `package.json` is because
# you don't need that script as a writer. It's only used in CI
# and it can't use the default CONTENT_ROOT that gets set in
# package.json.
yarn build ${{ env.GIT_DIFF_CONTENT }}

i.e.:

yarn build files/en-us/glossary/descriptor_(css)/index.md

Where the following should work:

yarn build "files/en-us/glossary/descriptor_(css)/index.md"

Maybe we should try wrapping these file paths in quotes

CCing @yin1999 @schalkneethling

@bsmth bsmth added the bug label Jan 13, 2023
@bsmth bsmth self-assigned this Jan 13, 2023
@bsmth bsmth added the CI label Jan 13, 2023
@yin1999
Copy link
Member

yin1999 commented Jan 13, 2023

I'll have a look on this

@yin1999
Copy link
Member

yin1999 commented Jan 13, 2023

I'll have a look on this

Try to fix this in #23608

@OnkarRuikar
Copy link
Contributor


Command syntax, mainly shell expansions, are a little bit different for each shell -- newer shells like Z shell and fish are more forgiving than Bash. So scripts/commands that work in these shell may not work exactly the same on bash. If you are using Apple products then most likely you are using Z shell and not Bash.
Our GitHub workflows use ubuntu-22.04 (need to check the shell and version). This is important because, I think the expansions are not happening the same as they happen in our local environment.

@estelle
Copy link
Member

estelle commented Jan 13, 2023

It looks like no files with a ( can be edited right now. See #23620. I updated via the Github UI, and it's failing due to the parenthesis.

@OnkarRuikar
Copy link
Contributor

OnkarRuikar commented Jan 14, 2023

It looks like no files with a ( can be edited right now. See #23620. I updated via the Github UI, and it's failing due to the parenthesis.

The change in PR is small and not affecting anything, so for now we can force merge it with failing workflow. We already do this for infrastructure related PRs e.g. #22874.
Then try to merge #20470 as soon as we can.

@Josh-Cena Josh-Cena added infra Infrastructure issues (npm, GitHub Actions, linting) for this repo and removed CI labels Jan 15, 2023
@bsmth
Copy link
Member Author

bsmth commented Jan 17, 2023

Thanks, all. I went ahead and merged Estelle's PR (#23602) in the meantime. I agree it would be good to get consensus on renaming files soon: #20470.

@yin1999
Copy link
Member

yin1999 commented Jan 24, 2023

Hi @bsmth, we may close this issue as we have merged #20470.

I'm also going to close the fixing PR.

@bsmth
Copy link
Member Author

bsmth commented Jan 24, 2023

Thanks @yin1999 - I've updated the issue description with the related PRs.

@bsmth bsmth closed this as completed Jan 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
infra Infrastructure issues (npm, GitHub Actions, linting) for this repo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants