add touch fn

[lread]

To support failing fast on invalid time, internal ->file-time now throws when input time is not a convertable to FileTime.

Added internal fs/now to support redeffing current time for tests.

This initial implementation does not pass touch opts through to create-file to avoid the complexity of dealing with :posix-file-permissions.

Closes #232

Please answer the following questions and leave the below in as part of your PR.

• This PR corresponds to an issue with a clear problem statement.

• This PR contains a test to prevent against future regressions

• I have updated the CHANGELOG.md file with a description of the addressed issue.

[borkdude]

Maybe a bit of an edge case, but:

The GNU touch atomically creates or opens an existing file. We have a bit of a race condition between the exists? check and the creation when it doesn't it. When another process creates the file in between, then our create can throw, which isn't caught. A way to deal with this is using the following interop:

  (-> (java.nio.channels.FileChannel/open
        (as-path path)
        (into-array [java.nio.file.StandardOpenOption/CREATE
                     java.nio.file.StandardOpenOption/WRITE]))
      (.close))

This ensures the file will exists, either opening an existing one or creating one.

[lread]

Yes, great idea! I will make it so.

[lread]

Oh, I'll also add tests for touching existing dirs too. And adjust accordingly.

[borkdude]

The updated approach has another problem.

If another process deletes the file after our create and before our set-last-modified-time, we'd get a NoSuchFileExcpetion.

To do the touch functionality atomically, we could do:

  (with-open [chan (java.nio.channels.FileChannel/open
                     path
                     (into-array java.nio.file.OpenOption
                       (cond-> [java.nio.file.StandardOpenOption/CREATE
                                java.nio.file.StandardOpenOption/WRITE]
                         nofollow-links (conj java.nio.file.LinkOption/NOFOLLOW_LINKS))))]
    (-> (.getFileAttributeView chan java.nio.file.attribute.BasicFileAttributeView)
        (.setTimes time nil nil)))

Actually (.setTimes time time nil) would be even more compatible with GNU touch since touch also sets the access time.

[lread]

Ah... interesting. I looked into setting time on the file channel open, but learned that attrs were only for posix-like attributes. TIL about how to get a file attribute view on a channel... wait... I don't see a .getFileAttributeView on FileChannel.

We could get the file attribute view on the path, but is there any advantage to doing this in the with-open block?

I'm not sure we can achieve a race-condition-free touch in Java(?).

We could set the last acccess time though...

[borkdude]

I just tried to find another way to deal without the try/catch, but we probably can't. Your approach seems valid. We could use BasicFileAttributeView.setTimes(time, time, nil) to set the access time. Perhaps:

(try (BasicFile.../setTimes ....) (catch NoSuchFileException ... (create the file) (recur))

?

[borkdude]

Nope, recur isn't going to work in try/catch. So the only thing left is very minor: the access time. We could also ignore that and merge yours.

[lread]

I'm guessing that setting last access time is not that important for bb fs.

But maybe I should make some mention that bb fs touch is not race-condition-free in docstring?

[borkdude]

So:

(try
    (.setTimes ... time time nil)
    (catch NoSuchFileException _
      (with-open [_ (FileChannel/open path ...)])
      (touch path opts)))

[borkdude]

sorry I missed the accompanying text:

I guess we could make it race condition free by calling ourselves when hitting the exception

[lread]

Lemme explore your retry idea:

case 1 - file/dir exists

• attempt to set times
• file/dir exists
• done

case 2 - file/dir does not exist

• attempt to set times
• file doesn't exist exception
• create file
• call-self
• attempt to set times
• done.

case 3 - race condition

• attempt to set times
• file doesn't exist exception
• create file
• call-self
• {something else deletes file}
• attempt to set times
• file doesn't exist exception
• create file
• call self
• attempt to set times
• done

Unlikely but: the retry could end up retrying forever (or until the stack blows).

Option 1: don't retry, don't document

Option 2: don't retry, document in docstring,
Caller can implement retry logic in their code.

Option 3: auto-retry with recursion (technically can blow stack)

Option 4: auto-retry with loop (technically can hang)

Option 5: add hard retry limit

Option 6: add configurable retry limit option

I think I like option 2. What would you prefer?

As for setting last access time, I have no strong preference, but I would lean torwards not bothering.

[borkdude]

Yes, option 2.

[borkdude]

Thanks!