Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Modified files with CRLF block CI #1443

Closed
BridgeAR opened this issue Aug 13, 2018 · 3 comments
Closed

Modified files with CRLF block CI #1443

BridgeAR opened this issue Aug 13, 2018 · 3 comments
Labels
ci-public

Comments

@BridgeAR
Copy link
Member

See https://ci.nodejs.org/job/node-test-commit-v8-linux/1590/nodes=rhel72-s390x,v8test=v8test/console

This has happened multiple times now. Is there something we can do to prevent running into this situation?

...
13:53:24 # Untracked files:
13:53:24 #   (use "git add <file>..." to include in what will be committed)
13:53:24 #
13:53:24 #	build/
13:53:24 #	depot_tools/
13:53:24 no changes added to commit (use "git add" and/or "git commit -a")
...
13:53:24 + git rebase --committer-date-is-author-date origin/master
13:53:24 Cannot rebase: You have unstaged changes.
@joyeecheung
Copy link
Member

joyeecheung commented Aug 14, 2018 •
edited
Loading

It's not untracked files that block the CI, but modified files. Specifically, deps/v8/third_party/jinja2/LICENSE which uses CRLF. See nodejs/reliability#12 (comment) and nodejs/reliability#12 (comment)

This also happens in regular CI and has failed 12 out of 20 recent PR CI runs (tonight in my time zone)

@joyeecheung joyeecheung changed the title Untracked files block v8 CI Modified files with CRLF block CI Aug 14, 2018
@mhdawson mhdawson mentioned this issue Aug 14, 2018
Closed
This was referenced Aug 14, 2018
Closed
Closed
Closed
refack pushed a commit to joyeecheung/node that referenced this issue Aug 17, 2018
@joyeecheung @refack
deps: fix CRLF in text file not present in upstream
76c07bf 
`deps/v8/third_party/jinja2/LICENSE` is not in upstream v8,
and contains CRLF, which is in conflict with `deps/v8.gitattributes`
which sets all text files to use LF.
This has caused failures in CI workers with older versions of Git.
This patch manually fixes up the file to use LF to resolve
the conflict.

The file has already been fixed in upstream jinja2,
which is pull into our repo when we update V8 so it should
be fixed the next time we update V8.

PR-URL: nodejs#22340
Refs: nodejs/build#1443
Refs: nodejs/reliability#12
Refs: nodejs/build#1453
Refs: https://chromium-review.googlesource.com/c/993812/
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
joyeecheung added a commit to nodejs/node that referenced this issue Aug 17, 2018
@joyeecheung
deps: fix CRLF in text file not present in upstream
478a78b 
`deps/v8/third_party/jinja2/LICENSE` is not in upstream v8,
and contains CRLF, which is in conflict with `deps/v8/.gitattributes`
which sets all text files to use LF.
This has caused failures in CI workers with older versions of Git.
This patch manually fixes up the file to use LF to resolve
the conflict.

The file has already been fixed in upstream jinja2,
which is pull into our repo when we update V8 so it should
be fixed the next time we update V8.

PR-URL: #22340
Refs: nodejs/build#1443
Refs: nodejs/reliability#12
Refs: nodejs/build#1453
Refs: https://chromium-review.googlesource.com/c/993812/
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit to nodejs/node that referenced this issue Aug 19, 2018
@joyeecheung @targos
deps: fix CRLF in text file not present in upstream
41fcf49 
`deps/v8/third_party/jinja2/LICENSE` is not in upstream v8,
and contains CRLF, which is in conflict with `deps/v8/.gitattributes`
which sets all text files to use LF.
This has caused failures in CI workers with older versions of Git.
This patch manually fixes up the file to use LF to resolve
the conflict.

The file has already been fixed in upstream jinja2,
which is pull into our repo when we update V8 so it should
be fixed the next time we update V8.

PR-URL: #22340
Refs: nodejs/build#1443
Refs: nodejs/reliability#12
Refs: nodejs/build#1453
Refs: https://chromium-review.googlesource.com/c/993812/
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@mhdawson mhdawson mentioned this issue Aug 29, 2018
Closed
targos pushed a commit to nodejs/node that referenced this issue Sep 3, 2018
@joyeecheung @targos
deps: fix CRLF in text file not present in upstream
eab377f 
`deps/v8/third_party/jinja2/LICENSE` is not in upstream v8,
and contains CRLF, which is in conflict with `deps/v8/.gitattributes`
which sets all text files to use LF.
This has caused failures in CI workers with older versions of Git.
This patch manually fixes up the file to use LF to resolve
the conflict.

The file has already been fixed in upstream jinja2,
which is pull into our repo when we update V8 so it should
be fixed the next time we update V8.

PR-URL: #22340
Refs: nodejs/build#1443
Refs: nodejs/reliability#12
Refs: nodejs/build#1453
Refs: https://chromium-review.googlesource.com/c/993812/
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@mhdawson mhdawson mentioned this issue Sep 19, 2018
Closed
@refack
Copy link
Contributor

refack commented Sep 25, 2018

I think this has been resolved on the two fronts.

  1. The irksome file deps/v8/third_party/jinja2/LICENSE was normalized
  2. autocrlf & safecrlf was disabled on as many CI worker as possible.

I'm not sure of any further actionable items. If noone has any suggestion we could close this issue.

@BridgeAR
Copy link
Member Author

@refack thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ci-public
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@refack @maclover7 @joyeecheung @BridgeAR