You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/06/15 23:41:40 UTC
[GitHub] [camel-k] ammachado opened a new pull request, #3363: Fixes `kamel local run` panic on Windows
ammachado opened a new pull request, #3363:
URL: https://github.com/apache/camel-k/pull/3363
<!-- Description -->
Fix panic while running `kamel local run` on Windows.
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
NONE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi merged pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi merged PR #3363:
URL: https://github.com/apache/camel-k/pull/3363
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3363:
URL: https://github.com/apache/camel-k/pull/3363#discussion_r898722496
##########
pkg/resources/resources_support.go:
##########
@@ -146,3 +148,11 @@ func Resources(dirName string) ([]string, error) {
return res, dir.Close()
}
+
+func fixPath(path string) string {
Review Comment:
And since `assets.Open(filepath.ToSlash(path))` needs to be invoked every time as a set, why don't we insert the path fixing code inside the `Open` method directly here instead?
https://github.com/apache/camel-k/blob/main/pkg/resources/resources.go#L717
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3363:
URL: https://github.com/apache/camel-k/pull/3363#discussion_r898722496
##########
pkg/resources/resources_support.go:
##########
@@ -146,3 +148,11 @@ func Resources(dirName string) ([]string, error) {
return res, dir.Close()
}
+
+func fixPath(path string) string {
Review Comment:
~And since `assets.Open(filepath.ToSlash(path))` needs to be invoked every time as a set, why don't we insert the path fixing code inside the `Open` method directly here instead?~
https://github.com/apache/camel-k/blob/main/pkg/resources/resources.go#L717
OK, `resources.go` is a generated file by vfsgen and it's not good idea to manually edit it. So at best we can only extract a function for the combination of `assets.Open(filepath.ToSlash(path))` like `func openAssets(path)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3363:
URL: https://github.com/apache/camel-k/pull/3363#discussion_r898714856
##########
pkg/resources/resources_support.go:
##########
@@ -146,3 +148,11 @@ func Resources(dirName string) ([]string, error) {
return res, dir.Close()
}
+
+func fixPath(path string) string {
Review Comment:
Instead of creatng a custom function, it's better to use the builtin function `filepath.FromSlash()`
https://pkg.go.dev/path/filepath#FromSlash
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3363:
URL: https://github.com/apache/camel-k/pull/3363#discussion_r898714856
##########
pkg/resources/resources_support.go:
##########
@@ -146,3 +148,11 @@ func Resources(dirName string) ([]string, error) {
return res, dir.Close()
}
+
+func fixPath(path string) string {
Review Comment:
Instead of creating a custom function here, it's better to use the builtin function `filepath.ToSlash()`
https://pkg.go.dev/path/filepath#ToSlash
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3363:
URL: https://github.com/apache/camel-k/pull/3363#discussion_r899071640
##########
pkg/resources/resources_support.go:
##########
@@ -104,7 +104,7 @@ func WithPrefix(pathPrefix string) ([]string, error) {
var res []string
for i := range paths {
- path := fixPath(paths[i])
+ path := filepath.FromSlash(paths[i])
Review Comment:
Thanks, looks great. But shouldn't this be `filepath.ToSlash()`?
##########
pkg/resources/resources_support.go:
##########
@@ -149,10 +149,6 @@ func Resources(dirName string) ([]string, error) {
return res, dir.Close()
}
-func fixPath(path string) string {
- if runtime.GOOS == "windows" {
- path = strings.ReplaceAll(path, "\\", "/")
- }
-
- return path
+func openAsset(path string) (http.File, error) {
+ return assets.Open(filepath.FromSlash(path))
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on a diff in pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi commented on code in PR #3363:
URL: https://github.com/apache/camel-k/pull/3363#discussion_r898714856
##########
pkg/resources/resources_support.go:
##########
@@ -146,3 +148,11 @@ func Resources(dirName string) ([]string, error) {
return res, dir.Close()
}
+
+func fixPath(path string) string {
Review Comment:
Instead of creating a custom function here, it's better to use the builtin function `filepath.FromSlash()`
https://pkg.go.dev/path/filepath#FromSlash
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] tadayosi commented on pull request #3363: Fixes `kamel local run` panic on Windows
Posted by GitBox <gi...@apache.org>.
tadayosi commented on PR #3363:
URL: https://github.com/apache/camel-k/pull/3363#issuecomment-1157237861
Also please submit this to `main` and optionally `1.8.x` as `1.8.x` is the branch you might want to have this fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org