Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Commit vendor/ with LF-style line endings #663

Merged
merged 4 commits into from
May 24, 2017
Merged

Conversation

seanknox
Copy link
Contributor

@seanknox seanknox commented May 23, 2017

What this PR does / why we need it:

  • Commits vendor/ with LF-style line endings to ensure vendor/ doesn't change every time glide install is run on Windows
  • No functional change.
  • acs-engine devs using Windows must set git config --global core.autocrlf input so git will automatically convert Windows-style CRLF endings to UNIX LF.

Which issue this PR fixes vendor/ was changing every time I ran make prereqs on macOS. This will commit + Windows devs setting core.autocrlf=input in their gitconfig should prevent noise.

Special notes for your reviewer:

Release note:


This change is Reviewable

@seanknox
Copy link
Contributor Author

@squillace @anhowe @colemickens To test, I did the following:

  1. ran glide install on my Mac which repulls dependencies with UNIX LF line endings
  2. committed (this PR)
  3. On a Windows 10 VM with acs-engine, ran glide install
  4. Confirmed with git status that vendor/ was unchanged

If either of y'all are on Windows, you can test by cloning/pulling, git checkout fix-line-encodings, and running make prereqs (assuming you have make on your box. Ultimately what's important is that you download and run glide:

prereqs:
	go get github.com/Masterminds/glide
	go get github.com/jteeuwen/go-bindata/...
	glide install

@seanknox
Copy link
Contributor Author

Oh, and you'll need to configure Git to convert CRLF to LF on commit:

just for this repo:
git config core.autocrlf input

globally (all projects you use git with)
git config --global core.autocrlf input

@seanknox
Copy link
Contributor Author

@acs-bot test this please

1 similar comment
@seanknox
Copy link
Contributor Author

@acs-bot test this please

@JackQuincy
Copy link
Contributor

How does this behave when you check it out in Linux?

@seanknox
Copy link
Contributor Author

seanknox commented May 24, 2017

Didn't try Linux, though all *NIX OSes (Linux, macOS, etc.) use LF-style line endings.

[edit]: and because they use LF-style endings, it will work fine on them. Sorry I left the key piece of info out.

Copy link
Contributor

@JackQuincy JackQuincy left a comment

Choose a reason for hiding this comment

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

LGTM, pulled the branch locally and in windows when I use the dockerfile like we ask devs to I had correct behavior.

@seanknox
Copy link
Contributor Author

seanknox commented May 24, 2017

Awesome. I confirmed Linux does the right thing too (just in case my neckbeard lore was flimsy):

$ docker run --rm -it ubuntu bash
root@29a1767814c2:~ go get github.com/Azure/acs-engine && cd $GOPATH/src/github.com/Azure/acs-engine
root@29a1767814c2:~/go/src/github.com/Azure/acs-engine# git checkout fix-line-encodings
root@29a1767814c2:~/go/src/github.com/Azure/acs-engine# make prereqs
go get github.com/Masterminds/glide
go get github.com/jteeuwen/go-bindata/...
glide install
[INFO]	Downloading dependencies. Please wait...
[INFO]	--> Found desired version locally github.com/Azure/azure-sdk-for-go 5841475edc7c8725d79885d635aa8956f97fdf0e!
[INFO]	--> Found desired version locally github.com/Azure/go-autorest 58f6f26e200fa5dfb40c9cd1c83f3e2c860d779d!
[INFO]	--> Found desired version locally github.com/dgrijalva/jwt-go 6c8dedd55f8a2e41f605de6d5d66e51ed1f299fc!
...
root@29a1767814c2:~/go/src/github.com/Azure/acs-engine# git status
On branch fix-line-encodings
Your branch is up-to-date with 'origin/fix-line-encodings'.
nothing to commit, working directory clean

@seanknox
Copy link
Contributor Author

CI timed out after nearly an hour. We'll fix this. Re-running again.

Error [deploy_template test-acs-kubernetes-westus-472-0]
SUCCESS [cleanup test-acs-kubernetes-westus-472-0]
Error: Test failed
Build step 'Execute shell' marked build as failure
Archiving artifacts
Setting status of 1ecacee64c597a9f760b3779c944f230de966977 to FAILURE with url https://jenkins.azure-containers.io/job/acs-engine-pr/472/ and message: 'Build finished. '
Finished: FAILURE

@seanknox
Copy link
Contributor Author

@acs-bot test this please

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants