Skip to content
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

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

Open
Stebalien opened this issue Oct 22, 2018 · 11 comments
Open
Assignees

Comments

@Stebalien
Copy link
Collaborator

Specifically, here:

panic("not handling dep changes yet")

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

@Stebalien
Copy link
Collaborator Author

@travisperson could you look into this?

@travisperson
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

That would be great!

@travisperson
Copy link
Collaborator

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)

  • lib-c => $GOPATH/src/example.com/user/lib-c
  • lib-b => $GOPATH/src/example.com/user/prog1/.vendor/lib-b
  • lib-a => $GOPATH/src/example.com/user/prog1/.vendor/lib-a

prog2

  • lib-c => $GOPATH/src/example.com/user/lib-c
  • lib-a => $GOPATH/src/example.com/user/prog1/.vendor/lib-a

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
Copy link
Collaborator Author

Stebalien commented Nov 15, 2018

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:

  • gxed lib-c depends on lib-a (only).
  • latest lib-c depends on lib-a and lib-b.
  • lib-b depends on lib-c.

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
Copy link
Collaborator Author

What's the status of this?

@travisperson
Copy link
Collaborator

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
  4. Remove any import for which an exact link already exists
  5. Remove any import for which a link exists, which is a parent package of the import
  6. If the import is a parent itself of an already linked package, remove the child link
  7. Link remaining imports
  8. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@travisperson @Stebalien and others