Skip to content

Conversation

@NEKO-CwC
Copy link

@NEKO-CwC NEKO-CwC commented Mar 20, 2025

Add the "llpkg_config" column to the "modules" table in the database, save the "llpkg.cfg" file content in it

Add LLPkg Details line of meta information to the unit page to display the llpkg info.
image
Add a new label LLPkg library at header
image

Add a new environment variable "LLPKG_METADATA_DIR" for storing the llpkgstore.json through llpkgstore/metadata.

Add a new path "/llpkg", which behaves and displays content similar to the official "/std", and constructs all LLPkgs for display through the hosted llpkgstore.json file.
image

@NEKO-CwC NEKO-CwC marked this pull request as draft March 26, 2025 10:03
@NEKO-CwC NEKO-CwC marked this pull request as ready for review March 28, 2025 09:36
@NEKO-CwC
Copy link
Author

need review
@visualfc @aofei

@aofei
Copy link
Member

aofei commented Mar 28, 2025

Comment on lines 16 to 18
if llpkg.IsOfficialLLPkgModule(modulePath) {
return nil, fmt.Errorf("%s@%s is not a LLPkg module", modulePath, version)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When llpkg.IsOfficialLLPkgModule(modulePath) returns true, logically, this should mean that modulePath is an official LLPkg module. However, the code block returns an error, with the error message indicating that modulePath is not an LLPkg module.This should be considered a logical error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done at da2d8b3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the LLPkg Details currently not shown for unofficial LLPkgModules? @NEKO-CwC @aofei

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently only consider official LLPkgs.

}

// GetLLPkgConfig is not supported at FetchDataSource is will always return an error.
// It should be implemented in the postgres package.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe determine this situation more clearly

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is mentioned at fetchdatasource package's top

// Package fetchdatasource provides an internal.DataSource implementation
// that fetches modules (rather than reading them from a database).
// Search and other tabs are not supported.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mean to comment on the function of FetchDataSource, but to comment on why GetLLPkgConfig is not implemented in FetchDataSource

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there is no definite answer at the moment, the last update comment is TODO at 23655bb

luoliwoshang

This comment was marked as duplicate.

Copy link
Member

@luoliwoshang luoliwoshang Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For yours question about a submodule without license can't show document in pkgsite.

I Used github.com/NEKO-CwC/llpkgstore as a GithubRepo for testing, and tested it with the included cjson. It appears that the module directory where cjson is located (i.e., the cjson directory itself) does not have a license, so its actual documentation information cannot be displayed.

image

same licence lack info in pkg.go.dev https://pkg.go.dev/github.com/NEKO-CwC/llpkgstore/cjson

but in this repo https://pkg.go.dev/github.com/luoliwoshang/goplus-llpkg/libxslt,it have not direct licence in the libxslt path,but it can show document.

image

but in the github.com/luoliwoshang/goplus-llpkg/libxslt , it have the release version, https://pkg.go.dev/github.com/luoliwoshang/goplus-llpkg/libxslt?tab=licenses

@NEKO-CwC may release your test cjson for test


but in github.com/luoliwoshang/goplus-llpkg/cjson it havent direct licence & it havent a release,it also can show its document.
image

❯ curl -H "Accept: application/vnd.github.v3+json" https://api.github.com/repos/luoliwoshang/goplus-llpkg/releases | jq '.[].tag_name'

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 56341    0 56341    0     0  68690      0 --:--:-- --:--:-- --:--:-- 68624
"libxml2/v1.2.1"
"libxslt/v1.0.1"
"libxslt/v1.0.0"
"libxml2/v1.2.0"
"zlib/v1.0.0"
"libxml2/v1.1.0"
"libxml2/v1.0.0"
(base) 
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After researching this issue, I can explain why the "Redistributable license False" error occurred and why Documentation wasn't displaying properly.

Go package behavior with goproxy

  1. When packaging, Go automatically inherits LICENSE from the root directory (if present)
    image
  2. When fetching from goproxy:
    • Latest release is pulled first (even with newer tags available)
    • Without releases, latest tag is used
    • Without tags, latest commit is used to create a pseudo-version

Root cause analysis

The issue occurred because when fetching cjson/v1.1.0 tag, this version didn't include a LICENSE in the root directory yet. The LICENSE was added in commit 36ed437, which came immediately after cjson/v1.1.0. Since the package zip pulled from goproxy didn't contain a LICENSE, pkgsite displayed "Redistributable license False".

Test case explanation

  • github.com/luoliwoshang/goplus-llpkg/libxslt works because it has a release with LICENSE in root
  • github.com/luoliwoshang/goplus-llpkg/cjson works in your test environment because it uses the latest commit's pseudo-version, which includes LICENSE

Resolution

This has been fixed in cjson/v1.4.0, which properly displays Documentation now that LICENSE is included.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM👍

return results, nil
}

// TODO: It is temporarily not implemented.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Note is recommended to use "MARKER(uid): note body" format.

Details

lint 解释

这个lint结果提示你建议使用“MARKER(uid): note body”格式来编写注释。这意味着在代码中添加注释时,应该遵循这种特定的格式,以便于识别和理解。

错误用法

以下是一个错误的注释示例:

// 这是一个错误的注释示例

正确用法

以下是一个正确的注释示例:

// MARKER(12345): 这是一个正确的注释示例

💡 以上内容由 AI 辅助生成,如有疑问欢迎反馈交流

@luoliwoshang
Copy link
Member

This PR needs corresponding tests, e.g., checking if nested directories display under \LLPkg and if an LLPkg shows the expected LLPkgDetail.

@luoliwoshang
Copy link
Member

Current Pkgsite's development uses lots of path replacements, such as substituting the LLPkg GitHub repo with a currently available test environment. It also requires replacing the path from which the LLPkgStore tool requests llpkgstore.json. Describing these changes in the PR comments would make this PR more comprehensive and more convenient for reviewers.

NEKO-CwC added 2 commits April 1, 2025 14:24
feat: logic change in llpkg.cfg verify
@qiniu-x
Copy link

qiniu-x bot commented Apr 2, 2025

[Git-flow] Hi @NEKO-CwC, There are some suggestions for your information:


Rebase suggestions

  • Following commits seems generated via git merge

    Merge remote-tracking branch 'origin/feat/llpkg' into feat/llpkg

    Merge branch 'feat/llpkg_route' of https://github.com/NEKO-CwC/pkgsite into feat/llpkg_route

  • Following commits have duplicated messages

    fix: verify

    fix: verify

    doc: english

    doc: english

    feat: "/llpkg" route grid style

    feat: "/llpkg" route grid style

    feat: "/llpkg" route grid style

Which seems insignificant, recommend to use git rebase command to reorganize your PR.

For other git-flow instructions, recommend refer to these examples.

Details

If you have any questions about this comment, feel free to raise an issue here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants