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 subproject dirs in subdirectories in the source tree again #2890

Merged
merged 2 commits into from
Jan 8, 2018
Merged

Allow subproject dirs in subdirectories in the source tree again #2890

merged 2 commits into from
Jan 8, 2018

Conversation

ximion
Copy link
Contributor

@ximion ximion commented Jan 7, 2018

The previous change disallowed any subdirectories for subproject dirs,
and therefore broke a couple of projects making use of that.
This change still prevents people from setting subproject dirs that are
not in the project's source tree, while allowing to specify any path
within the project's directory again.

This fixes the regression described in #2719

@jpakkane
Copy link
Member

jpakkane commented Jan 7, 2018

This should also have these:

        if not isinstance(spdirname, str):
            raise InterpreterException('Subproject_dir must be a string')
        if '..' in spdirname:
            raise InterpreterException('Subproject_dir must not contain a ".." segment.')

@jpakkane jpakkane added this to the 0.44.1 milestone Jan 7, 2018
@jpakkane
Copy link
Member

jpakkane commented Jan 7, 2018

And path absoluteness should be checked with os.path.isabs.

The previous change disallowed any subdirectories for subproject dirs,
and therefore broke a couple of projects making use of that.
This change still prevents people from setting subproject dirs that are
not in the project's source tree, while allowing to specify any path
within the project's directory again.

Resolves: #2719
@ximion
Copy link
Contributor Author

ximion commented Jan 7, 2018

I was actually wondering why os.path.isabs wasn't used in the first place ^^
This issue is resolved now, and the string-type check is added as well.
Thanks for the review!

@jpakkane
Copy link
Member

jpakkane commented Jan 7, 2018

The .. check is not there. It is needed because of things like dummy/../../../forbiddendir.

This is important so people can not trick Meson to select a
subproject_dir that is not in the project's source directory.
It also ensures a string is used for the path.
@ximion
Copy link
Contributor Author

ximion commented Jan 7, 2018

Eeek, yes, I didn't think of that (and though that section was already in there).
Patch is updated.

@jpakkane jpakkane merged commit ca2db0f into mesonbuild:master Jan 8, 2018
if spdirname.startswith('.'):
raise InterpreterException('Subproject_dir must not begin with a period.')
if '..' in spdirname:
Copy link
Member

Choose a reason for hiding this comment

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

Note: please use pathlib.PurePath().parts so that names like subprojects/some..dir/ don't fail this test.

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.

3 participants