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

Allow spaces in COB_CC #192

Closed

Conversation

ddeclerck
Copy link
Contributor

@ddeclerck ddeclerck commented Oct 9, 2024

This (small) PR allows the COB_CC variable to contain spaces, by adding quotes to any sprintf that use it.
Otherwise, one would need to escape all the spaces in COB_CC, and while this is acceptable "in general", this is slightly inconvenient, and besides the other variables do not work this way (for instance try escaping spaces in COB_CONFIG_DIR and you'll see it fails finding the config files - if the path contains spaces).

This of course works as long as COB_CC only contains the path to the C compiler and no argument - which should be the case since arguments should go in COB_CFLAGS & friends.

This would also allow us to get rid of the gcobc wrapper we have in our Windows installer (cf. your comment: "I'd like to know what "gcobc" is"), whose sole purpose is to call cobc with a space-escaped COB_CC.

EDIT: probably needs more work, considering the failures on Windows (had only tested on Linux so far)

@GitMensch
Copy link
Collaborator

As you also see in the failing tests - that doesn't work this way as we copy over verbatim what is in the CC variable - and things like -std and -march are commonly set that way.

Note: the more common way is to not use full paths for the binaries but instead prefix its path to PATH.

@GitMensch
Copy link
Collaborator

Should we close this (and you prefix PATH with the directory in gcobc)?

@ddeclerck
Copy link
Contributor Author

Should we close this (and you prefix PATH with the directory in gcobc)?

Yup, let me close this.

@ddeclerck ddeclerck closed this Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants