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

Validate project paths #756

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Validate project paths #756

wants to merge 5 commits into from

Conversation

L1ghtmann
Copy link
Member

What does this implement/fix? Explain your changes.

See title

Does this close any currently open issues?

No

Any relevant logs, error output, etc?

Any other comments?

Issue of custom paths potentially containing spaces and not being escaped/quoted was brought up by Ethan Arbuckle a few months back in the Theos discord. Was unsure if a fix had been proposed elsewhere, so figured I'd pr mine. Open to feedback/changes.

Where has this been tested?

Operating System:

Linux (WSL)

Platform:

Target Platform:

Toolchain Version:

SDK Version:

makefiles/common.mk Outdated Show resolved Hide resolved
makefiles/common.mk Outdated Show resolved Hide resolved
Comment on lines +275 to +283
ifneq ($(call __validate,$(THEOS_BUILD_DIR)),$(_THEOS_TRUE))
$(error “$(THEOS_BUILD_DIR)” either contains spaces or does not exist)
else ifneq ($(call __format_validate,$(THEOS_OBJ_DIR)),$(_THEOS_TRUE))
$(error “$(THEOS_OBJ_DIR)” contains spaces which are not supported in project paths)
else ifneq ($(call __format_validate,$(THEOS_STAGING_DIR)),$(_THEOS_TRUE))
$(error “$(THEOS_STAGING_DIR)” contains spaces which are not supported in project paths)
else ifneq ($(call __format_validate,$(THEOS_PACKAGE_DIR)),$(_THEOS_TRUE))
$(error “$(THEOS_PACKAGE_DIR)” contains spaces which are not supported in project paths)
endif
Copy link
Member

Choose a reason for hiding this comment

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

I think we can afford to skip these checks

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure I agree, considering they're defined conditionally (?=), but am fine to revert if we want to take a more lenient approach.

@L1ghtmann L1ghtmann marked this pull request as draft March 13, 2024 14:56
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.

None yet

2 participants