-
Notifications
You must be signed in to change notification settings - Fork 110
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
Basic lockfile support #206
Changes from 1 commit
af9a0fa
b50d0e7
34c3ca1
1b13616
998d6cb
7a879b0
5180d4d
ae22fda
4473e80
9a823a2
fb9bcda
0756c14
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 |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package gxutil | ||
|
||
import ( | ||
"encoding/json" | ||
"io/ioutil" | ||
) | ||
|
||
type LockDep struct { | ||
// full ipfs path /ipfs/<hash>/foo | ||
Ref string | ||
|
||
// Mapping of dvcs import paths to LockData | ||
Deps map[string]LockDep | ||
} | ||
|
||
type LockFile struct { | ||
Language string | ||
Deps map[string]LockDep | ||
} | ||
|
||
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. Per our discussion: type Lock struct {
// Empty for the top level package.
// In the future, we could use IPNS!
Ref string
Language string
// maps language namespace -> name in language -> dependency
Deps map[string]map[string]Lock
} 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. (or we could keep it non-recursive, that was just a thought) 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. Why the "double map": The initial proposal was
Unfortunately, this falls apart when we need multiple namespaces. For example, with CGO, we might want (not well thought out): {
"github.com/ipfs/go-ipfs": {
"ref": "/ipfs/....",
"language": "go"
},
"libs/libsnark.a": {
"ref": "/ipfs/....",
"language": "rust"
}
} package thing
// #cgo LDFLAGS: -L${SRCDIR}/libs -llibsnark
import "C" The problem here is that there's the "go package" namespace and the "cgo" namespace (really, just the filesystem) and we need some way to disambiguate. 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 double map would be:
|
||
func LoadLockFile(lck *LockFile, fname string) error { | ||
data, err := ioutil.ReadFile(fname) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
return json.Unmarshal(data, lck) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import ( | |
const GxVersion = "0.12.1" | ||
|
||
const PkgFileName = "package.json" | ||
const LckFileName = "gx-lock.json" | ||
|
||
var installPathsCache map[string]string | ||
var binarySuffix string | ||
|
@@ -155,6 +156,73 @@ func (pm *PM) InstallDeps(pkg *Package, location string) error { | |
return pm.installDeps(pkg, location, make(map[string]bool)) | ||
} | ||
|
||
// InstallDeps recursively installs all dependencies for the given package | ||
func (pm *PM) InstallLock(lck *LockFile, location string) error { | ||
return pm.installLock(lck, location, make(map[string]bool)) | ||
} | ||
|
||
func (pm *PM) installLock(lck *LockFile, location string, complete map[string]bool) error { | ||
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. This seems to have pretty much the same functionality as 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. Yes, absolutely. |
||
done := make(chan LockDep) | ||
errs := make(chan error) | ||
ratelim := make(chan struct{}, 2) | ||
var count int | ||
pm.ProgMeter.AddTodos(len(lck.Deps)) | ||
for dvcsPath, dep := range lck.Deps { | ||
if complete[dvcsPath] { | ||
pm.ProgMeter.MarkDone() | ||
continue | ||
} | ||
|
||
count++ | ||
|
||
go func(i int, dep LockDep, dvcsPath string) { | ||
ratelim <- struct{}{} | ||
VLog("installing %s", dvcsPath) | ||
defer func() { <-ratelim }() | ||
pm.ProgMeter.AddEntry(dep.Ref, dvcsPath, "[fetch] <ELAPSED>"+dep.Ref) | ||
|
||
var final error | ||
for i := 0; i < 4; i++ { | ||
_, final = pm.GetPackageTo(dep.Ref, fmt.Sprintf("%s/%s", location, dvcsPath)) | ||
if final == nil { | ||
break | ||
} | ||
|
||
if !isTempError(final) { | ||
break | ||
} | ||
|
||
time.Sleep(time.Millisecond * 200 * time.Duration(i+1)) | ||
} | ||
if final != nil { | ||
pm.ProgMeter.Error(dep.Ref, final.Error()) | ||
errs <- fmt.Errorf("failed to fetch package: %s: %s", dep.Ref, final) | ||
return | ||
} | ||
|
||
pm.ProgMeter.Finish(dep.Ref) | ||
|
||
done <- dep | ||
}(count, dep, dvcsPath) | ||
} | ||
|
||
var failed bool | ||
for i := 0; i < count; i++ { | ||
select { | ||
case <-done: | ||
case err := <-errs: | ||
Error("[%d / %d ] parallel fetch: %s", i+1, len(lck.Deps), err) | ||
failed = true | ||
} | ||
} | ||
|
||
if failed { | ||
return errors.New("failed to fetch dependencies") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (pm *PM) SetProgMeter(meter *prog.ProgMeter) { | ||
pm.ProgMeter = meter | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,26 @@ var ( | |
) | ||
|
||
const PkgFileName = gx.PkgFileName | ||
const LckFileName = gx.LckFileName | ||
|
||
func LoadLockFile(path string) (*gx.LockFile, error) { | ||
if path == LckFileName { | ||
root, err := gx.GetPackageRoot() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
path = filepath.Join(root, LckFileName) | ||
} | ||
|
||
var lck gx.LockFile | ||
err := gx.LoadLockFile(&lck, 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. I'm assuming that this naming model was inherited from |
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return &lck, nil | ||
} | ||
|
||
func LoadPackageFile(path string) (*gx.Package, error) { | ||
if path == PkgFileName { | ||
|
@@ -111,6 +131,7 @@ func main() { | |
DiffCommand, | ||
InitCommand, | ||
InstallCommand, | ||
LockInstallCommand, | ||
PublishCommand, | ||
ReleaseCommand, | ||
RepoCommand, | ||
|
@@ -822,6 +843,35 @@ var depCheckCommand = cli.Command{ | |
}, | ||
} | ||
|
||
var LockInstallCommand = cli.Command{ | ||
Name: "lock-install", | ||
Usage: "Install deps from lockfile into vendor", | ||
Action: func(c *cli.Context) error { | ||
lck, err := LoadLockFile(LckFileName) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
pm.ProgMeter = progmeter.NewProgMeter(c.Bool("nofancy")) | ||
|
||
cwd, err := os.Getwd() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
ipath, err := gx.InstallPath(lck.Language, cwd, false) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if err := pm.InstallLock(lck, ipath); err != nil { | ||
return fmt.Errorf("install deps: %s", err) | ||
} | ||
|
||
return nil | ||
}, | ||
} | ||
|
||
var CleanCommand = cli.Command{ | ||
Name: "clean", | ||
Usage: "cleanup unused packages in vendor directory", | ||
|
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.
Could we document what's the difference between a
LockDep
and aDependency
?I mean, why are we using this new structure instead of extending
Dependency
or including it in this structure hierarchy? I'm not saying it should, but reading this automatically makes me thing of what we now call a dependency (inside thepackage.json
), and now I'll be seeing a different file that also talks about dependencies but with a different structure which can be confusing.I understand that conceptually both are referring to a package (that is a dep of another package) but with a different representation in a different context so it would be nice to make that link stronger.