-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Filesystem] Add feature to create hardlinks for files #15458
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
Conversation
{ | ||
if (!$this->exists($originFile)) { | ||
throw new FileNotFoundException(null, 0, null, $originFile); | ||
} elseif (!is_file($originFile)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can make it a regular if
condition.
b43aa9c
to
029faeb
Compare
Open question: Would it be ok to aim at backporting this for 2.3 and up? I have seen similar small useful features being backported before which is why I ask. For FOSHttpCache that would be helpful to not have to require 2.8. |
@andrerom no it is not. New features are never backported, unless it is necessary to fix a security issue (or a PHP compatibility issue) |
ok, clear. |
} | ||
|
||
if (!is_file($originFile)) { | ||
throw new FileNotFoundException("Origin file '{$originFile}' is not a file"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use sprintf rather than interpolation, as mandated by our coding standards: https://symfony.com/doc/current/contributing/code/standards.html#structure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 0af9468
This actually depends on the use case. Saying that hardlinks is better is misleading. For instance, if you use hardlinks on a PHP file, |
8a35fee
to
0af9468
Compare
Right, corrected that statement, I was actually never considering the use case of linking executable php files with actual code as opposed to cache files or binary files. I would have expected it to report |
fae8b93
to
89e46ad
Compare
Not sure why this PR staled. @andrerom Can you rebase on current 2.8? I will then be able to merge this into master. |
@fabpot done, failure with fabbot.io seems to be unrelated to changes here. |
/** | ||
* @var null Flag for hard links on Windows | ||
*/ | ||
protected static $linkOnWindows = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be private, and imo there's no need for the docblock here (same below for $symlinkOnWindows
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 584cafe
I did not remove the phpdoc, the other properties have doc so I rather fixed these to say null|bool
to give correct hints for IDE's.
Thank you @andrerom. |
Todo:
Why
Symlinks are good for directories, but for files hardlinks are sometime a better match when needing to represent same file in several locations.
One use case for this feature is for multi tagging of Http Cache when running against FileSystem. Multi tagging is used in FoSHttpCache and supported when running Varnish, but not with Symfony HTTPCache, yet.. With hardlinks combined with meta information containing tags within the file, lookup and reverse lookup is thus easily possible.
What
Introduces method
hardlink()
with similar behavior assymlink
, difference being:symlink
removes existing target if not the same, but usesfileinode
to compare target & source