test: add new python linter to check file names and permissions

windsok commented at 1:50 am on April 21, 2021: contributor

Adds a new python linter test which tests for correct filenames and file permissions in the repository.

Replaces the existing tests in the test/lint/lint-filenames.sh and test/lint/lint-shebang.sh linter tests, as well as adding some new and increased testing. This increased coverage is intended to catch issues such as in #21728 and https://github.com/bitcoin/bitcoin/pull/16807/files#r345547050

Summary of tests:

Checks every file in the repository against an allowed regexp to make sure only lowercase or uppercase alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.
Checks only source files (*.cpp, *.h, *.py, *.sh) against a stricter allowed regexp to make sure only lowercase alphanumerics (a-z0-9), underscores (_), hyphens (-) and dots (.) are used in source code filenames. Additionally there is an exception regexp for directories or files which are excepted from matching this regexp (This should replicate the existing test/lint/lint-filenames.sh test)
Checks all files in the repository match an allowed executable or non-executable file permission octal. Additionally checks that for executable files, the file contains a shebang line.
Checks that for executable .py and .sh files, the shebang line used matches an allowable list of shebangs (This should replicate the existing test/lint/lint-shebang.sh test)
Checks every file that contains a shebang line to ensure it has an executable permission

Additionally updates the permissions on various files to comply with the new tests.

Fixes #21729

fanquake added the label Tests on Apr 21, 2021

in test/lint/lint-filenames-permissions.py:17 in d0016b1be5 outdated

12+import sys
13+from subprocess import check_output
14+
15+CMD_ALL_FILES = "git ls-files --full-name"
16+CMD_SOURCE_FILES = 'git ls-files --full-name -- "*.[cC][pP][pP]" "*.[hH]" "*.[pP][yY]" "*.[sS][hH]"'
17+CMD_SHEBANG_FILES = """git grep --full-name --line-number -I '^#!' | grep :1: | cut -d ":" -f 1"""

practicalswift commented at 8:01 am on April 21, 2021:

Nit: Consider doing the filtering (grep, cut) in Python instead to make the script easier to read and maintain. In the Python code a comment could be added that :1: is used to make sure only shebangs on the first line are considered.

windsok commented at 3:03 am on April 22, 2021:

updated to move the filtering into python :)

in test/lint/lint-filenames-permissions.py:39 in d0016b1be5 outdated

