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

Symlinks not resolved in get_main_dir(), preferences.plist not found when symlinked. #15

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

joraff
Copy link

@joraff joraff commented Sep 27, 2012

Hi Greg,

When someone installs reposado to, say, /usr/local/reposado then symlinks repo_sync and repoutil from /usr/bin, reposadocommon.py's get_main_dir() doesn't resolve that symlink and looks for preferences.plist in /usr/bin.

I would rather keep everything together and have my preferences.plist file in /usr/local/reposado/code, and only have the two symlinks in /usr/bin.

Easy fix, but it will likely break the configuration for existing reposado installs. I added some ugly logic that will give a preferences.plist file at the unresolved location precedence over one at the resolved path.

When determining the directory of the executables, symlinks will now be resolved into a full path before returning.

Useful for when you don't want a preferences.plist file in /usr/bin/, and instead want to symlink the exec's to another location such as /usr/local/reposado/code.
…installs

Get the preferences.plist file path will check first at a location with no resolved symlinks, and on an error, will resolve the symlinks and check again. This should allow a preferences.plist file to be at either location, with the unresolved path taking precedence.
@gregneagle
Copy link
Contributor

I won't merge this as-is because it could break existing installs. If you changed the logic to something like:

def prefsFilePath():
paths = [os.path.join(get_main_dir(), 'preferences.plist'), os.path.realpath(get_main_dir(True)), 'preferences.plist')]
for a_path in paths:
if os.path.exists(a_path):
return path
return paths[0]

So it would still default to the same location as before, but use the directory of the target of the symlink if it existed, I'd feel better about accepting this.

Or bring it up for discussion on the reposado list.

@joraff
Copy link
Author

joraff commented Sep 27, 2012

Greg, that's almost exactly what 1d5d27a addresses, albeit in a different way. I didn't mean to include the 1e8a4aa commit.

You'd still need a join on the second paths object, os.path.realpath(get_main_dir(True)), but I do like your logic better. I tend to use try's and opens to avoid race conditions, but that shouldn't be an issue here.

@n8felton
Copy link

@joraff Is this something you'd be willing to clean up the code to address Greg's concerns and also allow for it to be merged without conflicts? I'm interested in having this issue fixed as I currently have repo_sync and repoutil symlinked to /usr/local/bin

@gregneagle gregneagle changed the base branch from master to main July 1, 2020 21:58
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