Skip to content

Use os.SameFile() to check for mvnw working-dir echo bug#1167

Merged
balopat merged 1 commit intoGoogleContainerTools:masterfrom
briandealwis:mvnw-workaround
Oct 15, 2018
Merged

Use os.SameFile() to check for mvnw working-dir echo bug#1167
balopat merged 1 commit intoGoogleContainerTools:masterfrom
briandealwis:mvnw-workaround

Conversation

@briandealwis
Copy link
Copy Markdown
Member

Alternative implementation to workaround the mvnw bug described in #1155. Instead of canonicalizing each file to compare with the working directory, use os.SameFile() to compare the os.Stat() structures. Since we were obtaining the stat structures already, this should have minimal overhead.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 15, 2018

Codecov Report

Merging #1167 into master will decrease coverage by <.01%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1167      +/-   ##
==========================================
- Coverage   42.19%   42.18%   -0.01%     
==========================================
  Files          89       89              
  Lines        3996     3999       +3     
==========================================
+ Hits         1686     1687       +1     
- Misses       2142     2143       +1     
- Partials      168      169       +1
Impacted Files Coverage Δ
pkg/skaffold/jib/jib.go 84.84% <60%> (-5.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9efeeac...8e44c8d. Read the comment docs.

@balopat balopat merged commit ea5382d into GoogleContainerTools:master Oct 15, 2018
@briandealwis briandealwis deleted the mvnw-workaround branch October 15, 2018 20:49
@briandealwis
Copy link
Copy Markdown
Member Author

Turns out the mvnw.cmd for Windows outputs Found "<maven-wrapper.jar path>"

https://github.com/takari/maven-wrapper/blob/4096f68f48873e98af8376a30167afec4946d37d/mvnw.cmd#L129-L138

@briandealwis
Copy link
Copy Markdown
Member Author

takari/maven-wrapper#115

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.

4 participants