untested version where reads are through new abstraction #23

EECOLOR · 2016-03-20T21:11:45Z

No description provided.

EECOLOR · 2016-03-21T12:43:27Z

A major source of errors was a change in how I wrote the Permission container.

Note that I removed the effect from resolve(path: Path): Key. It made the semantics a bit complex if the same effect was used to obtain the Key and data or metadata. For example, at what point do you say: 'this does not exist?'

I first removed the resolve method entirely, but then realized that it's important we keep the act of 'moving into the file system' explicit. I do not yet have a clear vision of where this separation comes in handy, but my gut feeling tells me it's something we need to keep. On top of that, you added it and you have way more experience in this domain.

If we do reintroduce effects for obtaining the key, they should be a separate set of effects.

EECOLOR · 2016-03-21T12:44:19Z

There is still some errors which I'll try to chase down. I might add the (stripped) test suite I'm using if you feel that's of some worth.

EECOLOR · 2016-03-21T18:05:31Z

Ok, this branch now behaves the same (as far as I can tell) as master

EECOLOR · 2016-03-21T18:07:34Z

core/src/main/scala/api/sfs.scala

+
+  /** Allows for effects
+   */
+  type M[_]


Maybe we should have a different name for this. On the other hand, M[_] clearly communicates no assumptions are made about.

I like M[_]. I think the purpose is clear from the interface.

EECOLOR · 2016-03-21T18:18:09Z

core/src/main/scala/jio/package.scala

