Skip to content

Conversation

@seliopou
Copy link

  • Use link instead of cp -l on OS X, since the -l flag is not supported.
  • Change shebang to lookup bash in PATH. Ran across this problem on FreeBSD 9.0.

seliopou added 2 commits June 30, 2012 19:56
Not sure why 'cp -l' was used to begin with, but kept that behavior for
non-OS X systems in case that was in intentional.
This was problematic on FreeBSD 9.0.
@lmartinking
Copy link
Owner

Can you split out the second commit (cd926b7), and I'll merge that one.

@seliopou
Copy link
Author

seliopou commented Jul 8, 2012

Happy to do that, but I have two questions:

  1. Why is cp -l used instead of ln in local_file_add?
  2. What in particular about the first commit do you object to?

My goal in both commits is to make this project more portable. Since day-to-day I use OS X, I have an interest in seeing that portability issue resolved. Can you provide some feedback on the first commit so both myself and others using OS X can use this project out of the box?

@lmartinking
Copy link
Owner

If you use cp -l it will hard link the file if possible. I'm not sure what functionality the OS X ln command provides.

link seems to belong to coreutils, I am not sure if that is present by default in OS X.

@seliopou
Copy link
Author

ln will make a hard link and is available in OS X. Should I change local_file_add to use ln instead of cp -l for all platforms?

@seliopou
Copy link
Author

Ping.

Any plans on merging this, or any suggestions on how to change the merge request?

@seliopou
Copy link
Author

One last ping.

The way that use use ln in this project will create a hard link, just like cp -l will on OS X.

At the time of this writing you have two open pull requests for OS X support (as well as FreeBSD). Do you intend on accepting patches to that will fix portability issues, or should those of us that need to work across multiple platforms organize a fork that will accept these sorts of changes?

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.

2 participants