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.

Sign up for GitHub

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

Jump to bottom

feat(gnomod): forbid require and find dependencies without it #3123

Open

n0izn0iz wants to merge 39 commits into gnolang:master from n0izn0iz:gnomod-remove-require

Contributor

n0izn0iz commented Nov 14, 2024 •

edited

Loading

A step towards the importer package (#2932) and future of gno.mod (#2904)

BREAKING CHANGE: remove require statement support from gno.mod
use .gno files import statements to find dependencies
extract package download routines in gnovm/pkg/gnopkgfetch
extract imports gathering utils in gnovm/pkg/gnoimports

I decided to do this first to avoid having multiple ways to resolve dependencies lying around in the codebase and causing confusion in subsequent steps

Contributors' checklist...

Added new tests, or not needed, or not feasible
Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
Updated the official documentation or not needed
No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
Added references to related issues and PRs
Provided any useful hints for running manual tests

n0izn0iz added 30 commits

November 7, 2024 01:05


          chore: explicitely remove require support

6c702b9

Signed-off-by: Norman Meier <[email protected]>


          chore: finish remove require (+ tmp move utils to load package but sh…

946f9e3

…ould be reverted)

Signed-off-by: Norman Meier <[email protected]>


          feat: mod download without require

773c548

Signed-off-by: Norman Meier <[email protected]>


          feat: module download without require

ccf8bb8

Signed-off-by: Norman Meier <[email protected]>


          chore: refactor

14be54f

Signed-off-by: Norman Meier <[email protected]>


          chore: remove unused writePackage func

78ecedb

Signed-off-by: Norman Meier <[email protected]>


          chore: use PackageDir helper

70cfde5

Signed-off-by: Norman Meier <[email protected]>


          chore: merge remote-tracking branch 'origin/master' into gnomod-remov…

4e01fd3

…e-require


          fix: doc imports without require

13a1e21

Signed-off-by: Norman Meier <[email protected]>


          chore: fix tests

77d696d

Signed-off-by: Norman Meier <[email protected]>


          fix: txtar loader

43afcc9

Signed-off-by: Norman Meier <[email protected]>


          chore: forbid require

89ca164

Signed-off-by: Norman Meier <[email protected]>


          chore: tidy

409bb3c

Signed-off-by: Norman Meier <[email protected]>


          chore: fmt

a411efa

Signed-off-by: Norman Meier <[email protected]>


          chore: remove debug artifact

4bc9998

Signed-off-by: Norman Meier <[email protected]>


          chore: cleaner examples client'

3e2450e

Signed-off-by: Norman Meier <[email protected]>


          chore: go mod tidy

2ced616

Signed-off-by: Norman Meier <[email protected]>


          chore: refacto

Signed-off-by: Norman Meier <[email protected]>


          fix: return errors

eff07fb

Signed-off-by: Norman Meier <[email protected]>


          chore: move / rename

9bf675a

Signed-off-by: Norman Meier <[email protected]>


          chore: move back utils

bee03a0

Signed-off-by: Norman Meier <[email protected]>


          chore: rename funcs and write doc

c749846

Signed-off-by: Norman Meier <[email protected]>


          chore: refacto

8f32074

Signed-off-by: Norman Meier <[email protected]>


          chore: atomicize

c353b21

Signed-off-by: Norman Meier <[email protected]>


          fix: bad refactor

283b308

Signed-off-by: Norman Meier <[email protected]>


          chore: remove unused funcs

d4f6cad

Signed-off-by: Norman Meier <[email protected]>


          chore: dedup code

e1331fd

Signed-off-by: Norman Meier <[email protected]>


          chore: port test

965095b

Signed-off-by: Norman Meier <[email protected]>


          chore: add fetch test with replace

07cccc6

Signed-off-by: Norman Meier <[email protected]>


          chore: more test cases

e65600f

Signed-off-by: Norman Meier <[email protected]>

n0izn0iz added 2 commits

November 14, 2024 08:25


          chore: unexpose testing client

7b385d5

Signed-off-by: Norman Meier <[email protected]>


          chore: merge remote-tracking branch 'origin/master' into gnomod-remov…

9ebbf69

…e-require

github-actions bot added 🧾 package/realm 📦 🤖 gnovm 📦 ⛰️ gno.land labels

github-actions bot assigned n0izn0iz

github-advanced-security bot found potential problems

View reviewed changes

gnovm/pkg/gnopkgfetch/gnopkgfetch_testing.go


		res := ctypes.ResultABCIQuery{}

		finfo, err := os.Stat(target)

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of an integer with architecture-dependent bit size from

strconv.Atoi

to a lower bit size type uint16 without an upper bound check.
Incorrect conversion of an integer with architecture-dependent bit size from

strconv.Atoi

to a lower bit size type uint16 without an upper bound check.

Contributor Author

n0izn0iz Nov 14, 2024

wtf github

gnovm/pkg/gnopkgfetch/gnopkgfetch_testing.go

+              	}
+              	if finfo.IsDir() {
+              		entries, err := os.ReadDir(target)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4).

gnovm/pkg/gnopkgfetch/gnopkgfetch_testing.go

+              		}
+              		res.Response.Data = []byte(strings.Join(files, "\n"))
+              	} else {
+              		content, err := os.ReadFile(target)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4).

github-advanced-security bot found potential problems

View reviewed changes

gnovm/pkg/gnopkgfetch/gnopkgfetch_testing.go


		res := ctypes.ResultABCIQuery{}

		finfo, err := os.Stat(target)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a

user-provided value

.
This path depends on a

user-provided value

.
This path depends on a

user-provided value

.
This path depends on a

user-provided value

.

gnovm/pkg/gnopkgfetch/gnopkgfetch_testing.go

+              	}
+              	if finfo.IsDir() {
+              		entries, err := os.ReadDir(target)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a

user-provided value

.
This path depends on a

user-provided value

.
This path depends on a

user-provided value

.
This path depends on a

user-provided value

.

gnovm/pkg/gnopkgfetch/gnopkgfetch_testing.go

+              		}
+              		res.Response.Data = []byte(strings.Join(files, "\n"))
+              	} else {
+              		content, err := os.ReadFile(target)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a

user-provided value

.
This path depends on a

user-provided value

.
This path depends on a

user-provided value

.
This path depends on a

user-provided value

.


          fix: remove stdlibs from test imports

736d7f6

Signed-off-by: Norman Meier <[email protected]>

n0izn0iz changed the title ~~feat(gnomod): forbid require and find imports without it~~ feat(gnomod): forbid require and find dependencies without it

moul reviewed

View reviewed changes

gnovm/pkg/gnolang/helpers.go

		@@ -35,6 +36,11 @@ func IsStdlib(s string) bool {
		return !strings.HasPrefix(s, "gno.land/")

Member

moul Nov 14, 2024 •

edited

Loading

For your information, this will become dynamic in #2911, which will be finished soon.

I recommend waiting for #2911 to be completed and merged before making any further hardcoded changes to the domain.

Edit: I believe it's fine; the potential conflict should be easy to resolve from any direction. There's no need to wait for #2911.


          Merge branch 'master' into gnomod-remove-require

519c51b

n0izn0iz marked this pull request as ready for review

November 14, 2024 10:13

Contributor Author

n0izn0iz commented Nov 14, 2024 •

edited

Loading

~~@thehowl actually this is fully ready for review, the tests are failing due to codecov only it seems~~

Kouteki requested a review from thehowl

November 14, 2024 13:34


          chore: bump for CI

d10eb8a

Signed-off-by: Norman Meier <[email protected]>

codecov bot commented Nov 14, 2024 •

edited

Loading

Codecov Report

Attention: Patch coverage is 62.06897% with 99 lines in your changes missing coverage. Please review.

Project coverage is 63.71%. Comparing base (a1a7cb3) to head (485d63e).

Files with missing lines	Patch %	Lines
gnovm/pkg/gnopkgfetch/gnopkgfetch.go	51.21%	31 Missing and 9 partials ⚠️
gnovm/pkg/gnopkgfetch/gnopkgfetch_testing.go	65.62%	16 Missing and 6 partials ⚠️
gnovm/pkg/gnoimports/imports.go	65.90%	10 Missing and 5 partials ⚠️
gnovm/pkg/gnomod/file.go	14.28%	6 Missing ⚠️
gnovm/pkg/doc/dirs.go	81.48%	4 Missing and 1 partial ⚠️
gnovm/pkg/gnomod/pkg.go	73.68%	3 Missing and 2 partials ⚠️
gno.land/pkg/integration/testing_integration.go	70.00%	2 Missing and 1 partial ⚠️
gnovm/pkg/gnolang/helpers.go	0.00%	2 Missing ⚠️
gnovm/pkg/gnomod/gnomod.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3123      +/-   ##
==========================================
- Coverage   63.79%   63.71%   -0.08%     
==========================================
  Files         549      552       +3     
  Lines       78819    78734      -85     
==========================================
- Hits        50281    50165     -116     
- Misses      25146    25179      +33     
+ Partials     3392     3390       -2

Flag	Coverage Δ
contribs/gnodev	`60.54% <ø> (-0.63%)`	⬇️
contribs/gnofaucet	`14.82% <ø> (ø)`
gno.land	`73.67% <70.00%> (-0.03%)`	⬇️
gnovm	`67.67% <61.75%> (-0.26%)`	⬇️
misc/genstd	`79.72% <ø> (ø)`
tm2	`62.49% <ø> (+0.15%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

n0izn0iz marked this pull request as draft

November 14, 2024 18:47

Contributor Author

n0izn0iz commented Nov 14, 2024

seems some tests are failing after merging master, my bad, will fix asap

n0izn0iz and others added 2 commits

November 15, 2024 05:33


          Merge branch 'master' into gnomod-remove-require

b7ca880


          fix: remove self from pkg imports

144c0cc

Signed-off-by: Norman Meier <[email protected]>

n0izn0iz marked this pull request as ready for review

November 15, 2024 07:10

Contributor Author

n0izn0iz commented Nov 15, 2024

fixed

Kouteki added the in focus label

n0izn0iz and others added 2 commits

November 16, 2024 18:33


          Merge branch 'master' into gnomod-remove-require

03cee2b


          fix: remove require

485d63e

Signed-off-by: Norman Meier <[email protected]>

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

Labels

in focus 📦 ⛰️ gno.land 📦 🤖 gnovm 🧾 package/realm