34+    alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.
35+    """
36+    files = check_output(CMD_ALL_FILES, shell=True).decode("utf8").strip().split("\n")
37+    filename_regex = re.compile(ALLOWED_FILENAME_REGEXP)
38+    exit_code = 0
39+    for file in files:

practicalswift commented at 8:02 am on April 21, 2021:

Nit: file is a Python builtin. Consider using filename/filenames instead of file/files :)

windsok commented at 3:03 am on April 22, 2021:

good call, updated

in test/lint/lint-filenames-permissions.py:122 in d0016b1be5 outdated

117+    exit_code = 0
118+    for file in files:
119+        file_perms = int(oct(os.stat(file).st_mode)[-3:])
120+        if file_perms != ALLOWED_PERMISSION_EXECUTABLES:
121+            print(
122+                f"""File "{file}" contains a shebang line, but has the file permission {file_perms} instead of the expected executable permission {ALLOWED_PERMISSION_EXECUTABLES}. Do "chmod {ALLOWED_PERMISSION_EXECUTABLES} {file}" (or remove the shebang line)."""

practicalswift commented at 8:10 am on April 21, 2021:

This requirement holds for shebang files that expected to be executed directly.

To avoid this requirement for files that are not expected to be executed directly but still have shebangs consider adding exclusions for:

*.py files that do not contain if __name__ == '__main__': or if __name__ == "__main__":: these are typically modules that are not expected to be executed directly.
*.bash files: these are typically expected to be sourced and not executed directly.

It could be argued that shebangs should be removed from these types of files, but I think shebangs could still be useful to allow for language/dialect auto-detection in editors, shellcheck, etc.

windsok commented at 3:06 am on April 22, 2021:

Updated. I wonder if some other file types should also be skipped along with *bash? I notice the test is failing for some files like contrib/init/bitcoind.init, contrib/init/bitcoind.openrc, contrib/devtools/split-debug.sh.in and I’m thinking they might fit into the same category of files which are sourced but not executed?

There is also quite a few .sh files in ci/test/ like ci/test/00_setup_env_android.sh which fail the test, but I’m not sure if those really should be forced to be executable or not?

Thanks very much for the initial review and feedback :)

practicalswift commented at 7:19 pm on April 22, 2021:

Updated. I wonder if some other file types should also be skipped along with *bash? I notice the test is failing for some files like contrib/init/bitcoind.init, contrib/init/bitcoind.openrc, contrib/devtools/split-debug.sh.in and I’m thinking they might fit into the same category of files which are sourced but not executed?

Sounds reasonable. Skipping the requirement for *.{init,openrc,sh.in} should be fine.

There is also quite a few .sh files in ci/test/ like ci/test/00_setup_env_android.sh which fail the test, but I’m not sure if those really should be forced to be executable or not?

Perhaps we should simply make these executable to avoid the complexity of making the +x requirement depend also on directory (in addition to #! presence and suffix).

windsok commented at 11:37 pm on April 22, 2021:

Thanks! updated the condition to skip the requirement for *.{init,openrc,sh.in} files.

For the remaining files failing the test, what would be the correct way to go about updating those to comply with the test? Should it be a second commit on this PR?

Thanks again for the review and guidance.

practicalswift changes_requested

practicalswift commented at 8:26 am on April 21, 2021: contributor

Concept ACK: desired properties should where possible be checked automatically.

Linters are to “repo invariants” what constructors are to class invariants :)

Nice to see the linting being done in Python instead of bash: much better in terms of maintainability, portability and readability.

DrahtBot commented at 5:33 pm on April 21, 2021: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

windsok force-pushed on Apr 22, 2021

in test/lint/lint-filenames-permissions.py:17 in 886315ff36 outdated

12+import sys
13+from subprocess import check_output
14+
15+CMD_ALL_FILES = "git ls-files --full-name"
16+CMD_SOURCE_FILES = 'git ls-files --full-name -- "*.[cC][pP][pP]" "*.[hH]" "*.[pP][yY]" "*.[sS][hH]"'
17+CMD_SHEBANG_FILES = """git grep --full-name --line-number -I '^#!'"""

kiminuo commented at 3:00 pm on April 22, 2021:

nit: I wonder is """ required here or would " suffice?

windsok commented at 11:25 pm on April 22, 2021:

Thanks, updated :)

in test/lint/lint-filenames-permissions.py:55 in 886315ff36 outdated

50+    Checks only source files (*.cpp, *.h, *.py, *.sh) against a stricter allowed regexp to make sure only lowercase
51+    alphanumerics (a-z0-9), underscores (_), hyphens (-) and dots (.) are used in source code filenames.
52+
53+    Additionally there is an exception regexp for directories or files which are excepted from matching this regexp.
54+    """
55+    filenames = check_output(CMD_SOURCE_FILES, shell=True).decode("utf8").strip().split("\n")

kiminuo commented at 3:34 pm on April 22, 2021:

Out of curiosity: Would it make sense to replace strip() with rstrip() (https://docs.python.org/3/library/stdtypes.html?highlight=rstrip#str.rstrip)?

Just for fun: There is still one very specific case where this does not work (I hope I’m not missing something): You can add a file like this git add test/util/zzz.py (note the trailing whitespace) and strip() will make sure that test/util/zzz.py will be processed and not test/util/zzz.py . I think your code is fine but if it can be improved wrt robustness I’m all in (possibly split first and remove trailing empty entries?)

windsok commented at 11:32 pm on April 22, 2021:

Great catch!

I’ve updated the logic to not use a strip() at all, and instead split on the newlines and then remove the trailing empty element, as you suggested. I chose to do this using a list comprehension filtering for empty list elements, rather than a .pop() of the last element, as it seemed slightly more robust, but can change it to a pop() if I’m overthinking it.

I tested that this does now catch files with trailing whitespace:

0File "test/util/rpcauth-test.py " does not not match the allowed filename regexp ('^[a-zA-Z0-9/_.@][a-zA-Z0-9/_.@-]*$').

in test/lint/lint-filenames-permissions.py:81 in 886315ff36 outdated

76+    for filename in filenames:
77+        file_extension = os.path.splitext(filename)[1].strip(".")
78+        file_perms = int(oct(os.stat(filename).st_mode)[-3:])
79+
80+        if file_perms == ALLOWED_PERMISSION_EXECUTABLES:
81+            shebang = open(filename, "rb").readline().strip()

kiminuo commented at 3:54 pm on April 22, 2021:

I believe the strip() here is not correct. I don’t think that you can put whitespace before shebang.

It’s a question whether one should allow whitespace after shebank AFAIK it’s allowed but … It may be for the best simply remove .strip().

Just for fun: https://unix.stackexchange.com/questions/276751/is-space-allowed-between-and-bin-bash-in-shebang

windsok commented at 11:34 pm on April 22, 2021:

Great catch, thanks.

I updated this to use rstrip(b"\n") to remove only the trailing newline

windsok force-pushed on Apr 22, 2021

fanquake renamed this:
~~test: add new python linter test for checking filenames and file permissions~~
test: add new python linter to check file names and permissions
on Apr 23, 2021

fanquake commented at 4:22 am on April 23, 2021: member

Can you update the commit message to use the following format:

0prefix: title
1
2commit body

i.e:

0test: add new python linter to check file names and permissions
1
2explain what is being done in this commit
3explain some more etc

Also, for this to me merged. Obviously all linters / CI need to be passing. So if changes to other files need to be made, they will have to be done either in preceding commits, or as part of the same change.

windsok force-pushed on Apr 24, 2021

test: fix file permissions on various scripts

Updates permissions on files to comply with the new test added in the following commit

6f6bb3ebc7

windsok commented at 0:19 am on April 24, 2021: contributor

Can you update the commit message to use the following format:

Thanks @fanquake - I’ve updated the commit messages to the standard format

Also, for this to me merged. Obviously all linters / CI need to be passing. So if changes to other files need to be made, they will have to be done either in preceding commits, or as part of the same change.

Done - added a new commit before the main commit which updates the permissions on files which are failing the new test.

Would appreciate any additional review and feedback :)

windsok marked this as ready for review on Apr 24, 2021

practicalswift commented at 7:14 am on April 24, 2021: contributor

cr ACK 52c3fb3c9d4145819bd0fe839ba89482e73fce99: patch looks correct!

Nice first-time contribution @windsok. I hope to see more contributions from you in the future. Warm welcome as a contributor!

MarcoFalke added the label Needs gitian build on Apr 24, 2021

kiminuo commented at 7:56 am on April 24, 2021: contributor

@windsok This PR removes test/lint/lint-shebang.sh and test/lint/lint-filenames.sh and adds test/lint/lint-filenames-permissions.sh and adds test/lint/lint-filenames-permissions.py.

The new file names are confusing to me as they do something different than they hint. Would it make sense to you to rename them from “lint-filenames-permissions.{py|sh}” to “lint-files.{py|sh}” or something better?

Edit:

This PR modifies https://github.com/bitcoin/bitcoin/blob/master/contrib/gitian-descriptors/assign_DISTNAME permissions but it may be actually correct to add .sh extension - I’m not sure. Just mentioning it here so that others double check correctness of the change.

in test/lint/lint-filenames-permissions.py:176 in 52c3fb3c9d outdated

171+    failed_tests += check_source_filenames()
172+    failed_tests += check_all_file_permissions()
173+    failed_tests += check_shebang_file_permissions()
174+
175+    if failed_tests:
176+        sys.exit(1)

kiminuo commented at 8:05 am on April 24, 2021:

Nit: Given that failed_tests number is computed, one can probably print the number of failed tests too.

windsok commented at 11:41 pm on April 29, 2021:

Great idea, I’ve updated to keep track of the number of failures, and print a message with the total number of failures, if any :)

in test/lint/lint-filenames-permissions.py:40 in 52c3fb3c9d outdated

35+    eg:
36+    'ci/lint_run_all.sh' -> 'sh'
37+    'ci/retry/retry' -> None
38+    'contrib/devtools/split-debug.sh.in' -> 'sh.in'
39+    """
40+    filename_parts = filename.split(os.extsep, 1)

