search

intel / libvpl

Intel® Video Processing Library (Intel® VPL) API, dispatcher, and examples
https://intel.github.io/libvpl/
MIT License
262 stars 80 forks source link

Remove broken workaround for ILP32 detection on ARM #88

Closed StefanBruens closed 1 year ago

StefanBruens commented 1 year ago

Matching the compiler filesystem path with some quite arbitrary string like arm-linux is broken. It will not match a host compiler path like /usr/bin/c++, but may even erroneously match on other architectures.

Second, while it may be possible to make injecting an ILP32 define work, it requires the same workaround in every downstream user.

jonrecker commented 1 year ago

Thank you. This looks okay to me, as the WA is not robust enough to cover all compilers it is better to remove it to avoid confusion. The proper solution will be to update the definition in the header file as in https://github.com/oneapi-src/oneAPI-spec/pull/468

Your commit will be merged into an internal staging repo first for validation and then pushed to this repository in the following release. It will still be your commit in the git log with the original commit author, message etc.

Currently your commit fails one lint check in CI due to message title being slightly too long (max 50 chars). Unfortunately this is not part of automated CI in this repo yet. Would you mind tweaking that and updating the PR? You can locally run "script/lint" to verify that it passes. Thanks.

StefanBruens commented 1 year ago

I could change the subject to e.g. "Remove broken ILP32 detection on ARM workaround", which is only 47 instead of 51 characters, but does that really improve readability?

There is no hard limit in git for the subject line. The only thing approximating the 50 characters is the filename length default of 64, which leaves 53 characters for the subject (5 chars nnnn- prefix, .patch suffix).

The linux kernel policy limits the subject to 72 characters IIRC, with an average of about 50.

jonrecker commented 1 year ago

I could change the subject to e.g. "Remove broken ILP32 detection on ARM workaround", which is only 47 instead of 51 characters, but does that really improve readability?

There is no hard limit in git for the subject line. The only thing approximating the 50 characters is the filename length default of 64, which leaves 53 characters for the subject (5 chars nnnn- prefix, .patch suffix).

The linux kernel policy limits the subject to 72 characters IIRC, with an average of about 50.

Thanks. I have no concern with longer subject lines. This just happens to trip a CI checker. cc @mav-intel who can comment on the lint checker configuration.