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 FileWithLineNum compatible with various platforms #4456

Closed
wants to merge 1 commit into from
Closed

fix FileWithLineNum compatible with various platforms #4456

wants to merge 1 commit into from

Conversation

daheige
Copy link
Contributor

@daheige daheige commented Jun 11, 2021

fix FileWithLineNum func.
For runtime.Caller, the path separator of the file directory is always /, which is a problem left over from the history of golang. For the first parameter of strings.HasPrefix that obtains the file line number, the filepath.Dir mode needs to be used to achieve cross-platform correctness, so I am corrected here.
golang/go#3335
Thanks @Kisesy for helping me find this problem, thanks again.
@jinzhu Please agree with this, thank you very much!

@daheige daheige mentioned this pull request Jun 11, 2021
@jinzhu
Copy link
Member

jinzhu commented Jun 11, 2021

Change back to the regexp match?

@daheige
Copy link
Contributor Author

daheige commented Jun 11, 2021 via email

@daheige
Copy link
Contributor Author

daheige commented Jun 11, 2021

Change back to the regexp match?

FileWithLineNum func here

if ok && (!strings.HasPrefix(file, gormSourceDir) || strings.HasSuffix(file, "_test.go")) {

// need to be changed to the following way 
// compatible with various platforms
		if ok && (!strings.HasPrefix(filepath.Dir(file), gormSourceDir) || strings.HasSuffix(file, "_test.go")) {
			return file + ":" + strconv.FormatInt(int64(line), 10)
		}

@Kisesy
Copy link

Kisesy commented Jun 11, 2021

整个函数完全不需要 filepath ,因为根本就不需要跨平台,strings.HasPrefix() 就是在对比两次 Caller() 的内容嘛,来回转它干嘛

要做的事就是删掉后缀就完了,要嘛直接用 gormSourceDir = path.Dir(path.Dir(file))
再极端一点就是 gormSourceDir = strings.TrimSuffix(file, "/utils/utils.go")
或者怕以后 go 改动 caller() 的代码,不兼容,就用以前的正则删掉后缀就完了。

下面 strings.HasPrefix(file, gormSourceDir) 就保持这样,这里如果来回转,就浪费时间

@Kisesy
Copy link

Kisesy commented Jun 11, 2021

这也是个老问题了,我在以前就说过这里
9934207#comments

@daheige
Copy link
Contributor Author

daheige commented Jun 11, 2021 via email

@daheige
Copy link
Contributor Author

daheige commented Jun 11, 2021

这也是个老问题了,我在以前就说过这里
9934207#comments

The file directory acquisition for various platforms is indeed different, such as windows, unix, and linux are all different, so here I stick to my processing mode. I looked at the version after go1.3 and did deal with the delimiter here
// Copyright 2011 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris

package os

const (
PathSeparator ='/' // OS-specific path separator
PathListSeparator =':' // OS-specific path list separator
) 

@jinzhu

@daheige
Copy link
Contributor Author

daheige commented Jun 11, 2021

@Kisesy This is the official design of golang. There are some problems, but as a rigorous development, you need to consider various cross-platform path separator scenarios. If you really don’t want to deal with it, you can use the path.Dir mode you said.
If you really want to deal with utils/utils.go, it is recommended that one of the two modes is replacement and the other is filepath.Dir mode instead of path.Dir mode. I tested it here, and there is no problem with filepath.Dir.

@daheige
Copy link
Contributor Author

daheige commented Jun 11, 2021

@Kisesy You said that path.Dir is incompatible, but due to problems left over from the history of golang, runtime.Caller always returns the path of the / mode. This problem can be corrected by the golang official instead of using the path.Dir mode here. At present, the filepath.Dir mode is indeed cross-platform. Even if the problem is corrected by golang, there is no need to change it here. 原始邮件 发件人: @.> 收件人: @.> 抄送: @.>; @.> 发送时间: 2021年6月11日(周五) 22:34 主题: Re: [go-gorm/gorm] fix FileWithLineNum compatible with variousplatforms (#4456) 整个函数完全不需要 filepath ,因为根本就不需要跨平台,strings.HasPrefix() 就是在对比两次 Caller() 的内容嘛,来回转它干嘛 要做的事就是删掉后缀就完了,要嘛直接用 gormSourceDir = path.Dir(path.Dir(file)) , 再极端一点就是 gormSourceDir = strings.TrimSuffix(file, "/utils/utils.go"), 或者怕以后 go 改动 caller() 的代码,不兼容,就用以前的正则删掉后缀就完了。 下面 strings.HasPrefix(file, gormSourceDir) 就保持这样,这里如果来回转,就浪费时间 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

整个函数完全不需要 filepath ,因为根本就不需要跨平台,strings.HasPrefix() 就是在对比两次 Caller() 的内容嘛,来回转它干嘛

要做的事就是删掉后缀就完了,要嘛直接用 gormSourceDir = path.Dir(path.Dir(file))
再极端一点就是 gormSourceDir = strings.TrimSuffix(file, "/utils/utils.go")
或者怕以后 go 改动 caller() 的代码,不兼容,就用以前的正则删掉后缀就完了。

下面 strings.HasPrefix(file, gormSourceDir) 就保持这样,这里如果来回转,就浪费时间

Can I use English? If there is any ambiguity here, you can let other gorm contributors see the processing logic here.

@jinzhu jinzhu closed this in 3226937 Jun 13, 2021
@daheige daheige deleted the fix-gorm-dir branch July 14, 2021 13:42
mittwillson pushed a commit to itering/gorm that referenced this pull request Sep 27, 2021
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.

3 participants