kiminuo commented at 8:17 am on April 24, 2021:

I think people mostly do something like this https://source.dot.net/#System.Private.CoreLib/Path.cs,467ebc0e33e0820c to find a file extension. That is to look for the first os.extsep but not from the beginning of the string (filename) but from the end of the filename.

A counter-example can be 'contrib/devtools/split-debug.my.nice.helper.sh.in' where you would return my.nice.helper.sh.in.

Your code may be perfectly fine for what you want to achieve in this PR, yet I would be hesitant to use get_filename_extension name.

I have no clear suggestion here, letting it up to you to decide. :)

windsok commented at 11:47 pm on April 29, 2021:

Ah yes I originally was using os.path.splitext(self.file_path)[1].strip(".") which works similar to the linked implementation (only return the final extension, not everything from the first .)

I had changed this as I needed a way to detect .sh.in files to exclude them from one of the tests, but you bring up a very good point - with the change I had made it’s possible certain files with weird extensions like split-debug.my.nice.helper.sh would not actually be detected as a sh file, and would not have certain tests enforced.

Ive updated the code to add a new FileMeta class, and have changed the logic to use the appropriate version of the extension in each test. Let me know if this makes sense or not :)

kiminuo commented at 10:21 am on May 1, 2021:

It looks fine to me. Thanks for updating it. :)

DrahtBot removed the label Needs gitian build on Apr 25, 2021

windsok force-pushed on Apr 29, 2021

windsok commented at 11:48 pm on April 29, 2021: contributor

@windsok This PR removes test/lint/lint-shebang.sh and test/lint/lint-filenames.sh and adds test/lint/lint-filenames-permissions.sh and adds test/lint/lint-filenames-permissions.py.

The new file names are confusing to me as they do something different than they hint. Would it make sense to you to rename them from “lint-filenames-permissions.{py|sh}” to “lint-files.{py|sh}” or something better?

I’m happy to name this test as lint-files.{py|sh} - it does seem clearer and less confusing. Thanks for the suggestion :) I’ve updated this in the latest version commit.

This PR modifies https://github.com/bitcoin/bitcoin/blob/master/contrib/gitian-descriptors/assign_DISTNAME permissions but it may be actually correct to add .sh extension - I’m not sure. Just mentioning it here so that others double check correctness of the change.

I am also unsure on this one, but happy to take advice on which direction it should go

windsok commented at 11:51 pm on April 29, 2021: contributor

Nice first-time contribution @windsok. I hope to see more contributions from you in the future. Warm welcome as a contributor!

Thanks @practicalswift :) looking forward to getting more involved. Sorry for the delayed response on the last round of feedback.

windsok force-pushed on Apr 29, 2021

test: add new python linter to check file names and permissions

Replaces the existing tests in the test/lint/lint-filenames.sh and test/lint/lint-shebang.sh linter tests, as well as adding some new and increased testing.
Summary of tests:
- Checks every file in the repository against an allowed regexp to make sure only lowercase or uppercase alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.
- Checks only source files (*.cpp, *.h, *.py, *.sh) against a stricter allowed regexp to make sure only lowercase alphanumerics (a-z0-9), underscores (_), hyphens (-) and dots (.) are used in source code filenames. Additionally there is an exception regexp for directories or files which are excepted from matching this regexp (This should replicate the existing test/lint/lint-filenames.sh test)
- Checks all files in the repository match an allowed executable or non-executable file permission octal. Additionally checks that for executable files, the file contains a shebang line.
- Checks that for executable .py and .sh files, the shebang line used matches an allowable list of shebangs (This should replicate the existing test/lint/lint-shebang.sh test)
- Checks every file that contains a shebang line to ensure it has an executable permission

Fixes #21729

46b025e00d

practicalswift commented at 9:13 am on April 30, 2021: contributor

cr re-ACK 46b025e00df40724175735eb5606ac73067cb3b8: patch still looks correct

MarcoFalke added the label Needs gitian build on Apr 30, 2021

MarcoFalke deleted a comment on Apr 30, 2021

in test/lint/lint-files.py:77 in 46b025e00d

