fix: Windows Path Separator Compatibility Issue in getSortedSqlFileNames Function (#5150)#5151
Closed
2018yuli wants to merge 9 commits intoedgexfoundry:mainfrom
Closed
fix: Windows Path Separator Compatibility Issue in getSortedSqlFileNames Function (#5150)#51512018yuli wants to merge 9 commits intoedgexfoundry:mainfrom
2018yuli wants to merge 9 commits intoedgexfoundry:mainfrom
Conversation
Member
|
you need to sign-off the commit to pass the DCO |
cloudxxx8
requested changes
May 21, 2025
internal/pkg/db/postgres/utils.go
Outdated
| // NormalizePath normalizes file paths to be platform-independent. | ||
| func NormalizePath(baseDir, fileName string) string { | ||
| return filepath.ToSlash(filepath.Join(baseDir, fileName)) | ||
| } No newline at end of file |
cloudxxx8
requested changes
May 21, 2025
internal/pkg/db/postgres/utils.go
Outdated
|
|
||
| // NormalizePath normalizes file paths to be platform-independent. | ||
| func NormalizePath(baseDir, fileName string) string { | ||
| return filepath.ToSlash(filepath.Join(baseDir, fileName)) |
Member
There was a problem hiding this comment.
according to the go docs, filepath.Join should use the OS separator.
https://pkg.go.dev/path/filepath#Join
Windows is \ and Linux is /. If you wrap it with filepath.ToSlash, will it become always /?
Could this really solve the issue?
Author
|
Hi,
This issue is related to how Go's embed package works. On Windows, when you use go:embed, the file paths are normalized to use / as the separator, rather than \, which is the native separator on Windows.
For example, given the following directory structure:
/testdata/
file1.txt
If we embed this directory in a Go program, the paths will be represented as /testdata/file1.txt instead of \testdata\file1.txt on Windows. This is due to Go's behavior with embed paths, which always use / as the path separator, regardless of the operating system.
To illustrate this, here's a test case that ensures the paths behave as expected:
// main.go
package main
import (
"embed"
"fmt"
"github.com/stretchr/testify/assert"
"io/fs"
"testing"
)
//go:embed testdata/*
var content embed.FS
func TestEmbedPaths(t *testing.T) {
expectedPaths := []string{
"testdata",
"testdata/file1.txt",
}
unexpectedPaths := []string{
"testdata\\file1.txt",
}
// print all files in the "testdata" directory
err := fs.WalkDir(content, "testdata", func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
// print the path of each file in the "testdata" directory
fmt.Println("Embedded file:", path)
assert.Contains(t, expectedPaths, path, "path %s not found in expected paths", path)
assert.NotContains(t, unexpectedPaths, path, "path %s found in unexpected paths", path)
return nil
})
if err != nil {
t.Fatal(err)
}
}
=== RUN TestEmbedPaths
Embedded file: testdata
Embedded file: testdata/file1.txt
--- PASS: TestEmbedPaths (0.00s)
PASS
Would it be helpful to add a comment in the code to clarify this behavior, or would you prefer adding a test case to ensure that paths are properly validated across different platforms?
I tested on my Windows machine, using filepath.ToSlash to normalize paths to / should be fine.
________________________________
发件人: Cloud Tsai ***@***.***>
发送时间: 2025年5月21日 13:59
收件人: edgexfoundry/edgex-go ***@***.***>
抄送: 2018yuli ***@***.***>; Author ***@***.***>
主题: Re: [edgexfoundry/edgex-go] fix: Windows Path Separator Compatibility Issue in getSortedSqlFileNames Function (#5150) (PR #5151)
@cloudxxx8 requested changes on this pull request.
________________________________
In internal/pkg/db/postgres/utils.go<#5151 (comment)>:
@@ -144,3 +144,8 @@ func WrapDBError(message string, err error) errors.EdgeX {
}
return errors.NewCommonEdgeX(errors.KindDatabaseError, message, err)
}
+
+// NormalizePath normalizes file paths to be platform-independent.
+func NormalizePath(baseDir, fileName string) string {
+ return filepath.ToSlash(filepath.Join(baseDir, fileName))
according to the go docs, filepath.Join should use the OS separator.
https://pkg.go.dev/path/filepath#Join
Windows is \ and Linux is /. If you wrap it with filepath.ToSlash, will it become always /?
Could this really solve the issue?
—
Reply to this email directly, view it on GitHub<#5151 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AK2XQMDMPKJJKGWK5PDB25327QI37AVCNFSM6AAAAAB5SGBYZ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDQNJWGM4TAMRQHE>.
You are receiving this because you authored the thread.
|
Member
|
Yes, adding unit test could be very helpful. Also,
|
fixes edgexfoundry#5145 refactor the code to move the implementation of validateEvent from core-data to internal/pkg/utils for reuse. Signed-off-by: Jude Hung <jude@iotechsys.com> Signed-off-by: l898974766 <l898974766@outlook.com>
…mes Function (edgexfoundry#5150) converts a path to the format required for go embed by using '/' as the separator. Signed-off-by: 2018yuli l898974766@outlook.com Signed-off-by: l898974766 <l898974766@outlook.com>
…dgexfoundry#5155) Bumps [github.com/labstack/echo/v4](https://github.com/labstack/echo) from 4.13.3 to 4.13.4. - [Release notes](https://github.com/labstack/echo/releases) - [Changelog](https://github.com/labstack/echo/blob/master/CHANGELOG.md) - [Commits](labstack/echo@v4.13.3...v4.13.4) --- updated-dependencies: - dependency-name: github.com/labstack/echo/v4 dependency-version: 4.13.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: l898974766 <l898974766@outlook.com>
…oundry#5149) Bumps [github.com/jackc/pgx/v5](https://github.com/jackc/pgx) from 5.7.4 to 5.7.5. - [Changelog](https://github.com/jackc/pgx/blob/master/CHANGELOG.md) - [Commits](jackc/pgx@v5.7.4...v5.7.5) --- updated-dependencies: - dependency-name: github.com/jackc/pgx/v5 dependency-version: 5.7.5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: l898974766 <l898974766@outlook.com>
…edgexfoundry#5148) Bumps [github.com/go-co-op/gocron/v2](https://github.com/go-co-op/gocron) from 2.16.1 to 2.16.2. - [Release notes](https://github.com/go-co-op/gocron/releases) - [Commits](go-co-op/gocron@v2.16.1...v2.16.2) --- updated-dependencies: - dependency-name: github.com/go-co-op/gocron/v2 dependency-version: 2.16.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: l898974766 <l898974766@outlook.com>
Signed-off-by: yichun chou <yichunccc0830@gmail.com> Signed-off-by: l898974766 <l898974766@outlook.com>
Signed-off-by: yichun chou <yichunccc0830@gmail.com> Signed-off-by: l898974766 <l898974766@outlook.com>
Signed-off-by: yichun chou <yichunccc0830@gmail.com> Signed-off-by: l898974766 <l898974766@outlook.com>
…mes Function (edgexfoundry#5150) Signed-off-by: l898974766 <l898974766@outlook.com>
|
5 tasks
Member
Author
|
Since cloudxxx8 has resolved the conflict and made the changes in PR 5170, I believe this PR can be closed. Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Problem:
While developing on Windows with EdgeX v4.0.0, I encountered an issue where PostgreSQL (pg) does not initialize properly. This issue is caused by the handling of file paths in the getSortedSqlFileNames function within pkg/db/postgres/utils.go.
The current implementation:
assumes the Unix-style path separator, which causes issues on Windows systems.
Suggested Fix:
To ensure compatibility with Windows file path separators, I propose normalizing the path using filepath.ToSlash, as shown below:
Rationale:
Although EdgeX is primarily designed for Linux environments, this small fix will improve compatibility for developers working on Windows, making the project more accessible to a wider audience.
Related Issue:
This change is in response to the issue where PostgreSQL initialization fails on Windows due to path handling issues.
Testing Instructions: