Skip to content

Conversation

@danielbachhuber
Copy link
Member

Fixes #119

@danielbachhuber
Copy link
Member Author

@schlessera I'm not sure realpath() is the way to go because:

The running script must have executable permissions on all directories in the hierarchy, otherwise realpath() will return FALSE.

Thoughts?

@schlessera
Copy link
Member

Maybe use our own helper functions, something like this: https://3v4l.org/9Ga8c

@schlessera
Copy link
Member

Drush uses this: https://github.com/webmozart/path-util

@gitlost
Copy link
Contributor

gitlost commented Jul 19, 2017

As it's very Unixy could just do something like

if ( '~/' === substr( $package_dir, 0, 2 ) && ( $home = getenv( 'HOME' ) ) ) {
	$package_dir = $home . substr( $package_dir, 1 );
}

@schlessera
Copy link
Member

@gitlost Yes, I suggested a more robust version of that here: https://3v4l.org/9Ga8c

@danielbachhuber danielbachhuber changed the title Run $package_dir through realpath() for consistent paths Convert '~/' when supplied in $package_dir Jul 21, 2017
@danielbachhuber
Copy link
Member Author

@schlessera @gitlost Updated. I went with the simpler approach to begin with.

@schlessera
Copy link
Member

I'm not sure why you'd want to solve this partially only. It is highly probable someone will want to use a relative path with this as well, and just create a new bug report at that point.

Additionally, having ~ work, and not . or .. makes it even worse in that it is inconsistent as well.

@gitlost
Copy link
Contributor

gitlost commented Jul 23, 2017

I don't see how a relative path or . or .. won't just work - just tried --dir=~/../blah and it worked...

@schlessera
Copy link
Member

Oh, okay, I just assumed it would create issues as well. All good then.

@schlessera schlessera merged commit 8735e26 into master Jul 23, 2017
@schlessera schlessera deleted the 119-realpath branch July 23, 2017 14:15
schlessera added a commit that referenced this pull request Aug 9, 2022
Convert '~/' when supplied in `$package_dir`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants