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.

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

"make tidy" fails on windows #7723

Closed
slaren opened this issue Jul 11, 2013 · 8 comments
Closed

"make tidy" fails on windows #7723

slaren opened this issue Jul 11, 2013 · 8 comments
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. O-windows Operating system: Windows

Comments

@slaren
Copy link

slaren commented Jul 11, 2013

The file licenseck.py (ironically) fails its own license check, apparently because git uses CR LF line endings on windows for .py files with core.autocrlf enabled.

Also note that tidy is a target of check, so in turn this is making "make check" fail.

@slaren
Copy link
Author

slaren commented Jul 11, 2013

Adding *.py to the list of extensions in .gitattributes fixes the problem. Another option would be adding a "# xfail-license" tag to licenseck.py like the other .py files have, or making tidy.py remove the CR characters before passing the contents to do_license_check.

@pnkfelix
Copy link
Member

part of #8058

@klutzy
Copy link
Contributor

klutzy commented Aug 15, 2013

Small fix:

--- a/src/etc/licenseck.py
+++ b/src/etc/licenseck.py
@@ -80,6 +80,7 @@ exceptions = [

 def check_license(name, contents):
     valid_license = False
+    contents = contents.replace("\r\n", "\n")
     for a_valid_license in licenses:
         if contents.startswith(a_valid_license):
             valid_license = True

@huonw
Copy link
Member

huonw commented Aug 15, 2013

@klutzy I think

diff --git a/src/etc/tidy.py b/src/etc/tidy.py
old mode 100644
new mode 100755
index 4815bc6..8f5abd4
--- a/src/etc/tidy.py
+++ b/src/etc/tidy.py
@@ -41,6 +41,7 @@ current_contents = ""

 try:
     for line in fileinput.input(file_names,
+                                mode='rU',
                                 openhook=fileinput.hook_encoded("utf-8")):

         if fileinput.filename().find("tidy.py") == -1:

is the more reliable solution. (It uses Python's built-in support to implicitly treat \r, \n and \r\n as newline separators.)

@klutzy
Copy link
Contributor

klutzy commented Sep 4, 2013

@huonw I thought yours is better, but actually it does not work.
If openhook=fileinput.hook_encoded(...) is given, hook_encoded uses codecs.open and it ignores any newline conversion.

@adrientetar
Copy link
Contributor

Could that be fixed on the .gitattributes?

* text=auto
*.py eol=lf
https://www.kernel.org/pub/software/scm/git/docs/gitattributes.html

http://stackoverflow.com/a/10017566/2037879

@klutzy
Copy link
Contributor

klutzy commented Sep 15, 2013

@adridu59 In the case we have to force lf for nearly all source code, since make tidy reads files and compares its header and predefined license texts.
Anyway, I'm totally ok with forcing lf. In fact we already force it for some files since crlf leaded curious build failure (#8731 / #8738). I guess crlf can break more thing than we know so far.

@adrientetar
Copy link
Contributor

Looking at some refs:
http://stackoverflow.com/questions/2517190/how-do-i-force-git-to-use-lf-instead-of-crlf-under-windows
http://git-scm.com/docs/gitattributes
http://stackoverflow.com/questions/9933004/best-way-to-disable-git-end-of-line-normalization-crlf-to-lf-across-all-clones/10017566#10017566
It looks like * text=auto basically just does the automatic normalization, which means that if your git if configured as lf all files will be marked lf and vice-versa.
Now I think that replacing it with * eol=lf could probably override that by forcing LF on all files. Any thoughts?

EDIT: apparently adding *.* eol=lf should do the trick.
Github has a doc for that.

cc @thestinger

bors added a commit that referenced this issue Sep 17, 2013
This avoids default CRLF on msysgit for Windows which can cause trouble.
Cf. https://help.github.com/articles/dealing-with-line-endings#text-eollf

Closes #7723.
@bors bors closed this as completed in 62991e6 Sep 17, 2013
flip1995 pushed a commit to flip1995/rust that referenced this issue Nov 4, 2021
Fix manual_assert and match_wild_err_arm for `#![no_std]` and Rust 2021

Rust 2015 `std::panic!` has a wrapping block while `core::panic!` and Rust 2021 `std::panic!` does not. See rust-lang#88919 for details.

Note that the test won't pass until clippy changes in rust-lang#88860 is synced.

---

changelog: Fix [`manual_assert`] and [`match_wild_err_arm`] for `#![no_std]` and Rust 2021.

Fixes rust-lang#7723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. O-windows Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants