search

whyrusleeping / gx

A package management tool
MIT License
1.88k stars 111 forks source link

Replacing a gx-lock vendored dep with a symlink causes `gx install` to panic #215

Open Stebalien opened 5 years ago

Stebalien commented 5 years ago

Specifically, here: https://github.com/whyrusleeping/gx/blob/89338e509426d2895f20086685cf4c77d64cc5df/gxutil/get.go#L74

We should skip this as this is the replacement for gx-go link.

Stebalien commented 5 years ago

@travisperson could you look into this?

travisperson commented 5 years ago

Okay, so this in this case we have something like $VENDOR/github.com/foo/bar pointing to $GO_PATH/src/github.com/foo/bar.

Are you saying we should just ignore it? After a lock-install I think we probably want to always be in the same consistent state, which would mean correcting any symlinks that are not pointing to the correct location.

Would doing this be to abrasive to current workflows?

travisperson commented 5 years ago

After reading the comment that references this issue it sounds like we do not want to correct these symlinks.

This panic is in place for changes to the ref in the lock file. This is what the panic references when it says not handling dep changes yet, it saying we are not handling updating lock dependencies when they change. To handle them, we would simply update the symlink to point to the new ref.

The issue we will face here is that we need to distinguish between previous runs of lock-install and user linking for development.

Stebalien commented 5 years ago

After reading the comment that references this issue it sounds like we do not want to correct these symlinks.

You're right, there are two ways to think about this:

  1. We want to correct this and, more than that, import the affected packages into gx.
  2. We don't want to correct this, just move on and build (because we're trying to emulate "gx-go link").

Maybe there's some way to indicate this? That is, some kind of gx-link.json file with a list of linked packages? That would depend on #216.

travisperson commented 5 years ago

Having a gx-link.json file would be a quick and easy solution to this. I think it would have to be list of all the linked package hashes.

During the install we would simply ignore any dependency with a ref listed.

gx link would call out to the language implementation, and update the gx-link.json file recording that it is linked.

Stebalien commented 5 years ago

That would be great!

travisperson commented 5 years ago

Just removing that panic isn't going to help at all to get that PR through without gx link / gx-go link working right?

Linking is a lot more complicated than just trying to establish a link from vendor to the package in gopath. If a package (lib-c) being linked has a transitive dependency (lib-a, through lib-b, a dep of lib-c), which is also a direct dependency of the main project, we also have to link the middle package (lib-b) to make sure that we are using the same lib-a everywhere.

Here is an example GOPATH setup: QmWB541fYhBMVXDJAeeufAnMxPMMzybP3mMQ812hJNKQqk

You can ipfs get that and set it as your GOPATH, and then build / run prog1 and prog2

Inside there are three library packages (lib-a, lib-b, lib-c) lib-c depends on lib-b, which depends on lib-a example.com/user/lib-{a,b,c} export Name which is an uppercase character of the library name, concat'd with it's dependency (lib-c, Name == "C" + b.Name) prog1 and prog2 has a .vendor (this would be where packages are cached) which contains copies of lib-{a,b,c} which export Name with a lowercase character.

prog1 has vendor links of (absolute here for clarity, they are relative in the object)

prog2

Both prog1 and prog2 print c.Name, a.Name

prog1 produces Cba a prog2 produces CBA a

I'm worried that without properly handling this kind of issue we will run into all sorts of strange behavior.

Stebalien commented 5 years ago

Just removing that panic isn't going to help at all to get that PR through without gx link / gx-go link working right?

~No.~ Right (damn ambiguity in english)

...

So, usually, lib-b will already be gxed (so we'll continue to use the gexed version). However, I guess there's the case where the dependency was just added. That is, you might have a case where:

So yeah, I think the only reasonable fix there is to make link recursive (sort of). That is, when linking a package, walk through all transitive dependencies and, if they don't already have a symlink in the vendor directory (pointing either into $GOPATH or the gx package cache), add a symlink pointing into the gopath (link it).

Does that sound reasonable?

Stebalien commented 5 years ago

What's the status of this?

travisperson commented 5 years ago

Hey, sorry for the lack of updates. I've been working on something I think we will work, or at least get close.

It basically does this:

1) Link the desired package in 2) Get all imports for the package 3) Remove stdlib imports 3) Remove any import for which an exact link already exists 4) Remove any import for which a link exists, which is a parent package of the import 5) If the import is a parent itself of an already linked package, remove the child link 6) Link remaining imports 7) Repeat from step (1) for all imports at step (6).

I have a feeling though that this is missing something. I thought about trying to only link "base" packages, (eg: github.com/foo/bar), but that has a lot of issues itself.