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

use hard links for fonts instead of symlinks #2258

Merged
merged 1 commit into from
Jan 10, 2014

Conversation

rolandwalker
Copy link
Contributor

per an issue reported by @jgarber623

@rolandwalker
Copy link
Contributor Author

This is intended to address a new issue mentioned in ongoing discussion on #1860 .

@jgarber623 please let me know if this fixes your problem. It is working for me now (Mavericks).

PS it would have been cleaner to create a new issue. Sometimes people don't notice new discussion on an old PR.

@jgarber623
Copy link
Contributor

PS it would have been cleaner to create a new issue. Sometimes people don't notice new discussion on an old PR.

Fair enough. I'd thought of doing that, but wasn't sure if that was the preferred over commenting. Thanks for clarifying and for making the change!

@vitorgalvao vitorgalvao mentioned this pull request Dec 31, 2013
@rolandwalker
Copy link
Contributor Author

@jgarber623 just to be clear, I haven't entered the change yet. This PR passes build tests and works for me. Now I'm going to let it hang out for a few days and gather comment.

If possible, it would be good to verify that this approach fixes your issue before proceeding any further. If it is not easy for you to check out this branch and run it, could you try manually creating hard links (instead of cp as you may have done before) ? The most important part is: no -s flag after ln.

Example recipe

rm ~/Library/Fonts/SourceCodePro-*.otf
# here, stop and run Font Book to make sure Source Code Pro not present
ln /opt/homebrew-cask/Caskroom/font-source-code-pro/1.017/SourceCodePro_FontsOnly-1.017/OTF/SourceCodePro-*.otf ~/Library/Fonts/
# here, stop and run Font Book to make sure Source Code Pro IS present

@jgarber623
Copy link
Contributor

Followed those instructions and can confirm that the fonts show up in Font Book. 👍

@Peeja
Copy link
Contributor

Peeja commented Jan 3, 2014

Not sure I follow: What's the advantage of hard links over copying?

@rolandwalker rolandwalker mentioned this pull request Jan 3, 2014
@rolandwalker
Copy link
Contributor Author

@Peeja it's a good question. Same as the advantages of symlinks over copying, though a few implementation details are different.

First of all, only a single copy of each file is kept on disk, saving space. (Not the world's biggest consideration)

Second, the installer can detect if the target holds a link or an ordinary file. If an ordinary file, brew cask install declines to overwrite the target. The uninstaller can do the same, and decline to remove an ordinary file. brew-cask does not yet follow the link back to the caskroom to verify that brew-cask is the owner of the links, though it ought to.

Because of those similarities, this change has lower impact than switching to copies.

However, some users want to install copies. This is not completely out of the question, but requires more changes to the code. It would probably make the most sense as global option which applies to all artifacts. It should still be possible to keep some install/uninstall checks as described above by testing for identical files in the caskroom vs the installation directories.

@Peeja
Copy link
Contributor

Peeja commented Jan 3, 2014

How do you detect the difference between a hard link and an ordinary file? A hard link is an ordinary file, no?

@rolandwalker
Copy link
Contributor Author

Yes, I am eliding some details for simplicity. A filename is roughly an entry in a directory structure holding a reference to the file contents (contents are somewhere else in the filesystem). So when you first create a file you can think of that as a hard link with only 1 link, but we usually just call it a "file".

When you use ln to create a hard link, it creates a new entry in a directory structure, but that entry refers to the same underlying file contents as another entry. The OS keeps track of the link count so it knows when the contents can be deleted.

So, you can use a stat() call to count the number of currently-existing links to the underlying contents. That count distinguishes a "hard link" from an "ordinary file", but you are right that it is sort of a matter of semantics.

@Peeja
Copy link
Contributor

Peeja commented Jan 4, 2014

Right, but that doesn't actually let us reuse anything from the symlink method. They're both created with ln, but the method for figuring out if they point to the same thing is different.

If there's any interest in using non-symlinks for applications as well, I'd suggest going with copies instead of hard links, since you can't hard link an application (because it's a directory).

per an issue reported by @jgarber623.  Recast for compatibility
with Homebrew#2300.
@rolandwalker
Copy link
Contributor Author

I have updated this PR in light of the cleanup in #2300, merge conflict resolved.

@Peeja, I think your points are smart and thoughtful, and I am in favor of a user option to enable copying for all artifacts.

But I also think that God and Dennis created hardlinks for a reason. Now that code duplication is being removed maybe it will be clearer that this change does indeed reuse most of the symlink method. See the class Cask::Artifact::Hardlinked, derived from Cask::Artifact::Symlinked:
https://github.com/rolandwalker/homebrew-cask/blob/4123b05efac890b8263093a6d64cac3472c681f0/lib/cask/artifact/hardlinked.rb .

PS You can create directory hardlinks on some systems. Recent OS X + HFS+ is one example. They are used for the implementation of Time Machine. We were discussing possible uses recently in #2264.

@rolandwalker rolandwalker mentioned this pull request Jan 4, 2014
@Peeja
Copy link
Contributor

Peeja commented Jan 4, 2014

Oh. Well. In that case, neat!

@phinze
Copy link
Contributor

phinze commented Jan 10, 2014

Okay after discussing with @rolandwalker I think this is a reasonable move to make. We can always fall back to a ::Copied artifact type if we find that hard links give us too much trouble. And this is for newer functionality with a smaller surface area - so I'm cool to give it a shot. 🔫

phinze added a commit that referenced this pull request Jan 10, 2014
use hard links for fonts instead of symlinks
@phinze phinze merged commit 9135ff5 into Homebrew:master Jan 10, 2014
@rolandwalker rolandwalker deleted the hardlink_fonts branch February 8, 2014 21:49
@rolandwalker rolandwalker mentioned this pull request Oct 21, 2014
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants