RFR: 1236: Add jcheck option to check for large binary files

[Magnus Ihse Bursie]

On Tue, 30 Nov 2021 11:42:56 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> 
> This patch adds the large binary file check to the jcheck.
> 
> The corresponding configuration is like the following code. Each key/value in the `[checks "binary"]` is a mapper. The `key` means the file pattern and the `value` means the limited file size. For example, `.*\.bin=200B` means the file which ends with `.bin` should not exceed 200 Bytes.
> 
> 
> [checks]
> error = binary
> 
> [checks "binary"]
> .*.bin=200B
> .*.o=1k
> 
> 
> The limited file size can use one of these several units:  b(Byte), kb(KiloByte), mb(MegaByte), gb(GigaByte). The units is case insensitive, which means the `Kb`, `kB` and `KB` are equals to `kb(KiloByte)`. If the unit is not provided, it defauts to `b(Byte)`. And these is no a unit called `bit`.
> 
> The corresponding test cases are added.
> 
> Best Regards,
> -- Guoxiong

I agree with the other reviewers that we only need a single binary limit. 

But be aware that actually deploying a change in the main JDK repo which enables this in jcheck is a separate discussion. I don't think we've ever had a hard limit on binary file sizes, only just that you should "try to avoid too large files". I foresee a highly opinionated debate on what size limit to use, if we should even activate it at all. Having a very conservative upper value is more likely to be accepted (let's say the currently largest file times two), and if that seems to work fine, we might try lowering the value.

In a separate, but related note, I'd really like to let jcheck have some kind of "warnings", information that is not strictly blockers, but signals that you should make sure you really intend to do what you're doing. If we had that, then we could have a (generous) upper bound for binary sizes where jcheck makes it an error, and a much lower bound for which jcheck warns you that you perhaps should reconsider.

jcheck/src/main/java/org/openjdk/skara/jcheck/BinaryCheck.java line 69:

> 67:                     try {
> 68:                         fileSize = path != null ? Files.size(path) : 0;
> 69:                         needCheck = true;

Does that mean that jcheck will block renaming an existing binary file if it is larger than the currently accepted maximum? I don't think that is the right behavior. Only adding a new file should trigger this.

-------------

PR: https://git.openjdk.java.net/skara/pull/1247