feat: support for Go modules to imports [needs unit test review] #1265

switchupcb · 2021-09-27T11:47:36Z

Timeline

A timeline of this pull request has been created.

July 5, 2023: Still waiting on unit test revision.
Jan 22, 2023: Current state and scope of PR is discussed: "UNIT TESTS THAT DON'T SETUP THE INTERPRETER CORRECTLY WILL INCORRECTLY FAIL."
Aug 9, 2022: Debugging of CI occurs.
Aug 6, 2022: New Implementation is developed due to oversights in the replaced and original implementation.
June 21, 2022: Call for maintainer review after long period of inactivity. @mvertes recommends vendoring third-party imports because supporting Go modules introduces an added dependency: "It [support for third-party modules using a go package] requires discussions internally on our side."

Support Go Modules in Imports

This pull request aims to solve these issues:

These give the error error: unable to find source related to: "github.com/switchupcb/copygen/cli/models". Either the GOPATH environment variable, or the Interpreter.Options.GoPath needs to be set.

Problem

There were a few issues with the pkgDir method.

The paths printed are all backwards.
The function runs three times for one call? Presumably to change the separator.
Setting goPath from options makes the program hang.
References GTA5.
Irrelevant variable root is returned.

OUTPUT of PROBLEM CODE
pkgDir(), previousRoot(), effectivePkg()
RPATH generator\vendor
DIR src\generator\vendor\github.com\switchupcb\copygen\cli\models
goPath
importPath github.com/switchupcb/copygen/cli/models
effectivePkg() generator\github.com\switchupcb\copygen\cli\models
RPATH vendor
DIR src\vendor\github.com\switchupcb\copygen\cli\models
goPath
importPath github.com/switchupcb/copygen/cli/models
EFFECTIVE github.com\switchupcb\copygen\cli\models
RPATH vendor
DIR src\vendor\github.com\switchupcb\copygen\cli\models
goPath
importPath github.com/switchupcb/copygen/cli/models
effectivePkg() github.com\switchupcb\copygen\cli\models
prevRoot
interp.opt.filesystem &{}
FRAG github.com\switchupcb\copygen\cli\models

THE PROGRAM HANGS ON THIS FUNCTION WHEN GOPATH IS SET

