steffenfritz / FileTrove

FileTrove indexes files and creates metadata from them.
https://filetrove.fritz.wtf
GNU Affero General Public License v3.0
22 stars 5 forks source link

[BUG] Recent filesystem changes break the build #43

Closed ross-spencer closed 3 months ago

ross-spencer commented 3 months ago

Describe the bug A clear and concise description of what the bug is.

Windows and MacOS specific imports cannot be understood in Linux after recent changes: https://github.com/steffenfritz/FileTrove/blob/5e5c16b19c5d5a9eab840331619889f82891a329/filesystem.go

To Reproduce Steps to reproduce the behavior:

Building fileTrove in Linux results in error, for example, build cannot find syscall.NewLazyDLL

Expected behavior A clear and concise description of what you expected to happen.

Builds work on all OS.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here, e.g. test data

My workaround was to separate the OS dependent functions into the following additional files:

With the specific build commands for MacOS and Windows at the top.

Then in filesystem.go, the platform dependent naming can be removed in favor of a generic function name that will work on each independent build environment:

diff --git a/filesystem.go b/filesystem.go
index f6f990c..6465db7 100644
--- a/filesystem.go
+++ b/filesystem.go
@@ -9,27 +9,25 @@ import (
    "os"
    "runtime"
    "strings"
-   "syscall"
-   "unsafe"
 )

 // GetFileSystemType returns the filesystem type of the specified mount point or volume.
 func GetFileSystemType(path string) (string, error) {
    if runtime.GOOS == "linux" {
-       return getLinuxFileSystemType(path)
+       return getFileSystemType(path)
    }
    if runtime.GOOS == "windows" {
-       return getWindowsFileSystemType(path)
+       return getFileSystemType(path)
    }
    if runtime.GOOS == "darwin" {
-       return getMacFileSystemType(path)
+       return getFileSystemType(path)
    } else {
        return "", fmt.Errorf("unsupported operating system: %s", runtime.GOOS)
    }
 }

 // getLinuxFileSystemType returns the filesystem type of the specified mount point on Linux.
-func getLinuxFileSystemType(path string) (string, error) {
+func getFileSystemType(path string) (string, error) {
    file, err := os.Open("/proc/mounts")
    if err != nil {
        return "", err
@@ -45,44 +43,3 @@ func getLinuxFileSystemType(path string) (string, error) {
    }
    return "", fmt.Errorf("mount point not found: %s", path)
 }
-
-// getWindowsFileSystemType returns the filesystem type of the specified volume on Windows.
-func getWindowsFileSystemType(volumeName string) (string, error) { <-- moved to filesystem_windows.go and renamed getFileSystemType
-   kernel32 := syscall.NewLazyDLL("kernel32.dll")
-   getVolumeInformation := kernel32.NewProc("GetVolumeInformationW")
-
-   var volumeSerialNumber, maximumComponentLength, fileSystemFlags uint32
-   var fileSystemNameBuffer [255]uint16
-
-   volumeNameBuffer := syscall.StringToUTF16Ptr(volumeName)
-
-   _, _, err := getVolumeInformation.Call(
-       uintptr(unsafe.Pointer(volumeNameBuffer)),
-       uintptr(unsafe.Pointer(&fileSystemNameBuffer[0])),
-       uintptr(len(fileSystemNameBuffer)),
-       uintptr(unsafe.Pointer(&volumeSerialNumber)),
-       uintptr(unsafe.Pointer(&maximumComponentLength)),
-       uintptr(unsafe.Pointer(&fileSystemFlags)),
-       0,
-       0,
-   )
-   if err != nil {
-       return "", err
-   }
-
-   fileSystemName := syscall.UTF16ToString(fileSystemNameBuffer[:])
-   return fileSystemName, nil
-}
-
-// getMacFileSystemType returns the filesystem type of the specified mount point on macOS.
-func getMacFileSystemType(path string) (string, error) { <-- moved to filesystem_macos.go and renamed getFileSystemType
-   var stat syscall.Statfs_t
-
-   err := syscall.Statfs(path, &stat)
-   if err != nil {
-       return "", err
-   }
-
-   fsType := string(stat.Fstypename[:])
-   return fsType, nil
-}
steffenfritz commented 3 months ago

As mentioned in #45 some commits break builds at the moment. As more people have a look into this and try it out, I have to be more "branch-true".

And you are right, I will split it into different files (there is also a local branch where I did that) and add a build pragma on top of every file, e.g. //go:build linux

But I am in general not sure if we want the filesystem type identified by FileTrove. Do you have an opinion here, @ross-spencer ? This is driven by #30

ross-spencer commented 3 months ago

But I am in general not sure if we want the filesystem type identified by FileTrove

With the caveat I am still learning about filetrove, I feel this in general a really useful thing for tooling to report on and I don't think we have any digipres tools doing this already? Yet, there are definitely issues transferring data across file-system boundaries. So +1 from me!

steffenfritz commented 3 months ago

Build issue solved