-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Instrumented path based operations using hooks defined in Checker
#325
Changes from 7 commits
2f357dc
8c02044
ec303a9
169f66a
95fe56a
b06e184
bfb2319
3b4d504
9e32e77
5bb76cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,8 +22,12 @@ import scala.util.Try | |
* ignore the destination if it already exists, using [[os.makeDir.all]] | ||
*/ | ||
object makeDir extends Function1[Path, Unit] { | ||
def apply(path: Path): Unit = Files.createDirectory(path.wrapped) | ||
def apply(path: Path): Unit = { | ||
checker.value.onWrite(path) | ||
Files.createDirectory(path.wrapped) | ||
} | ||
def apply(path: Path, perms: PermSet): Unit = { | ||
checker.value.onWrite(path) | ||
Files.createDirectory( | ||
path.wrapped, | ||
PosixFilePermissions.asFileAttribute(perms.toSet()) | ||
|
@@ -38,6 +42,7 @@ object makeDir extends Function1[Path, Unit] { | |
object all extends Function1[Path, Unit] { | ||
def apply(path: Path): Unit = apply(path, null, true) | ||
def apply(path: Path, perms: PermSet = null, acceptLinkedDirectory: Boolean = true): Unit = { | ||
checker.value.onWrite(path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original implementation creates any necessary enclosing directories via Here the checker is only checking for write access on the full path. Don't we need to apply it to the enclosing directories as well? I see this would entail providing a shim for |
||
// We special case calling makeDir.all on a symlink to a directory; | ||
// normally createDirectories blows up noisily, when really what most | ||
// people would want is for it to succeed since there is a (linked) | ||
|
@@ -84,6 +89,8 @@ object move { | |
atomicMove: Boolean = false, | ||
createFolders: Boolean = false | ||
): Unit = { | ||
checker.value.onWrite(from) | ||
checker.value.onWrite(to) | ||
if (createFolders && to.segmentCount != 0) makeDir.all(to / up) | ||
val opts1 = | ||
if (replaceExisting) Array[CopyOption](StandardCopyOption.REPLACE_EXISTING) | ||
|
@@ -176,6 +183,8 @@ object copy { | |
createFolders: Boolean = false, | ||
mergeFolders: Boolean = false | ||
): Unit = { | ||
checker.value.onRead(from) | ||
checker.value.onWrite(to) | ||
if (createFolders && to.segmentCount != 0) makeDir.all(to / up) | ||
val opts1 = | ||
if (followLinks) Array[CopyOption]() | ||
|
@@ -191,18 +200,17 @@ object copy { | |
s"Can't copy a directory into itself: $to is inside $from" | ||
) | ||
|
||
def copyOne(p: Path): file.Path = { | ||
def copyOne(p: Path): Unit = { | ||
val target = to / p.relativeTo(from) | ||
if (mergeFolders && isDir(p, followLinks) && isDir(target, followLinks)) { | ||
// nothing to do | ||
target.wrapped | ||
} else { | ||
Files.copy(p.wrapped, target.wrapped, opts1 ++ opts2 ++ opts3: _*) | ||
} | ||
} | ||
|
||
copyOne(from) | ||
if (stat(from, followLinks = followLinks).isDir) walk(from).map(copyOne) | ||
if (stat(from, followLinks = followLinks).isDir) for (p <- walk(from)) copyOne(p) | ||
Comment on lines
-194
to
+213
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A minor optimization that eliminates the creation of an unused collection. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice optimization! Would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying to avoid this. |
||
} | ||
|
||
/** This overload is only to keep binary compatibility with older os-lib versions. */ | ||
|
@@ -311,6 +319,7 @@ object copy { | |
object remove extends Function1[Path, Boolean] { | ||
def apply(target: Path): Boolean = apply(target, false) | ||
def apply(target: Path, checkExists: Boolean = false): Boolean = { | ||
checker.value.onWrite(target) | ||
if (checkExists) { | ||
Files.delete(target.wrapped) | ||
true | ||
|
@@ -322,6 +331,7 @@ object remove extends Function1[Path, Boolean] { | |
object all extends Function1[Path, Unit] { | ||
def apply(target: Path) = { | ||
require(target.segmentCount != 0, s"Cannot remove a root directory: $target") | ||
checker.value.onWrite(target) | ||
|
||
val nioTarget = target.wrapped | ||
if (Files.exists(nioTarget, LinkOption.NOFOLLOW_LINKS)) { | ||
|
@@ -350,6 +360,8 @@ object exists extends Function1[Path, Boolean] { | |
*/ | ||
object hardlink { | ||
def apply(link: Path, dest: Path) = { | ||
checker.value.onWrite(link) | ||
checker.value.onWrite(dest) | ||
Files.createLink(link.wrapped, dest.wrapped) | ||
} | ||
} | ||
|
@@ -359,6 +371,12 @@ object hardlink { | |
*/ | ||
object symlink { | ||
def apply(link: Path, dest: FilePath, perms: PermSet = null): Unit = { | ||
checker.value.onWrite(link) | ||
checker.value.onWrite(dest match { | ||
case p: RelPath => link / RelPath.up / p | ||
case p: SubPath => link / RelPath.up / p | ||
case p: Path => p | ||
}) | ||
val permArray: Array[FileAttribute[_]] = | ||
if (perms == null) Array[FileAttribute[_]]() | ||
else Array(PosixFilePermissions.asFileAttribute(perms.toSet())) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,9 @@ object zip { | |
deletePatterns: Seq[Regex] = List(), | ||
compressionLevel: Int = java.util.zip.Deflater.DEFAULT_COMPRESSION | ||
): os.Path = { | ||
checker.value.onWrite(dest) | ||
// check read preemptively in case "dest" is created | ||
for (source <- sources) checker.value.onRead(source.src) | ||
Comment on lines
+49
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, the implementation should be refactored to defer file creation until all data is collected. |
||
|
||
if (os.exists(dest)) { | ||
val opened = open(dest) | ||
|
@@ -268,6 +271,7 @@ object unzip { | |
excludePatterns: Seq[Regex] = List(), | ||
includePatterns: Seq[Regex] = List() | ||
): Unit = { | ||
checker.value.onWrite(dest) | ||
for ((zipEntry, zipInputStream) <- streamRaw(source, excludePatterns, includePatterns)) { | ||
val newFile = dest / os.SubPath(zipEntry.getName) | ||
if (zipEntry.isDirectory) os.makeDir.all(newFile) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package os | ||
|
||
import scala.annotation.StaticAnnotation | ||
|
||
/** | ||
* Annotation to mark experimental API, which is not guaranteed to stay. | ||
*/ | ||
class experimental extends StaticAnnotation {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
I am a restricted cow |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
I am restricted cow | ||
Hear me moo | ||
I weigh twice as much as you | ||
And I look good on the barbecue |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Contents of restricted folder one |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Contents of restricted nested A |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Contents of restricted nested B |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
I am a restricted cow |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
../folder1 |
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.
How about adding these for ergonomics?
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.
I think for now we can punt on these, since it's a pretty advanced feature so being easy to use isn't as important