@@ -186,6 +210,7 @@ package object jio extends DecorateAsScala with DecorateAsJava {
  type OpenOption         = jnf.OpenOption
  type Path               = jnf.Path
  type PathDirStream      = jnf.DirectoryStream[Path]
+  type NoSuchFileException  = jnf.NoSuchFileException


indentation

paulp · 2016-03-21T18:23:45Z

Adding the test suite you're using would be great.

EECOLOR · 2016-03-21T18:24:25Z

I'll do that based on master

EECOLOR · 2016-03-21T18:46:29Z

#25 pulls in the test suite

EECOLOR · 2016-03-21T19:17:11Z

I removed the structural type implicit.

The location of files might need some change. Which reminds me, I think we should have some guidelines of where to place what. I don't have a very strong preference, but I do need some guidelines in order to put things in the right (and consistent) places. It would also help me figure out where to find things.

In any case. When you are at a normal (code friendly) computer again I'd love to hear your feedback.

paulp · 2016-03-21T20:08:29Z

core/src/main/scala/api/attributes.scala

+  object Permissions {
+    implicit def empty: api.Empty[Permissions] =
+      api.Empty(Permissions(false, false, false, false, false, false, false, false, false))
+  }


I don't care if it's changed at this second, but I am a big disbeliever in the "nine booleans" approach to this sort of thing and would like this to go away eventually. The set of possible permissions is vastly larger than these nine bits - these are, specifically, the unix filesystem interface permissions, and as a result there's little reason not to store it just as the unix filesystem does, which is to say as an integer of some width. Not sure if I already shared this, but here's one from one of my fuse projects. This includes the translation between java and fuse.

final case class UnixAttrs(perms: UnixPerms, times: UnixTimes, owners: UnixOwners) final case class UnixOwners(uid: Int, gid: Int) final case class UnixTimes(created: FileTime, modified: FileTime, accessed: FileTime) final class UnixPerms private (val mask: Long) { def bits: Set[Long] = FuseBitsSet filter (bit => (bit & mask) != 0) def java: Set[PosixFilePermission] = bits map FuseToJava def fuse: ModeWrapper = new ModeWrapper(mask) def attr: FileAttribute[jSet[PosixFilePermission]] = PosixFilePermissions asFileAttribute java.asJava def isExecutable = java(OTHERS_EXECUTE) | java(GROUP_EXECUTE) | java(OWNER_EXECUTE) override def toString = permString(mask) } object UnixPerms { private lazy val Part = "[-r][-w][-x]" private lazy val PermRegex = ("^" + (Part * 3) + "$").r private lazy val JavaBits = Vector[PosixFilePermission]( OWNER_READ, OWNER_WRITE, OWNER_EXECUTE, GROUP_READ, GROUP_WRITE, GROUP_EXECUTE, OTHERS_READ, OTHERS_WRITE, OTHERS_EXECUTE ) private lazy val FuseBits = Vector[Long]( S_IRUSR, S_IWUSR, S_IXUSR, S_IRGRP, S_IWGRP, S_IXGRP, S_IROTH, S_IWOTH, S_IXOTH ) private lazy val FuseBitsSet: Set[Long] = FuseBits.toSet private lazy val Letters: Vector[Char] = "rwxrwxrwx".toVector private lazy val JavaToFuse = JavaBits zip FuseBits toMap private lazy val FuseToJava = FuseBits zip JavaBits toMap private def fuseMask(perms: String): Long = ( for ((perm, ch) <- FuseBits zip perms.toSeq ; if ch != '-') yield perm ).foldLeft(0L)(_ | _) private def permString(mask: Long): String = ( for ((perm, ch) <- FuseBits zip Letters) yield if ((mask & perm) == 0) '-' else ch ) mkString "" private def fuseMask(path: Path): Long = ( path.direct.permissions map JavaToFuse ).foldLeft(0L)(_ | _) def apply(perms: String): UnixPerms = perms match { case PermRegex() => apply(fuseMask(perms)) case _ => sys.error(s"Invalid permission string: $perms") } def apply(bits: Long): UnixPerms = new UnixPerms(bits) def apply(m: ModeWrapper): UnixPerms = apply(m.mode) def apply(p: Path): UnixPerms = apply(fuseMask(p)) def apply(f: File): UnixPerms = apply(f.toPath) def `777` = apply("rwxrwxrwx") def `755` = apply("rwxr-xr-x") def `644` = apply("rw-r--r--") def `444` = apply("r--r--r--") def `555` = apply("r-xr-xr-x") }

I would love to put it in. The problem here is that I would cut them up and use them on two sides of the abstraction. So the question is, what to put in the middle, in api.attributes?

Hmm, thinking about it, it would be UnixPerms. I'll take a look in a minute to see how it fits in.

Ok, I put it in

I meant: I have put it in

EECOLOR · 2016-03-21T20:20:25Z

Slightly related to the discussion on permissions. FuseFs is a consumer filesystem, it consumes (or uses other file systems). It demands filesystems of a certain type. It however can not express what metadata it can consume. Similarly, other filesystems might 'require' certain metadata to be present. I am not sure if the cost of having the ability to preserve types, or state type / attribute requirements, would be too high. But it might be something we need to think about.

paulp · 2016-03-21T20:41:12Z

core/src/main/scala/api/sfs.scala

-  def metadata(key: Key): Metadata
-  def lookup(key: Key): Data
+  def metadata(key: Key): M[Metadata]
+  def lookup(key: Key): M[Data]


I think it remains to be seen whether having an unconstrained M[_] is worth the price of admission, but we'll find out. I also think it's likely that these three methods should be either two or one, even if performance may drive us away from it. The only method I see a need for is Path => Metadata. Paths which don't exist would return NoMetadata; the primary key is exposed through the metadata; the data is exposed through the metadata. This subsumes all manner of effects as well, because a metadata key need not produce Data, it could produce M[Data].

I think this is true even after we have read/write filesystems. The interface through which you perform filesystem writes is (wait for it) available through the metadata.

I have been thinking about that as well and I like that idea.

What I'm troubled with, is how to state metadata requirements. It will be a constant source / documentation lookup if we can not make clear what is supposed / expected to be present in the metadata for something to work.

I'm thinking something along the lines of:

val underlying: FileSystem { type RecognisedMetadata = Size :+: Atime :+: ... }

This subsumes all manner of effects as well, because a metadata key need not produce Data, it could produce M[Data].

Yeah, I'm eager to explore this with the write side of the story. Even if we don't have any means of communication metadata capabilities.

But for now I think we should try and see if we can do this version.

I mean, see if we can get this branch into master

As you might have noticed I removed the effects in #30

In what sense are you saying fuse is blocking?

All functions require you to return something. For example getattr requires you to tell it when a file does not exist. If this information came in through a Future, we would have to block on it. I'm unsure of the read semantics: "Returns the number of bytes transferred", will it try again if the size was larger than the amount of bytes transferred?

If you don't have to touch filesystems when adding a new effect, you have invented magic. Effects aren't composable. There's no way to say a priori how they interact with existing effects.

Can make this more concrete, maybe with a (fictional) example?

Can make this more concrete, maybe with a (fictional) example?

How about you first give me a set of effects you imagine being useful here. Which I wouldn't mind hearing anyway.

All functions require you to return something.

The unix filesystem interface and by extension fuse require you to tell all kinds of lies. There's no way to say "I don't know what the size of this file is" for example, but it's critical not to have to compute every single file in its entirety simply to respond to ls -l. I have a bunch of ideas for mitigating this, but it would be a long process because it requires not only out of band data (e.g. extended attributes) but tools which understand it.

However, I'm trying to arrange things here so the metadata returned is explicitly that information which does not require blocking. You should always get an instant response to ls -l. The instant response may kick off an asynchronous process which obtains the missing metadata and gives you a better answer the next time, but it isn't going to make you wait for it.

Real life on real filesystems is already like this because (effectively) every file is mutable and might change or disappear at any time. So it's not like this is a step backward. And suffuse-aware programs can take advantage of the aforementioned out-of-band data immediately: they don't have to act as if the size of that file is really -1 bytes or Long.MaxValue or whatever it is which breaks the least stuff.

Playing the cards we're dealt, here.

What you say there about an asynchronous process to give you a better answer reminds me a lot about Stream.toString and cats.Show[Stream] with regards to referential transparency: typelevel/cats#775

paulp · 2016-03-21T21:58:09Z

core/src/main/scala/jio/package.scala

@@ -134,10 +153,35 @@ package object jio extends DecorateAsScala with DecorateAsJava {
  )

  def file(s: String, ss: String*): File        = ss.foldLeft(new File(s))(new File(_, _))
-  def path(s: String, ss: String*): Path        = ss.foldLeft(jnf.Paths get s)(_ resolve _)
+  def path: String => Path                      = jnf.Paths get _


What's the motivation for this? And why not file as well?

Type inference, the difference between def x(x:X): Y and def x: X => Y and it was not used with more than one argument.

I did not change file because I wanted to keep the changes to a minimum. Happy to change file as well.

paulp · 2016-03-21T22:37:01Z

There are bits of this I want to change but it's good enough to merge, thanks.

untested version where reads are through new abstraction

untested version where reads are through new abstraction

58d9eb1

EECOLOR assigned paulp Mar 20, 2016

EECOLOR added 4 commits March 20, 2016 23:14

resolve did not what I thought it did

2ef3825

Merge branch 'master' into read-fs

cd98b4a

permissions were not correctly set

e858aca

removed the effect from obtaining a key

39e2ef2

🐛 size and block count are only available for files

dd86dad

EECOLOR reviewed Mar 21, 2016
View reviewed changes

moved Wrapper and made it more generic

62691a4

EECOLOR reviewed Mar 21, 2016
View reviewed changes

fixed indentation

38b58a9

moved Functor and removed free implicit based on structural type

a78f5e1

paulp reviewed Mar 21, 2016
View reviewed changes

removed Permissions in favor of UnixPerms

1dd0dc9

paulp reviewed Mar 21, 2016
View reviewed changes

paulp added a commit that referenced this pull request Mar 21, 2016

Merge pull request #23 from suffuse/read-fs

bd9069f

untested version where reads are through new abstraction

paulp merged commit bd9069f into master Mar 21, 2016

paulp deleted the read-fs branch March 21, 2016 22:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

untested version where reads are through new abstraction #23

untested version where reads are through new abstraction #23

EECOLOR commented Mar 20, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

EECOLOR Mar 21, 2016

paulp Mar 21, 2016

EECOLOR Mar 21, 2016

paulp commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

paulp Mar 21, 2016

EECOLOR Mar 21, 2016

EECOLOR Mar 21, 2016

EECOLOR Mar 21, 2016

EECOLOR commented Mar 21, 2016

paulp Mar 21, 2016

EECOLOR Mar 21, 2016

EECOLOR Mar 21, 2016

EECOLOR Mar 21, 2016

EECOLOR Mar 21, 2016

EECOLOR Mar 22, 2016

EECOLOR Mar 22, 2016

paulp Mar 23, 2016

paulp Mar 23, 2016

dwijnand Mar 23, 2016

paulp Mar 21, 2016

EECOLOR Mar 21, 2016

paulp commented Mar 21, 2016

untested version where reads are through new abstraction #23

untested version where reads are through new abstraction #23

Conversation

EECOLOR commented Mar 20, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulp commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

EECOLOR commented Mar 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EECOLOR commented Mar 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulp commented Mar 21, 2016