Skip to content

Use StdEncoding for git hash directory name #6071

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

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Jun 23, 2021

Fixes: #6025

Description

base64.UrlEncoding uses the "alternative alphabet" from RFC 4648, which supports the - characters. When this character is the leading character of a computed hash, using this as an argument to any git command causes an error because it's misinterpreted as a command switch.

base64.StdEncoding does not support the - character in its alphabet, so we should be able to use this encoder as a drop-in replacement with no issues.

@nkubala nkubala requested a review from a team as a code owner June 23, 2021 17:55
@nkubala nkubala requested a review from MarlonGamez June 23, 2021 17:55
@google-cla google-cla bot added the cla: yes label Jun 23, 2021
@codecov
Copy link

codecov bot commented Jun 23, 2021

Codecov Report

Merging #6071 (5e45356) into master (3a531d6) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6071      +/-   ##
==========================================
- Coverage   70.22%   70.21%   -0.02%     
==========================================
  Files         474      474              
  Lines       18165    18165              
==========================================
- Hits        12757    12755       -2     
- Misses       4473     4474       +1     
- Partials      935      936       +1     
Impacted Files Coverage Δ
pkg/skaffold/git/gitutil.go 65.43% <100.00%> (ø)
pkg/skaffold/util/tar.go 50.66% <0.00%> (-5.34%) ⬇️
pkg/skaffold/docker/parse.go 87.14% <0.00%> (+0.95%) ⬆️

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 3a531d6...5e45356. Read the comment docs.

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm !

@MarlonGamez MarlonGamez merged commit 9293ed8 into GoogleContainerTools:master Jun 23, 2021
@nkubala nkubala deleted the git-hash branch June 24, 2021 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some hashes may cause git clone to fail
2 participants