Skip to content

Canonicalizes paths when attempting to compensate for odd mvnw working-directory echo bug #1155

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

Closed

Conversation

briandealwis
Copy link
Member

@briandealwis briandealwis commented Oct 13, 2018

There is an odd bug in some mvnw wrappers (particularly the one generated by the Spring Initializr) where it prints out the current directory name. This bug affects the Scaffold-Jib integration as Skaffold requests mvnw jib:_skaffold-files. For example:

$ pwd
/Users/bsd/Projects/skaffold-jib/spring-boot-logging

$ ./mvnw -q jib:_skaffold-files
/Users/bsd/Projects/skaffold-jib/spring-boot-logging
/Users/bsd/Manumitting/Projects/skaffold-jib/spring-boot-logging/pom.xml
/Users/bsd/Manumitting/Projects/skaffold-jib/spring-boot-logging/src/main/java
/Users/bsd/Manumitting/Projects/skaffold-jib/spring-boot-logging/src/main/resources
/Users/bsd/Manumitting/Projects/skaffold-jib/spring-boot-logging/src/main/resources
/Users/bsd/Manumitting/Projects/skaffold-jib/spring-boot-logging/src/main/jib

A consequence is that Skaffold then monitors the entire source directory and picks up files from the build, and continuously triggers new builds.

To compensate, the Scaffold-Jib dependencies parser compares each path to the workspace directory. But there are two problems:

  1. The workspace directory is the context field value and is usually relative to the current working directory, whereas the files reported by jib:_skaffold-files are absolute.
  2. I have a symlink from /Users/bsd/Projects/skaffold-jib to /Users/bsd/Manumitting/Projects/skaffold-jib. So just using filepath.Abs is insufficient as it does not expand symlinks.

This patch adds a util.Canonical(path) method that attempts to expand a provided path, and uses this new method to compare canonicalized paths.

@briandealwis
Copy link
Member Author

Odd, I'm now seeing this error too.

@briandealwis
Copy link
Member Author

briandealwis commented Oct 15, 2018

I thought jib:_skaffold-files returned canonical file paths, but it does not.
Ah: jib:_skaffold-files does return canonical file paths, but the test in pkg/skaffold/jib/jib_test.go does not. It's probably safest to just canonicalize files in the comparison.

Even, better, use os.SameFile().

@briandealwis
Copy link
Member Author

Closing in favour of #1167.

@briandealwis briandealwis deleted the canonical branch October 15, 2018 18:28
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.

2 participants