Skip to content

Fix bug with symlink creation #3028

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

Merged
merged 3 commits into from
Aug 17, 2017
Merged

Fix bug with symlink creation #3028

merged 3 commits into from
Aug 17, 2017

Conversation

agjohnson
Copy link
Contributor

Previously, this was executing a command using a shell, but without properly
escaping the input. This instead passes a list in to Popen, to ensure escape is
working properly.

# TODO this should use os.symlink, not a call to shell. For now,
# this passes command as a list to be explicit about escaping
# spaces.
status, _, stderr = run(['ln', '-nsf', docs_dir, symlink])
Copy link
Member

Choose a reason for hiding this comment

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

All of the other calls in this file seem to not pass a list here. Should those be fixed, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might as well. I'll push up some cleanup and fix some of those too

Previously, this was executing a command using a shell, but without properly
escaping the input. This instead passes a list in to Popen, to ensure escape is
working properly.
@agjohnson agjohnson force-pushed the fix-subproject-symlink branch from ddb8037 to 06cb5fd Compare August 3, 2017 15:09
@agjohnson
Copy link
Contributor Author

I updated this to also change the rest of the symlink calls and refactor out the shell kwarg, as this was unused by all the calls, and is superseded mostly by passing in a list instead of a string.

@agjohnson agjohnson added the PR: hotfix Pull request applied as hotfix to release label Aug 3, 2017
@agjohnson agjohnson merged commit e5652c1 into master Aug 17, 2017
@agjohnson agjohnson deleted the fix-subproject-symlink branch August 17, 2017 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: hotfix Pull request applied as hotfix to release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants