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

fix bug in windows #5844

Merged
merged 4 commits into from Nov 21, 2022
Merged

fix bug in windows #5844

merged 4 commits into from Nov 21, 2022

Conversation

kvii
Copy link
Contributor

@kvii kvii commented Nov 5, 2022

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

This pr is intent to fix the bug that I made in #5836.

"runtime.Caller" will return path like "C:/Users/name/go/pkg/mod/gorm.io/gorm@v1.2.3/utils/utils.go" in windows, not "C:\foo\bar".

And "TestSourceDir" redeclared in windows. Even I placed it to "utils_unix_test.go" and "utils_windows_test.go".

User Case Description

See https://github.com/kvii/gormt.git

@kvii kvii mentioned this pull request Nov 7, 2022
3 tasks
@jinzhu
Copy link
Member

jinzhu commented Nov 7, 2022

Hi @kvii

Thank you for the fix, but please don't add the build tag for the windows tests, or we can't cover those cases with GitHub actions.

Thank you.

@kvii
Copy link
Contributor Author

kvii commented Nov 7, 2022

I made a mistake that I thought the "_unix" suffix was also supported by go.

So I got some "TestFn redeclared in this block" error in windows.

That's the reason I was use build constraint (build tag).

Related go issue: golang/go#51572

@kvii
Copy link
Contributor Author

kvii commented Nov 10, 2022

@jinzhu I removed build tags in windows tests.

@kvii
Copy link
Contributor Author

kvii commented Nov 21, 2022

@jinzhu 您方便继续跟进一下这个 windows bug 吗?

"//go:build unix" 确实不能删除。因为如果删除它的话,"utils_unix_test.go" 在 windows 系统下也会被编译。这会导致 window 用户的编译错误!虽然有 "// +build unix" 的文件在 windows 系统下确实不会被编译。但这并不意味着 "_unix.go" 也有同样的作用。具体原因可以参考我之前的评论。在这里再举一个 go 标准库的例子。可以看到 go 源码的 "src/path/filepath/example_unix_test.go" 也使用了 "//go:build"。

https://github.com/golang/go/blob/cf93b25366aa418dea3eea49a7b85447631c2a1d/src/path/filepath/example_unix_test.go#L1-L7

有可能您是让我只删除 "//go:build unix" 那一行。因为项目 go.mod 中的要求的 go 版本为 1.16,而 "//go:build unix" 格式的声明是 go 1.17 的新特性。可能因为您怕新格式和旧版本 go 不兼容。但这应该是您多虑了。因为这种新老版本混合的写法是兼容旧版本的。并且这是新版 go fmt 命令的默认行为。可以参考 中文博客build constaints 设计草案

最后很抱歉我使用了中文并且直接 @ 了您。因为我看到已经有 2 个关于这个问题的 issue,说明开发者已经开始陆陆续续地升级到 1.24.1 版本了。我希望这个问题能尽快地被解决。我也为我在 #5836 犯下的错误感到抱歉。

@jinzhu jinzhu merged commit b6836c2 into go-gorm:master Nov 21, 2022
@dreamlu
Copy link

dreamlu commented Sep 14, 2023

this bug again in the latest version:

gorm.io/driver/mysql v1.5.1
gorm.io/driver/postgres v1.5.2
gorm.io/driver/sqlserver v1.5.1
gorm.io/gorm v1.25.4

image

@kvii
Copy link
Contributor Author

kvii commented Sep 14, 2023

@dreamlu Seems like "github.com/casbin/gorm-adapter/v3" it not an official package. Gorm only filters source path contains "gorm.io".

gormSourceDir = "/Users/name/go/pkg/mod/gorm.io/" at runtime.

gorm/utils/utils.go

Lines 36 to 41 in e57e5d8

for i := 2; i < 15; i++ {
_, file, line, ok := runtime.Caller(i)
if ok && (!strings.HasPrefix(file, gormSourceDir) || strings.HasSuffix(file, "_test.go")) {
return file + ":" + strconv.FormatInt(int64(line), 10)
}
}

@dreamlu
Copy link

dreamlu commented Sep 15, 2023

@dreamlu Seems like "github.com/casbin/gorm-adapter/v3" it not an official package. Gorm only filters source path contains "gorm.io".

gormSourceDir = "/Users/name/go/pkg/mod/gorm.io/" at runtime.

gorm/utils/utils.go

Lines 36 to 41 in e57e5d8

for i := 2; i < 15; i++ {
_, file, line, ok := runtime.Caller(i)
if ok && (!strings.HasPrefix(file, gormSourceDir) || strings.HasSuffix(file, "_test.go")) {
return file + ":" + strconv.FormatInt(int64(line), 10)
}
}

in that picture, the right is printed by
github.com/casbin/gorm-adapter/v3
this project use old gorm version and is right
image

in that picture, the not right print is printed by

gorm.io/driver/mysql v1.5.1
gorm.io/driver/postgres v1.5.2
gorm.io/driver/sqlserver v1.5.1
gorm.io/gorm v1.25.4

@dreamlu
Copy link

dreamlu commented Sep 15, 2023

my system:linux
ide: goland 2023.2.1

@kvii
Copy link
Contributor Author

kvii commented Sep 15, 2023

@dreamlu What's your expect behavior?

reproduce repo

image image

@dreamlu
Copy link

dreamlu commented Sep 15, 2023

@kvii
same problem:
#5870

in goland, it should be click to origin code, but can't
github.com/casbin/gorm-adapter/v3 sql log is can be click

gorm.io/driver/mysql v1.5.1
gorm.io/driver/postgres v1.5.2
gorm.io/driver/sqlserver v1.5.1
gorm.io/gorm v1.25.4

sql log is can not be click
image

I think the latest gorm , lost some symbol in linux print

@dreamlu
Copy link

dreamlu commented Sep 15, 2023

Is there anyone can solve the problem?

@kvii
Copy link
Contributor Author

kvii commented Sep 15, 2023

You should create a new issue and provide a minimal reproduction project if you want to solve this "problem".
Although in my personal opinion it just because your path contains Chinese. @dreamlu

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

Successfully merging this pull request may close these issues.

None yet

3 participants