gobuffalo / packr

The simple and easy way to embed static files into Go binaries.
MIT License
3.41k stars 196 forks source link

packr: do not reset modTime when creating a file #163

Open zachgersh opened 5 years ago

zachgersh commented 5 years ago

Problem I ran into

packr here reset the mod time of a file no matter how you have added that file to the box.

What I would expect

packr would store most of the metadata about a file when packing (for in memory) or if reading off disk would take the file info and embed it into its own file object.

Would be glad to throw a PR here if this is acceptable.

Reproduction

This will give you a different mod time every single time you resolve

package main

import (
    "fmt"

    "github.com/gobuffalo/packr/v2"
)

func main() {
    box := packr.New("test", "./test")
    err := box.AddString("binary", "this is a test binary")
    if err != nil {
        panic(err)
    }

    for {
        f, err := box.Resolve("binary")
        if err != nil {
            panic(err)
        }

        info, err := f.FileInfo()
        if err != nil {
            panic(err)
        }

        fmt.Println(info.ModTime())
    }
}
markbates commented 5 years ago

A PR would definitely be welcome.


Mark Bates On Feb 21, 2019, 11:42 AM -0500, Zach Gershman notifications@github.com, wrote:

Problem I ran into packr here reset the mod time of a file no matter how you have added that file to the box. What I would expect packr would store most of the metadata about a file when packing (for in memory) or if reading off disk would take the file info and embed it into its down file object. Would be glad to throw a PR here if this is acceptable. Reproduction This will give you a different mod time every single time you resolve package main

import ( "fmt"

   "github.com/gobuffalo/packr/v2"

)

func main() { box := packr.New("test", "./test") err := box.AddString("binary", "this is a test binary") if err != nil { panic(err) }

   for {
           f, err := box.Resolve("binary")
           if err != nil {
                   panic(err)
           }

           info, err := f.FileInfo()
           if err != nil {
                   panic(err)
           }

           fmt.Println(info.ModTime())
   }

} — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

zachgersh commented 5 years ago

@markbates did you have any thoughts about how this sort of metadata would be embedded into the format that is generated when you pack? Reading off the disk and relaying the file back admittedly easier. I was having a tiny bit of trouble finding a nice place to insert this sort of metadata.

markbates commented 5 years ago

yeah, I'm not sure either. I've been trying to figure that out too. the best I've come up with is figuring out a way to prepend metadata when it's packed and then read that metadata when it's unpacked, but I haven't dived into it any more than that.

ciphercules commented 5 years ago

Hi @markbates, I've been working with @zachgersh on this, and we've come up with two possible formats.

  1. We include a "*.info" map entry for each file that contains file metadata as a string. The benefits of this approach is that I don't believe it will break current implementation, and we can just add the ability to read the extra "*.info" map entry.
    
    package packrd

import ( "github.com/gobuffalo/packr/v2" "github.com/gobuffalo/packr/v2/file/resolver" )

var _ = func() error { const gk = "263ac315b3401aa0fdd11824b806530c" g := packr.New(gk, "") hgr, err := resolver.NewHexGzip(map[string]string{ "e9cb145202973e9e3d5fdd3a3540e230": "1f8b08000000000000ff4aca49cce002040000ffff2d32c45005000000", "e9cb145202973e9e3d5fdd3a3540e230.info": "file-info", }) if err != nil { panic(err) } g.DefaultResolver = hgr

func() {
    b := packr.New("test", "./test")
    b.SetResolver("foo.txt", packr.Pointer{ForwardBox: gk, ForwardPath: "e9cb145202973e9e3d5fdd3a3540e230"})
}()

return nil

}()


2.  We create a new struct that holds both the file contents and the file info to encapsulate this data. The benefits of this approach is that it is easier to understand, and the drawbacks are that we would have to change at least `NewHexGzip` to support the struct. 
```go
package packrd

import (
    "github.com/gobuffalo/packr/v2"
    "github.com/gobuffalo/packr/v2/file/resolver"
)

type Gzip struct {
    Content  string
    FileInfo string
}

var _ = func() error {
    const gk = "263ac315b3401aa0fdd11824b806530c"
    g := packr.New(gk, "")
    hgr, err := resolver.NewHexGzip(map[string]Gzip{
        "e9cb145202973e9e3d5fdd3a3540e230": Gzip{
            Content:  "1f8b08000000000000ff4aca49cce002040000ffff2d32c45005000000",
            FileInfo: "file info",
        },
    })
    if err != nil {
        panic(err)
    }
    g.DefaultResolver = hgr

    func() {
        b := packr.New("test", "./test")
        b.SetResolver("foo.txt", packr.Pointer{ForwardBox: gk, ForwardPath: "e9cb145202973e9e3d5fdd3a3540e230"})
    }()

    return nil
}()
markbates commented 5 years ago

I like them both, leaning more towards the second. We wouldn't have to necessarily change NewHexGzip, but instead, we could create a different function to handle the new syntax. That would keep existing packr from breaking, and add support for file infos, so basically, create a new Store that handles it.

My bigger question, is how to reflect this in the interfaces to ensure it works with different stores. Thoughts on that?

ciphercules commented 5 years ago

Could you elaborate more on the different store interfaces? I'm not sure I'm understanding the problem fully.

markbates commented 5 years ago

For example, there are 3 defined in packr itself https://godoc.org/github.com/gobuffalo/packr/v2/jam/store each one needs to make sure to pack the file info and the resolvers also need to make sure to unpack the file info into each file.

So, if I use the Legacy store, how is the data packed into that store so it doesn't lose file info data, and how is it pulled out of that store.

You've outlined a really nice solution for that particular resolver (the store being the Disk store that generates that file.

I just want to make sure the any touch points that aren't just this one resolver are capable of supporting file info. Resolvers can also be implemented by end users. For example how does this impact the Disk or InMemory resolvers?

There might not be any issues, but I just want to make sure everything is thought through.

timraymond commented 5 years ago

@Polar-Beard @zachgersh Still interested in taking this on?

kmanley commented 4 years ago

+1 for storing actual modtime with each file in the packr.Box. That would make it easier to implement cache control in http servers embedding static assets via packr.