feat: add `llgo get` #1038

seri037 · 2025-03-24T10:03:34Z

add mod package to sync and query llpkgstore.json
complete logic of llgo get

luoliwoshang · 2025-03-25T01:32:22Z

compiler/internal/mod/metadata.go

+	return nil
+}
+
+func (m *metadataMgr) buildVersionsHash() error {


The function name buildVersionsHash might be misleading. What it actually builds is just a mapping. Using "Hash" to describe this mapping functionality could be misinterpreted as performing hash encryption on this JSON.

done at 720cc2e

luoliwoshang · 2025-03-25T02:04:44Z

compiler/internal/mod/mod.go

+	} else if major == "v0" || major == "v1" {
+		return fmt.Sprintf("%s/%s", LLPkgPathPrefix, name), nil
+	} else {
+		return fmt.Sprintf("%s/%s/v%s", LLPkgPathPrefix, name, major), nil


duplicates the "v" prefix. Since semver.Major already returns values with "v" (e.g., "v2"), this creates paths with double "v" (like "/vv2").

donne at fix: duplicated 'v' prefix of semver

luoliwoshang · 2025-03-25T09:56:32Z

compiler/internal/get/get.go

+	isLLPkg, err := mod.IsLLPkg(actualModule)
+	if err != nil {
+		return err
+	} else if !isLLPkg {
+		// no more work
+		return nil
+	}
+
+	// 3. Now we need to parse the llpkg.cfg file.
+	cfgPath, err := mod.LLPkgCfgFilePath(actualModule)


Suggest optimizing the usage of mod.IsLLPkg in Do method. Currently it calls mod.IsLLPkg to check if it's an LLPkg, then calls LLPkgCfgFilePath to get the path. Since mod.IsLLPkg internally calculates the config file path as well, suggest modifying mod.IsLLPkg to:

func IsLLPkg(module string) (isLLPkg bool, cfgPath string, err error)

This would avoid calculating the config file path twice in the Do method.

Using multiple return values can lead to misunderstandings. Instead, a direct approach is preferable since the performance difference can be insignificant.

feat: add version mappings querying func.

seri037 · 2025-03-27T09:52:19Z

need review @visualfc @aofei

xushiwei · 2025-03-27T15:51:33Z

compiler/go.mod

@@ -6,16 +6,15 @@ require (
 	github.com/goplus/gogen v1.16.9
 	github.com/goplus/llgo v0.9.9
 	github.com/goplus/llgo/runtime v0.0.0-00010101000000-000000000000
+	github.com/goplus/llpkgstore v0.0.0-20250318065259-1f6d7d1376ae


don't depend this repo.

We integrate some llpkg-related functionalities (e.g. version query, binary download) into goplus/llpkgstore because they're going to be used by other components, that is pkgsite, GitHub Actions. It can also reduce the redundancy in code.

done at c066031

luoliwoshang · 2025-03-28T09:13:35Z

xtool/env/env.go

@@ -61,6 +61,7 @@ func expandEnvWithCmd(s string) (string, bool) {

 		var out []byte
 		var err error
+


unrelated line update

done at 5729a26

luoliwoshang · 2025-03-28T09:41:25Z

compiler/internal/mod/cache.go

+type cache[T any] struct {
+	data T // stores cached data


Given that the cache currently has only one user, perhaps we could simplify the implementation by temporarily removing the generic design?

sure cache has only one user, but using generic design keeps extensibility. I don't think it a proper way to integrate type of metadata directly into cache

visualfc · 2025-03-31T01:48:49Z

compiler/cmd/internal/get/get.go

 )

 // llgo get
 var Cmd = &base.Command{
-	UsageLine: "llgo get [-t -u -v] [build flags] [packages]",
+	UsageLine: "llgo get [clibs/packages]",


llgo get 原有的 flags 可以保留支持，传递到 go get 中。

done at 0041c77

visualfc · 2025-03-31T02:16:58Z

compiler/internal/env/env.go

+	return filepath.Join(userCacheDir, "llgo")
+}
+
+func LLPkgCache() string {


没有使用到的函数在这个 PR 本身不需要定义

done at b56a73a

aofei · 2025-04-01T00:43:29Z

compiler/internal/get/get.go

Consider moving from compiler/internal/get to compiler/internal/modget for better semantics.

done at 3b07d0c

aofei · 2025-04-01T00:53:40Z

compiler/cmd/internal/get/get.go

+
+	for _, m := range modules {
+		// Extract the name/path and version from the first argument
+		name, version := parse(m)


Suggested change

name, version := parse(m)

name, version, _ := strings.Cut(m, "@")

And remove parse as it is no longer needed.

done at 326f1e8

aofei · 2025-04-01T00:56:47Z

compiler/internal/env/env.go

+	// If not set, LLGOCACHE defaults to {UserCacheDir}/llgo/
+	userCacheDir, err := os.UserCacheDir()
+	if err != nil {
+		panic("Can not get user cache dir: " + err.Error())


Suggested change

panic("Can not get user cache dir: " + err.Error())

panic(fmt.Errorf("Can not get user cache dir: %w", err))

To preserve the original error in case an upper layer wants to recover from our panic, we use %w for wrapping instead of string concatenation. (See https://pkg.go.dev/fmt#Errorf)

done at 801690b

aofei · 2025-04-01T01:00:40Z

compiler/internal/get/get.go

+
+	cmd := exec.Command("go", args...)
+	cmd.Stderr = os.Stderr
+	_, err := cmd.Output()


Is hiding stdout here intentional? If not, consider using cmd.Run() instead, and setting cmd.Stdout = os.Stdout above.

done at 5449ef3

aofei · 2025-04-01T01:03:46Z

compiler/internal/mod/metadata.go

+)
+
+var (
+	remoteMetadataURL      = "https://llpkg.goplus.org/llpkgstore.json" // change only for testing


Replace this with https://goplus.github.io/llpkg/llpkgstore.json.

done at b5004f6

xushiwei · 2025-04-01T06:01:01Z

Don't need a cache package. Just use a cache missing mechanism.

xushiwei · 2025-04-01T06:04:55Z

compiler/cmd/internal/get/get.go

+
+	flags := []string{}
+	flagEndIndex := -1
+	for idx, arg := range args {


use flag.Parse to parse command line arguments.

flag.Parse has been called in main(). we process the args it parsed here

I think the args passed in here are already all non-flag arguments, since they come from flag.Args():

llgo/compiler/cmd/llgo/llgo.go

Line 59 in 3b9b716

args := flag.Args()

seri037 · 2025-04-01T08:50:40Z

Don't need a cache package. Just use a cache missing mechanism.

does it means that we should modify the function metadata.update(), and integrate all func. in cache.go to the metadata.go? Is it ok to mix file loading, file updating and file parsing functions all in one go file?

seri037 marked this pull request as draft March 24, 2025 10:03

luoliwoshang reviewed Mar 25, 2025

View reviewed changes

seri037 and others added 15 commits March 27, 2025 16:17

feat: add env variable LLGOCACHE

aa88042

feature:llgo get

1a365e5

chore: revert go.mod changes

0b84309

feat: move metadata query func. from llpkgstore

df33ded

feat: add version mappings querying func.

feat: complete llgo get

3c13d9d

fix(mod): empty cacheDir of MetadataMgr

1f7e1d7

fix(get): repeated annotation

767e391

feat!: follow changes of metadata struct

ddc91d7

fix(annotate): erase old llpkg info, adjust annotation format

751bc85

feat: change LLGOCACHE/pkg-config to LLGOCACHE/llpkg

42f05d8

refactor: rename buildVersionsHash to buildFlatVersionMaps

0f88d08

fix: duplicated 'v' prefix of semver

280f7a0

feat!: move metadata query func. back to llpkgstore

2ecac86

fix: replace omitted LLGOCACHE/pkg-config, add test

81375c7

feat: change llgo build to load .pc file when building llpkg

80f1ef7

seri037 force-pushed the work/cmdget branch from 67c4e6e to 80f1ef7 Compare March 27, 2025 08:18

seri037 marked this pull request as ready for review March 27, 2025 08:19

xushiwei reviewed Mar 27, 2025

View reviewed changes

feat!: remove llgo build changes, simplify llgo get logic

28a9683

NEKO-CwC mentioned this pull request Mar 28, 2025

feat: llgo list command with -m -versions -json flags #1041

Open

seri037 added 2 commits March 28, 2025 16:47

feat: replace dependency llpkgstore with internal impl.

c066031

fix: parse latest

de31232

luoliwoshang reviewed Mar 28, 2025

View reviewed changes

chore: remove unrelated line update

5729a26

luoliwoshang reviewed Mar 28, 2025

View reviewed changes

visualfc reviewed Mar 31, 2025

View reviewed changes

seri037 added 2 commits March 31, 2025 11:45

feat: support flags, and get multiple pkgs once

0041c77

refactor: remove unused func.

b56a73a

aofei reviewed Apr 1, 2025

View reviewed changes

xushiwei reviewed Apr 1, 2025

View reviewed changes

seri037 added 5 commits April 1, 2025 14:31

chore: rename compiler/internal/get to .../modget

3b07d0c

refactor: use strings.Cut instead of custom func.

326f1e8

refactor: use %w for wrapping instead of string concatenation

801690b

refactor: output stdout directly

5449ef3

feat: update remoteMetadataURL

b5004f6

seri037 added 2 commits April 2, 2025 17:52

fix: incorrect behaviour when using latest

bdfe312

fix: timezone error

a401aa4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `llgo get` #1038

feat: add `llgo get` #1038

seri037 commented Mar 24, 2025 •

edited

Loading

luoliwoshang Mar 25, 2025

seri037 Mar 26, 2025

luoliwoshang Mar 25, 2025

seri037 Mar 26, 2025

luoliwoshang Mar 25, 2025

seri037 Mar 27, 2025

seri037 commented Mar 27, 2025

xushiwei Mar 27, 2025

seri037 Mar 28, 2025 •

edited

Loading

seri037 Mar 28, 2025

luoliwoshang Mar 28, 2025

seri037 Mar 28, 2025

luoliwoshang Mar 28, 2025

seri037 Mar 28, 2025

visualfc Mar 31, 2025

seri037 Mar 31, 2025

visualfc Mar 31, 2025

seri037 Mar 31, 2025

aofei Apr 1, 2025

seri037 Apr 1, 2025

aofei Apr 1, 2025

seri037 Apr 1, 2025

aofei Apr 1, 2025

seri037 Apr 1, 2025

aofei Apr 1, 2025

seri037 Apr 1, 2025

aofei Apr 1, 2025

seri037 Apr 1, 2025

xushiwei commented Apr 1, 2025

xushiwei Apr 1, 2025

seri037 Apr 1, 2025

aofei Apr 1, 2025

seri037 commented Apr 1, 2025

		@@ -61,6 +61,7 @@ func expandEnvWithCmd(s string) (string, bool) {

		var out []byte
		var err error

	name, version := parse(m)
	name, version, _ := strings.Cut(m, "@")

	panic("Can not get user cache dir: " + err.Error())
	panic(fmt.Errorf("Can not get user cache dir: %w", err))

feat: add llgo get #1038

Are you sure you want to change the base?

feat: add llgo get #1038

Conversation

seri037 commented Mar 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seri037 commented Mar 27, 2025

Choose a reason for hiding this comment

seri037 Mar 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xushiwei commented Apr 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seri037 commented Apr 1, 2025

feat: add `llgo get` #1038

feat: add `llgo get` #1038

seri037 commented Mar 24, 2025 •

edited

Loading

seri037 Mar 28, 2025 •

edited

Loading