72+    Checks every file in the repository against an allowed regexp to make sure only lowercase or uppercase
73+    alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.
74+    """
75+    # We avoid using rstrip() to ensure we catch filenames which accidentally include trailing whitespace
76+    filenames = check_output(CMD_ALL_FILES, shell=True).decode("utf8").split("\n")
77+    filenames = [filename for filename in filenames if filename != ""]  # removes the trailing empty list element

kiminuo commented at 9:50 am on May 1, 2021:

nit: The comment does not correspond with the code actually.

One could probably do: filenames = filenames[:-1]

windsok commented at 0:56 am on May 5, 2021:

right, I chose to remove the trailing element by checking for an empty string as I was worried about some case in the future where the output of the git ls-files command changes somehow and the last element is not actually empty. I’m probably overthinking it though.

windsok commented at 1:05 am on May 5, 2021:

On a review of this, I’m fairly certain I can just safely rewrite this code as this one liner, which still catches the original edge case of a file with whitespace at the end of the files list.

0filenames = check_output(CMD_ALL_FILES, shell=True).decode("utf8").rstrip('\n').split("\n")

Happy to update this if people don’t mind re-reviewing. Let me know.

Edit: though that said, apparently it is actually possible for a filename to have a newline in it so I guess that one-liner would not catch this case, though it does seem fairly unlikely to happen.

kiminuo commented at 10:04 am on May 5, 2021:

right, I chose to remove the trailing element by checking for an empty string as I was worried about some case in the future where the output of the git ls-files command changes somehow and the last element is not actually empty. I’m probably overthinking it though.

One would need to ask on a Git mailing list whether the text output of the command is stable or not. But your solution works well, so it’s good as it is I think. 👍

MarcoFalke commented at 3:35 pm on May 5, 2021:

If a filename had a newline in its name it would also be split by the split("\n")

windsok commented at 5:27 pm on May 5, 2021:

I’ll think about a way to handle this case. I think currently if this happens the test overall will still fail, as the permissions test would not be able to locate the file and should error out - we can definitely handle this better, though.

MarcoFalke commented at 5:30 pm on May 5, 2021:

I think .rstrip('\n').split("\n") as you suggested is fine

windsok commented at 1:38 am on May 7, 2021:

Addressed in https://github.com/bitcoin/bitcoin/pull/21873

in test/lint/lint-files.py:67 in 46b025e00d

62+    @property
63+    def permissions(self) -> int:
64+        """
65+        Returns the octal file permission of the file
66+        """
67+        return int(oct(os.stat(self.file_path).st_mode)[-3:])

kiminuo commented at 10:16 am on May 1, 2021:

super nit: I’m not sure whether os.stat should behave have follow_symlinks equal to True (default) or False. Possibly relevant is https://stackoverflow.com/questions/954560/how-does-git-handle-symbolic-links/18791647#18791647.

I don’t want to hold up this PR. Leaving this for comment here for the sake of completeness. Maybe it’s stupid. IDK.

windsok commented at 0:51 am on May 5, 2021:

Hmm interesting. It seems that there are currently no symbolic links in the repository itself, at least none that I could find with:

0ls -lR ~/code/bitcoin | grep ^l

I wonder if that is intentional, or it’s just never come up before?

In any case, if there were symbolic links my thinking is that we probably would want to follow them, as it seems that symlinks themselves can’t actually have their permissions changed (At least on linux), so either we would want to exclude symlinks entirely from the permissions test, or just do the default here and follow the link to check the linked file.

kiminuo commented at 10:01 am on May 5, 2021:

My take on this is: I think that adding a symbolic link should produce a warning so that others can notice the fact. The fact that there are no symbolic links at the moment hints that they are probably not necessary.

However, this is really out of scope of this PR, I guess. If somebody finds it important it’s always possible to address this in a follow up PR. :)

kiminuo commented at 10:23 am on May 1, 2021: contributor

code review ACK 46b025e00df40724175735eb5606ac73067cb3b8 if contrib/gitian-descriptors/assign_DISTNAME permission change is deemed OK.

Great contribution!

DrahtBot commented at 9:31 am on May 3, 2021: member

🕵️ @achow101 @sipa have been requested to review this pull request as specified in the REVIEWERS file.

laanwj commented at 3:10 pm on May 5, 2021: member

Code review ACK 46b025e00df40724175735eb5606ac73067cb3b8 Also ACK on the permission changes in 46b025e00df40724175735eb5606ac73067cb3b8. Thanks for working on this.

Nice to see the linting being done in Python instead of bash: much better in terms of maintainability, portability and readability.

Agree. Shell script tends to become virtually unreviewable quickly when the logic gets more complex than simply executing some commands in a batch.

in test/lint/lint-files.sh:7 in 46b025e00d

0@@ -0,0 +1,7 @@
1+#!/usr/bin/env bash
2+
3+export LC_ALL=C
4+
5+set -e
6+cd "$(dirname $0)/../.."
7+test/lint/lint-files.py

laanwj commented at 3:12 pm on May 5, 2021:

Just a question (not a blocker): why does this need a bash wrapper?

windsok commented at 5:23 pm on May 5, 2021:

I believe the main reason is the the lint-all.sh script that CI runs is currently setup to automatically run all .sh tests in the test/lint directory, but won’t automatically run .py files.

In addition it allows lint tests to opt in or out of locale dependence

If we start to move more towards using python based linters, it probably makes sense to make some updates here, so we can get rid of the requirement for the .sh wrapper. @practicalswift may have more context here.

laanwj merged this on May 5, 2021

laanwj closed this on May 5, 2021

in test/lint/lint-files.py:124 in 46b025e00d

119+    filenames = check_output(CMD_ALL_FILES, shell=True).decode("utf8").strip().split("\n")
120+    failed_tests = 0
121+    for filename in filenames:
122+        file_meta = FileMeta(filename)
123+        if file_meta.permissions == ALLOWED_PERMISSION_EXECUTABLES:
124+            shebang = open(filename, "rb").readline().rstrip(b"\n")

MarcoFalke commented at 3:39 pm on May 5, 2021:

nit: shouldn’t open be used in a context manager, so that it is closed again?

windsok commented at 5:13 pm on May 5, 2021:

Good call, I will address in a follow-up

in test/lint/lint-files.py:127 in 46b025e00d

122+        file_meta = FileMeta(filename)
123+        if file_meta.permissions == ALLOWED_PERMISSION_EXECUTABLES:
124+            shebang = open(filename, "rb").readline().rstrip(b"\n")
125+
126+            # For any file with executable permissions the first line must contain a shebang
127+            if shebang[:2] != b"#!":

MarcoFalke commented at 3:43 pm on May 5, 2021:

nit: Doesn’t matter here, but it could use startswith for type safety.

MarcoFalke referenced this in commit f8575bce31 on May 5, 2021

sidhujag referenced this in commit a7812d7be7 on May 5, 2021

sidhujag referenced this in commit 415bdb814f on May 5, 2021

DrahtBot commented at 0:42 am on May 6, 2021: member

Gitian builds

File	commit 3f8f238deb5a72db04d29c899cea11df14cd984c(master)	commit 4f1c6e5ae1e8f26799891178b0a353502e22b87d(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`1d782713b323d8b8...`	`00706d683dc7fdb3...`
*-aarch64-linux-gnu.tar.gz	`d6ea38d31834b245...`	`7fc438afb1329705...`
*-arm-linux-gnueabihf-debug.tar.gz	`4dc746c0a4e127f5...`	`1723f3a260829ade...`
*-arm-linux-gnueabihf.tar.gz	`ee7559f5ce241e5d...`	`4294b3e4f15990e7...`
*-osx-unsigned.dmg	`eef8d656b3f0229d...`	`87a5702ab3f06d26...`
*-osx64.tar.gz	`83f3690cd9006ef3...`	`b3c92c583789b2c7...`
*-powerpc64-linux-gnu-debug.tar.gz	`4e6bb99605730f87...`	`4345d0a6eee207f6...`
*-powerpc64-linux-gnu.tar.gz	`f7f89deb2cb0819a...`	`0b73302264fe0eae...`
*-powerpc64le-linux-gnu-debug.tar.gz	`e1e9e084d4088dec...`	`fa3fbbf1bf64614e...`
*-powerpc64le-linux-gnu.tar.gz	`bc95d5e7ba71cd31...`	`e7633a594fe25426...`
*-riscv64-linux-gnu-debug.tar.gz	`95f182eb1d00e5f5...`	`0701b7b84a8683ee...`
*-riscv64-linux-gnu.tar.gz	`6e1ee8834e0f17ec...`	`94c9e06c8cbc1956...`
*-win64-debug.zip	`1a5115bafee424f2...`	`9363c1aa27d3ce46...`
*-win64-setup-unsigned.exe	`03d79fc50eceea8f...`	`f20c0017ecd90652...`
*-win64.zip	`4333c2e76bf73c2c...`	`1ba0d6d65b89a14a...`
*-x86_64-linux-gnu-debug.tar.gz	`18ce6c08c5bc691e...`	`8645294ec08c6a64...`
*-x86_64-linux-gnu.tar.gz	`a58dbdea99493df5...`	`cc88f00d07b4784c...`
*.tar.gz	`7cdbf224e1151aa1...`	`4419dfaa17218c19...`
bitcoin-core-linux-22-res.yml	`97ceb4f415d8c4a2...`	`85ce1f40436f8cb5...`
bitcoin-core-osx-22-res.yml	`80d164889106ca0a...`	`96f161a171eddcab...`
bitcoin-core-win-22-res.yml	`dd694a3ccfd5e934...`	`12fbc619bafb0987...`
linux-build.log	`c2df17b298bea4de...`	`521a16a9ead3839a...`
osx-build.log	`d185ccafdeb25ee6...`	`0d80d2d9f91e6111...`
win-build.log	`0d0ac751380fb5e4...`	`6a72628fc4afefaf...`
bitcoin-core-linux-22-res.yml.diff		`dcb55fb490600b5b...`
bitcoin-core-osx-22-res.yml.diff		`e7803ad2ab6f4aa8...`
bitcoin-core-win-22-res.yml.diff		`04c8390c0da31c97...`
linux-build.log.diff		`4ed426a1862a5aa1...`
osx-build.log.diff		`1c2436e98e4c7fbe...`
win-build.log.diff		`f6d48e161363f2cf...`

DrahtBot removed the label Needs gitian build on May 6, 2021

MarcoFalke referenced this in commit a33f360fcd on May 7, 2021

in test/lint/lint-files.py:25 in 46b025e00d

20+ALLOWED_SOURCE_FILENAME_REGEXP = "^[a-z0-9_./-]+$"
21+ALLOWED_SOURCE_FILENAME_EXCEPTION_REGEXP = (
22+    "^src/(secp256k1/|univalue/|test/fuzz/FuzzedDataProvider.h)"
23+)
24+ALLOWED_PERMISSION_NON_EXECUTABLES = 644
25+ALLOWED_PERMISSION_EXECUTABLES = 755

ryanofsky commented at 9:24 pm on May 24, 2021:

Since this PR, when I run the linter I see a flood of errors and have to disable it. It seems inappropriate to check user, group, read, and write permissions when the only permission git tracks is whether the file is marked executable. Other permissions are local to the checkout and don’t affect the project or other developers. Errors I see are:

0[...]
1File "test/lint/lint-whitespace.sh" contains a shebang line, but has the file permission 700 instead of the expected executable permission 755. Do "chmod 755 test/lint/lint-whitespace.sh" (or remove the shebang line).
2File "test/util/bitcoin-util-test.py" contains a shebang line, but has the file permission 700 instead of the expected executable permission 755. Do "chmod 755 test/util/bitcoin-util-test.py" (or remove the shebang line).
3File "test/util/rpcauth-test.py" contains a shebang line, but has the file permission 700 instead of the expected executable permission 755. Do "chmod 755 test/util/rpcauth-test.py" (or remove the shebang line).
4ERROR: There were 2464 failed tests in the lint-files.py lint test. Please resolve the above errors.
5^---- failure generated from test/lint/lint-files.sh

gwillen referenced this in commit 6ce592d7cd on Jun 1, 2022

DrahtBot locked this on Aug 16, 2022

test: add new python linter to check file names and permissions #21740

Conflicts

Gitian builds