// Find the previous source root (vendor > vendor > ... > GOPATH).
func previousRoot(filesystem fs.FS, rootPath, root string) (string, error) {
    rootPath = filepath.Clean(rootPath)
    parent, final := filepath.Split(rootPath)
    parent = filepath.Clean(parent)

    // TODO(mpl): maybe it works for the special case main, but can't be bothered for now.
...

Fix

Remove the problem code.
Don't reverse the filepath.
Use cross-platform implementation via filepath.Join.
Provide more informative error messages.

Solution

Go modules are located in $GOPATH/pkg/mod. Vendor files are located somewhere in the project relative to the go.mod file. You state that "In all other cases, absolute import paths are resolved from the GOPATH and the nested "vendor" directories." Therefore, we only need to search in a few places.

The GOPATH src (for regular imports)
- Installed during Go installation.
The GOPATH pkg/mod (for Go modules and/or "vendor" dirs)
- Installed during go get or placed.
The relative case.
- Did not touch; however the new case may implement.
The vendor files are in the project.

For 1, 2, and 4: https://pkg.go.dev/golang.org/x/tools/go/packages#Package gives a list of files we can filter for the absolute import path. 3 is untouched, BUT should work with the given method. For #856 specifically, the unsafe issue is fixed by searching in the GOTOOLDIR (completed with 9605bc7).

The following code finds a relative path's absolute path from the current working directory. The other code regarding relative path wasn't commented, so I did not implement it.

absgopath, err := filepath.Abs(rPath)
    if err != nil {
        return nil, fmt.Errorf("There was an error retrieving the absolute filepath for the relative path.". rPath)
    }

Test

If GOCACHE is not set, you will get this error:

err: exit status 1: stderr: build cache is required, but could not be located: GOCACHE is not defined and %LocalAppData% is not defined`.

While running the interpreter, you might receive

stderr: go: creating work dir: mkdir C:\WINDOWS\go-build1065008415: Access is denied.

IF an import can't be found AND you aren't in administrator (from the command line).

go test ./_test 
_test\bad0.go:1:1: expected 'package', found println
package github.com/traefik/yaegi/_test
        imports github.com/traefik/yaegi/_test/c1
        imports github.com/traefik/yaegi/_test/c2
        imports github.com/traefik/yaegi/_test/c1: import cycle not allowed
_test\pkgname0.go:4:2: no required module provides package guthib.com/bar; to add it:
        go get guthib.com/bar
_test\pkgname0.go:5:2: no required module provides package guthib.com/baz; to add it:
        go get guthib.com/baz
_test\ipp_as_key.go:5:2: no required module provides package guthib.com/tata; to add it:
        go get guthib.com/tata
_test\ipp_as_key.go:4:2: no required module provides package guthib.com/toto; to add it:
        go get guthib.com/toto

Note: Did not run install.sh cause its not supported.

Disclaimer

These changes allow me to use yaegi imports without yaegi extract.

CLAassistant · 2021-09-27T11:47:41Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ldez
❌ switchupcb

switchupcb seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

switchupcb · 2021-09-28T08:14:02Z

Update

I can confirm that the import is found. However, I still receive an error while running when I run the interpreter without the imported symbols.

panic: C:\Program Files\Go\src\unsafe\unsafe.go:192:1: CFG post-order panic: runtime error: index out of range [3] with length 3

StackTrace

Issue fixed...

switchupcb · 2021-09-28T08:20:14Z

This was solved with the following code:

i := interp.New(interp.Options{GoPath: os.Getenv("GOPATH"), GoCache: goCache, GoToolDir: build.ToolDir})
	i.Use(stdlib.Symbols)
	if _, err := i.Eval(source); err != nil {
		return nil, fmt.Errorf("An error occurred loading the template file: %v\n%v", absfilepath, err)
	}

Therefore, the stdlib symbols must be used to import external or vendored packages.

switchupcb · 2021-09-28T10:06:44Z

I'm pretty sure this pull request works. My only trouble now is with the library:

// an error occurs here.
bar := v.Interface().(func(string) string)

panic: interface conversion: interface {} is func(struct { Filepath string; Loadpath string; Template struct { Headpath string; Funcpath string }; Package string; Imports []string; Functions []struct { Name string; To []struct { Package string; Name string; VariableName string; Fields []*unsafe2.dummy; Options struct { Import string; Pointer bool; Depth int; Deepcopy string; Custom map[string]interface {} } }; From []struct { Package string; Name string; VariableName string; Fields []*unsafe2.dummy; Options struct { Import string; Pointer bool; Depth int; Deepcopy string; Custom map[string]interface {} } }; Options struct { Custom map[string]interface {} } } }) string, not func(models.Generator) string

switchupcb · 2021-10-05T08:40:39Z

@mvertes

switchupcb · 2021-10-19T03:04:08Z

Update

This pull request can use Go Module imports without an issue finding the source. If @mpl can let me know all the edge cases he is solving in importSrc and updates the tests to use importSrc instead of its sub-methods I can improve this.

Error

When you attempt to use an object (not loaded by symbols) from the import, you will receive:

panic: C:\Program Files\Go\src\unsafe\unsafe.go:192:1: CFG post-order panic: runtime error: index out of range [3] with length 3

This is solved by using the following code.

if err := i.Use(unsafe.Symbols); err != nil {
  // ...
}

switchupcb · 2021-10-26T06:25:30Z

When a user attempts to print an object from the Go Module, this error occurs in cfg.go at line 2629 in method arrayTypeLen(). This happens when you call fmt.Println() with a custom module type:

panic: reflect: call of reflect.Value.Int on uintptr Value

goroutine 1 [running]:
reflect.Value.Int(...)
        C:/Program Files/Go/src/reflect/value.go:1334
github.com/traefik/yaegi/interp.arrayTypeLen(0x17dad20, 0xc0001027e0)
        C:/Users/User/Documents/Github/yaegi/interp/cfg.go:2647 +0x33c
...see comment edit for full stacktrace

These users understand cfg.go: @mvertes @nrwiersma @mpl

bobfang1992 · 2021-11-08T22:10:36Z

How far did you get with this? Is this at least working on your machine? I really hope this got more traction and get merged eventually.

switchupcb · 2022-01-18T22:42:37Z

How far did you get with this? Is this at least working on your machine? I really hope this got more traction and get merged eventually. @bobfang1992

I will test it with the next update of Copygen but that issue is a responsibility of those users (as they coded cfg.go which is 2730 lines) [@mvertes @nrwiersma @mpl ]. There's not really anything else I can do on my end.

c0b41 · 2022-02-12T12:13:14Z

Any Update @mvertes

switchupcb · 2022-03-25T00:10:21Z

When a user attempts to print an object from the Go Module, this error occurs in cfg.go at line 475. This happens when you call fmt.Println() with a custom module type (string alias):

package template

import (
	"fmt"

	"github.com/switchupcb/copygen/cli/generator" // third party module
	"github.com/switchupcb/copygen/cli/models" //  extracted module
)

func Generate(gen *models.Generator) (string, error) {
	content := string(gen.Keep) + "\n"
	fmt.Println(generator.GenerateFunction) // custom type in third party module

	for i := range gen.Functions {
		content += Function(&gen.Functions[i]) + "\n"
	}

	return content, nil
}

Stacktrace

panic: runtime error: index out of range [3] with length 3 [recovered]
        panic: C:\Program Files\Go\src\unsafe\unsafe.go:192:1: CFG post-order panic: runtime error: index out of range [3] with length 3

goroutine 1 [running]:
github.com/traefik/yaegi/interp.(*Interpreter).cfg.func2.1()
        C:/Users/User/go/pkg/mod/github.com/traefik/yaegi/interp/cfg.go:475 +0x78
panic({0x10b9620, 0xc001408ba0})
        C:/Program Files/Go/src/runtime/panic.go:1038 +0x215
...see comment edit for full stacktrace

zale144 · 2022-03-27T12:13:37Z

@switchupcb So I'm getting an error:

2022/03/27 13:07:50 1:21: import "gitlab.com/user/pkg" error: An import source could not be found: "gitlab.com/user/pkg"
The current GOPATH=/Users/myself/go, GOCACHE=/Users/myself/Library/Caches/go-build, GOTOOLDIR=/usr/local/go/pkg/tool/darwin_arm64
The target filepath "gitlab.com/user/pkg" is not within the path "/usr/local/go"

while setting options as such:

	i := interp.New(interp.Options{
		GoPath:    os.Getenv("GOPATH"),
		GoCache:   os.Getenv("GOCACHE"),
		GoToolDir: build.ToolDir,
		Stdout:    &stdout, Stderr: &stderr,
	})

any ideas?

switchupcb · 2022-03-28T18:22:51Z

@zale144 I'm not sure that os.Getenv("GOCACHE") will fetch the correct cache. Use os.UserCacheDir(). However, the actual issue says that gitlab.com/user/pkg isn't in /usr/local/go. So first check that it's there (or within a subdirectory). If it is there, there is an error. If it's not there, you need to go get it.

If it's already go get then there is some logic error that throws an error in this function. It's called from this line, which means the import couldn't be found using Go's package tool either (which handles import).

zale144 · 2022-03-28T22:19:34Z

@zale144 I'm not sure that os.Getenv("GOCACHE") will fetch the correct cache. Use os.UserCacheDir(). However, the actual issue says that gitlab.com/user/pkg isn't in /usr/local/go. So first check that it's there (or within a subdirectory). If it is there, there is an error. If it's not there, you need to go get it.

If it's already go get then there is some logic error that throws an error in this function. It's called from this line, which means the import couldn't be found using Go's package tool either (which handles import).

Thanks for the quick reply. I tried it and the same problem persists.

switchupcb · 2022-03-28T23:06:57Z

@zale144 > #1265 (comment)
What directory is the package you go get located on your machine?

zale144 · 2022-03-29T11:56:44Z

@zale144 > #1265 (comment)
What directory is the package you go get located on your machine?

Silly me, I haven't added the dependency in the go.mod file. However, in order to keep it there, I need to have it imported, which for me kind of defeats the purpose, so I'll just probably go with something like git submodules and have it available locally.

Though, this error remains (from the current latest):

import "github.com/spf13/cobra" error: /Users/myself/go/pkg/mod/github.com/spf13/[email protected]/args.go:61:9: cannot use  (type funcT) as type funcT in return argument

So at this point I might just get back to the go plugins torture.

Thanks anyway!

switchupcb · 2022-03-29T19:14:02Z

@zale144 You don't need the dependency in your go.mod file, but it has to be located on the machine. Otherwise, there is nothing to interpret.

import "github.com/spf13/cobra" error: /Users/myself/go/pkg/mod/github.com/spf13/[email protected]/args.go:61:9: cannot use (type funcT) as type funcT in return argument

This is an error indicating a problem with the package you're attempting to import. Does the import work without an interpreter? If so, the error is from the interpreter itself.

switchupcb · 2022-04-20T21:41:19Z

It's puwl wequest wednesday.

tonyalaribe · 2022-06-21T20:52:15Z

Hi, How can I help to push this forward? Should I rebase it?

These days pretty much every Go package runs on Gomodules, so it's difficult to use yaegi on any real world project without it.

switchupcb · 2022-06-22T00:46:52Z

@tonyalaribe Sure! I'm not sure why your comment was marked as off-topic, but I have updated the branch. Here is a description of the current state of this pull request.

TLDR: Maintainers need to approve workflows/review.

I will run another Go Module test around the release of Copygen v0.4, but the general idea is to create a file that uses a non-extracted third-party module and determine if that code runs. Note that you must go get the module prior to its non-extracted use; it doesn't need to be in the go.mod. As an example, the errors present in #1265 (comment) occur in the cfg.go file which may indicate an error with the interpretation of imported types, or just an error with the interpreter in general. Using imports with the standard library works as outlined in a disgo generate.go copygen file.

Specifically, I'm confident that the current state of this pull-request fixes the issue with importing; as the original code was platform-specific and unfinished (according to @mpl's comments). If that statement is correct, the errors I'm facing are edge cases within the interpreter, which is NOT necessarily in the scope of this pull request. In other words, this pull request is finished and those errors should be solved in a separate pull request.

The main issue is that this pull request has gone 9 months without a comment from the maintainers; let alone workflow approval. In addition, it has the "needs design review" label which seems to historically be placed on pull requests that are never merged. In this case, the best way to move this forward is to bring this pull request to the attention of the maintainers @mvertes, @mpl. A discussion involving this pull request was addressed by @ldez in #1292 but nothing happened since that time.

ldez · 2022-07-29T19:28:10Z

I was forced to squash your commits.

I will insist on the fact to address changes as commits and use rebase (not squash) instead of merge.

$ git remote -v 
origin  [email protected]:switchupcb/yaegi.git (fetch)
origin  [email protected]:switchupcb/yaegi.git (push)
upstream        [email protected]:traefik/yaegi.git (fetch)
upstream        [email protected]:traefik/yaegi.git (push)

$ git fetch upstream
$ git rebase upstream/master

I created a backup of the previous content here: https://github.com/ldez/yaegi/tree/backup/switchupcb-master

switchupcb · 2022-07-29T19:43:16Z

Great! I can fix that.

ldez · 2022-07-29T19:50:40Z

To update your local branch with my changes:

$ git remote -v 
origin  [email protected]:switchupcb/yaegi.git (fetch)
origin  [email protected]:switchupcb/yaegi.git (push)
upstream        [email protected]:traefik/yaegi.git (fetch)
upstream        [email protected]:traefik/yaegi.git (push)

$ git switch master
$ git fetch origin
$ git reset --hard origin/master

switchupcb · 2022-07-29T19:52:44Z

To be specific, I believe build cache is required, but could not be located: GOCACHE is not defined and neither $XDG_CACHE_HOME nor $HOME are defined is related to the make environment not defining those environment variables. GOCACHE is used in this pull request. I am not sure what $XDG_CACHE_HOME nor $HOME is referencing.

In the other case, go: GOPATH entry is relative; must be absolute path: "./_pkg". is addressed in the original pull request post (#1265 (comment)).

Did not touch; however the new case may implement.

I will ensure an absolute path is used in whichever function is causing that error.

switchupcb · 2022-07-29T19:56:43Z

I can begin the work for those fixes tommorow.

switchupcb · 2022-08-06T23:50:58Z

Started the work now.

Realization

What I have realized is that both the old and new approaches simply address the happy path, but don't fundamentally address the main issue. The old approach by @mpl attempts to do what the packages.Load() function does, with some extra details regarding relative paths. The new method uses packages.Load(), then attempts to fill a gap — caused by a lack of environment variables — with remnants of the old method.

I looked at the packages module to view what is required to load packages.

Basically, these 6 environment variables can determine how a module is loaded, and where it needs to be loaded from.

Issue Revisited

In the current version of yaegi, the user is advised to set the GOPATH to allow usage of modules. This may work in some cases, where the GOPATH is able to resolve to the exact package it's importing. The issue is that this is likely not the case. As an example, using go get github.com\switchupcb\[email protected] will create a directory in go\pkg\mod\github.com\switchupcb\[email protected], while the importPath may be github.com/switchupcb/copygen/cli/models. In this case, there is no github.com/switchupcb/copygen/cli/models, only \github.com\switchupcb\[email protected]\cli\models, so the import fails; regardless of the method being used.

Adding the GOCACHE can help the packages module find the actual package, which is why it is useful to add. This situation is the case for most of the "Required Environment Variables" which is why they aren't always required. In this PR, GOTOOLDIR is set, but not actually used by packages module. Instead, it is used by an approach of finding packages inspired by the "remnants of the old method". In hindsight, I don't recommend requiring GOTOOLDIR if it isn't required.

The following code represents the required variables that are passed into packages.Load(&config, path) from the config.Env []string{}.

vars := map[string]*string{
  "GOCACHE":     &envVars.gocache,
  "GOPATH":      &envVars.gopath,
  "GOROOT":      &envVars.goroot,
  "GOPRIVATE":   &envVars.goprivate,
  "GOMODCACHE":  &envVars.gomodcache,
  "GO111MODULE": &envVars.go111module,
}

The final issue is that the tests (of yaegi) do not cover every detail involved with importing a module. It is no surprise that there were issues with the old approach, and no surprise that the current approach is flawed.

Solution

As a result, a real solution that will work in every case is allowing the user to define these 6 environment variables as required.

Implementation

User can pass environment variables from an Options struct which is passed to the Interpreter in New() at Line 367.
A packages.Config.Env field is instantiated using interp.opt.env["environment variable"]. Note that — in theory — interp.context.GOPATH is the same GOPATH as interp.opt.env["GOPATH"]. In actuality, a user may wish to specify a different GOPATH to import modules from.
Package is loaded using the absolute import path.

TODO

update README Documentation for Go Module Support Section (specifying how to set environment variables from New()).
set Required Environment Variables for import tests (GOCACHE) in Makefile such that tests pass.

switchupcb · 2022-08-06T23:57:55Z

For backwards compatibility purposes, I have left the Interpreter Options struct with its GoPath field that is used throughout the program. However, this field could be removed and replaced with Options.Env and set accordingly in New() to address 2. in #1265 (comment).

NOTE: This is related to Line 170 of src.go

switchupcb · 2022-08-07T00:01:30Z

I am not confident in setting the MAKEFILE according to the repository standard. I ask that @ldez or another maintainer complete that task (context in #1265 (comment) and #1265 (comment)). In any case, I can run another integration test to create a valid README example for the documentation around the release of Copygen 0.5. As a result, I have re-marked this PR as a draft (since it won't pass CI without the correct environment variables).

ldez · 2022-08-07T02:44:21Z

@switchupcb I will once again clean your PR, please read my previous comment to learn how to rebase.

Note: the GOPATH mode must be kept because it's a requirement for the plugin system of Traefik.

ldez · 2022-08-07T02:54:27Z

I was once again forced to squash your commits.
Please learn how to do a clean rebase.

To update your local branch with my changes, you can follow this previous comment.

switchupcb · 2022-08-10T03:58:08Z

@ldez Great, thanks. Steps didn't work the first time for some reason.

The reason that the tests for CI didn't pass is described.

1.

err: exit status 2: stderr: go: GOPATH entry is relative; must be absolute path: "./_pkg".

As an example, closure_test.go TestFunctionCall contains the following code.

i := interp.New(interp.Options{GoPath: "./_pkg"})
if err := i.Use(stdlib.Symbols); err != nil {
   t.Fatal(err)
}

Solution

Obviously, the complaint from the packages.Load call is that the environment variable is relative. This is fixed by converting a relative GOPATH to absolute when applicable.

2

err: exit status 1: stderr: build cache is required, but could not be located: GOCACHE is not defined and neither $XDG_CACHE_HOME nor $HOME are defined

As stated in #1265 (comment), this error occurs because the Github Runner (not Makefile) has an undefined $GOCACHE (or $HOME) environment variable.

Solution

Setup GOCACHE in Github Action. Use it in the test files.

- name: Setup GOCACHE
   run: go env -w GOCACHE=${{ github.workspace }}/<INSERT CACHE DIR HERE>

I don't have an Ubuntu or Linux so I am not sure about the value to expect for the "CACHE DIR". I guessed using a setup go guide; so if it's wrong it will throw a (different) error.

Other

Following these fixes, it becomes obvious why the administrator error occurs. When GOCACHE is set, packages.Load will attempt to create a file that requires privileges.

err: exit status 1: stderr: go: creating work dir: mkdir C:\WINDOWS\go-build3375461458: Access is denied.

Solution

Run in administrator (or in a mode that gives file creation permissions in that directory). Once I did this, the only issue I experienced was not having the actual package I needed to be imported.

Alternatively, I'd experience issues with tests such as TestPackagesError/different_packages_in_the_same_directory which gives the following error.

pkg_test.go:188: got "1:21: import \"github.com/foo/pkg\" error: an import source could not be found for \"github.com/foo/pkg\"", want "1:21: import \"github.com/foo/pkg\" error: found packages pkg and pkgfalse in _pkg9\\src\\github.com\\foo\\pkg"

It wants a specific string for the error but got a different error. In this case, it's obvious that the issue comes from running the test without the fake package. However, I've noted this issue in case a mismatch in error formatting is present in the next CI run.

Disclaimer

Even with the GOCACHE environment variable set, it must be passed to interp.opt.env["GOCACHE"] using the New(Options{...}. Unfortunately, the tests do NOT use the interp.New function which sets the interp.opt.env accordingly. I will not be able to fix that issue (originating from unit test code) in a timely manner. Moreso, I don't want to edit unit tests without full context to what is being tested.

In other words, UNIT TESTS THAT DON'T SETUP THE INTERPRETER CORRECTLY WILL INCORRECTLY FAIL with err: exit status 1: stderr: build cache is required, but could not be located: GOCACHE is not defined and neither $XDG_CACHE_HOME nor $HOME are defined. CI is expected to throw this error in the current state of the PR.

In addition, I do not have the fake packages and don't want to go through the effort of setting them up. cmake (for Windows) has an issue with the go generate call and I'm too lazy to set up WSL2 for this. I'm going to rely on the CI to make edits, but it requires workflow approval every time.

As a result of the above issues, it may be a better idea may be to have an actual maintainer complete the PR from here.

switchupcb · 2023-01-22T22:08:14Z

does your (switchupcb master branch) fork of yaegi support external modules?

To answer the quoted question in full, please consider the following information.

The main master branch of yaegi (unforked) does not allow you to import third party modules: Any attempt to do so will be met with an error. This PR (fork) will allow you to import those third party modules, but does not guarantee anything beyond that (i.e usage of the imported code).

For the switchupcb master branch of yaegi (fork), #1265 (comment) goes into depth on the limitations of third party module imports. In short, you must have valid permissions on the machine (of the interpreter), and must pass valid Go environment variables to the interpreter via the interp.Options struct during the interp.New instantiation. If these conditions are met, you will be able to import third party modules via the interpreter.

Will the import work? Yes.

Will the code you imported work? I'm not sure. I haven't tested it in a thorough manner.

Contribution

If you are interested in contributing to this pull request, what needs to be done?

You must fix the unit tests such that they use interp.New to set up the interpreter. See the Disclaimer of #1265 (comment) for more information about this pull request in relation to yaegi's unit tests. You can also read #1265 (comment) for information on the current implementation of this feature.

This can be done with the following steps.

Rebase the branch.
Fix new conflicts.
Add changes (unit test replace).
Pass CICD workflows.

After these steps, the scope of this pull request is technically completed.

james-lawrence · 2023-01-28T20:27:12Z

dunno if this helps anyone but I've been carrying a patch for yaegi to support go modules go 2ish years which uses golang builtin tooling to resolve imports. (along with another fix for an erroneous error message that claims imports already imported when an import fails to import properly)

I think the long term solution here will be to allow users to provide a resolving function. and have a the current implementation as the default and the go modules one as a swap in replacement.

I'd be willing to do the lifting to get it over the line if there is interest.

switchupcb · 2023-01-31T00:45:33Z

I'd be willing to do the lifting to get it over the line if there is interest
— @james-lawrence

Great, all you have to do is fix up the unit tests as stated in #1265 (comment). The root method (of the old implementation) isn't sufficient (and doesn't work on Windows) for reasons explained in #1265 (comment).

I think the long term solution here will be to allow users to provide a resolving function. and have a the current implementation as the default and the go modules one as a swap in replacement.
— @james-lawrence

The current implementation allows users to provide the environment variables required to resolve imports correctly. This is how yaegi currently has users import modules (by setting the interp.Options). This behavior is also supported in the current implementation of this PR.

mewmew · 2023-03-23T18:48:12Z

I'd be willing to do the lifting to get it over the line if there is interest.

Great work @james-lawrence! There's definitely interest to have Go mod support in Yaegi! What is needed to get your local branch up to speed with the latest version of Yaegi?

Cheers,
Robin

james-lawrence · 2023-03-23T19:05:47Z

@mewmew its up to date. i just havent had the time to sit down and make a PR and ensure I still had the test hanging around. likely won't get to this for awhile given current life plans.

mewmew · 2023-03-24T03:30:18Z

@mewmew its up to date. i just havent had the time to sit down and make a PR and ensure I still had the test hanging around. likely won't get to this for awhile given current life plans.

That's fair. Life comes first.

Wish you a good start of the Spring James!

Cheers,
Robin

switchupcb · 2023-07-05T18:47:15Z

As a reminder, this pull request is waiting on "UNIT TESTS THAT DON'T SETUP THE INTERPRETER CORRECTLY" (by not using interp.New) to be fixed. Read #1265 (comment) for details.

I will review this pull request again prior to the next update of Copygen which is coming soon.

maddalax · 2024-10-27T21:17:42Z

any update on this?

switchupcb changed the title ~~feat: add partial support for Go modules to imports~~ feat: add support for Go modules to imports Sep 27, 2021

mvertes added enhancement New feature or request status/1-needs-design-review labels Oct 5, 2021

switchupcb mentioned this pull request Oct 9, 2021

Interface Conversion Error due to unsafe2.dummy #1283

Closed

switchupcb changed the title ~~feat: add support for Go modules to imports~~ add support for Go modules to imports Oct 10, 2021

switchupcb changed the title ~~add support for Go modules to imports~~ fix support for Go modules to imports Oct 10, 2021

switchupcb mentioned this pull request Jan 18, 2022

Under windows, the interpreter "import" cannot exit #1351

Open

switchupcb marked this pull request as draft March 24, 2022 22:56

switchupcb marked this pull request as ready for review March 25, 2022 00:12

switchupcb mentioned this pull request Mar 25, 2022

Fix 3rd Party Imports Cross-Platform cosmos72/gomacro#121

Open

ldez force-pushed the master branch 2 times, most recently from b154b4e to 4b1426b Compare July 29, 2022 19:36

switchupcb marked this pull request as draft August 6, 2022 23:51

feat: support for Go modules to imports

11165f1

ldez force-pushed the master branch from 2a521ad to 11165f1 Compare August 7, 2022 02:51

switchupcb added 4 commits August 9, 2022 21:58

resolve relative GOPATH

6f296d9

revert absolute import path resolution

2dae3c2

add GOCACHE to go env

657705d

update error message for failed import

7151428

switchupcb mentioned this pull request Dec 3, 2022

add Hot Reload Guide switchupcb/disgo#46

Closed

5 tasks

switchupcb changed the title ~~feat: support for Go modules to imports~~ feat: support for Go modules to imports [needs unit test review] Jul 5, 2023

feat: support for Go modules to imports [needs unit test review] #1265

Are you sure you want to change the base?

feat: support for Go modules to imports [needs unit test review] #1265

Conversation

switchupcb commented Sep 27, 2021 • edited Loading

Timeline

Support Go Modules in Imports

Problem

Fix

Solution

Test

Disclaimer

CLAassistant commented Sep 27, 2021 • edited Loading

switchupcb commented Sep 28, 2021 • edited Loading

Update

StackTrace

switchupcb commented Sep 28, 2021

switchupcb commented Sep 28, 2021

switchupcb commented Oct 5, 2021

switchupcb commented Oct 19, 2021 • edited Loading

Update

Error

switchupcb commented Oct 26, 2021 • edited Loading

bobfang1992 commented Nov 8, 2021

switchupcb commented Jan 18, 2022 • edited Loading

c0b41 commented Feb 12, 2022

switchupcb commented Mar 25, 2022 • edited Loading

Stacktrace

zale144 commented Mar 27, 2022 • edited Loading

switchupcb commented Mar 28, 2022 • edited Loading

zale144 commented Mar 28, 2022

switchupcb commented Mar 28, 2022

zale144 commented Mar 29, 2022

switchupcb commented Mar 29, 2022

switchupcb commented Apr 20, 2022

tonyalaribe commented Jun 21, 2022

switchupcb commented Jun 22, 2022 • edited by ldez Loading

ldez commented Jul 29, 2022 • edited Loading

switchupcb commented Jul 29, 2022

ldez commented Jul 29, 2022

switchupcb commented Jul 29, 2022 • edited Loading

switchupcb commented Jul 29, 2022

switchupcb commented Aug 6, 2022 • edited Loading

Realization

Issue Revisited

Solution

Implementation

TODO

switchupcb commented Aug 6, 2022

switchupcb commented Aug 7, 2022

ldez commented Aug 7, 2022

ldez commented Aug 7, 2022

switchupcb commented Aug 10, 2022 • edited Loading

1.

Solution

2

Solution

Other

Solution

Disclaimer

switchupcb commented Jan 22, 2023 • edited Loading

Contribution

james-lawrence commented Jan 28, 2023 • edited Loading

switchupcb commented Jan 31, 2023

mewmew commented Mar 23, 2023

james-lawrence commented Mar 23, 2023

mewmew commented Mar 24, 2023

switchupcb commented Jul 5, 2023

maddalax commented Oct 27, 2024

switchupcb commented Sep 27, 2021 •

edited

Loading

CLAassistant commented Sep 27, 2021 •

edited

Loading

switchupcb commented Sep 28, 2021 •

edited

Loading

switchupcb commented Oct 19, 2021 •

edited

Loading

switchupcb commented Oct 26, 2021 •

edited

Loading

switchupcb commented Jan 18, 2022 •

edited

Loading

switchupcb commented Mar 25, 2022 •

edited

Loading

zale144 commented Mar 27, 2022 •

edited

Loading

switchupcb commented Mar 28, 2022 •

edited

Loading

switchupcb commented Jun 22, 2022 •

edited by ldez

Loading

ldez commented Jul 29, 2022 •

edited

Loading

switchupcb commented Jul 29, 2022 •

edited

Loading

switchupcb commented Aug 6, 2022 •

edited

Loading

switchupcb commented Aug 10, 2022 •

edited

Loading

switchupcb commented Jan 22, 2023 •

edited

Loading

james-lawrence commented Jan 28, 2023 •

edited

Loading