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

deal with () in filenames by escaping them (NO_JIRA) #27

Merged
merged 9 commits into from
Mar 14, 2025

Conversation

jasonccole
Copy link
Contributor

@jasonccole jasonccole commented Mar 12, 2025

This change should 'escape' file names when calling subprocess commands so that they dont fall down on linux platforms (as is currently happening in this PR) for filenames containing certain characters:

See: https://github.com/ccdc-confidential/cpp-apps-main/actions/runs/13816850366/job/38652179948?pr=4142

I'd appreciate a careful check - seems to work on my machine, but I've no easy way to test it in the 'live' repo.

Ironically the PR fails in checks ... because of the python version :-}

Update: PR now works since I've changed the python version in the PR. Horribly self-referential!

Update 2: following a chat with Simon & Matt - I've changed it so that we instead use subprocess without Shell=True - this should resolve the syntactic issues with files

@jasonccole jasonccole requested a review from siong-chin March 14, 2025 11:32
Copy link
Contributor

@siong-chin siong-chin left a comment

Choose a reason for hiding this comment

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

A few f-strings no longer needed. Otherwise should be fine.

@jasonccole
Copy link
Contributor Author

A few f-strings no longer needed. Otherwise should be fine.

Will remove

@jasonccole jasonccole merged commit dcd736a into main Mar 14, 2025
6 checks passed
@jasonccole jasonccole deleted the no_jira_sanitise_filenames branch March 14, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants