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

check_language.py misc. problems #908

Open
oddhack opened this issue Aug 30, 2020 · 3 comments
Open

check_language.py misc. problems #908

oddhack opened this issue Aug 30, 2020 · 3 comments

Comments

@oddhack
Copy link

oddhack commented Aug 30, 2020

Some things I noted trying to run the script on the Vulkan repo:

  • Seems to require python 3, which /usr/bin/env python may return python 2 in some environments. It's possible asking for 'env python3' would suffice. If run with python 2, exits immediately with no failure code. Python 2 is at EOL now and I am definitely not suggesting backwards compatibility, just that it execute the right Python version.
  • Matches 'master' inside a link to another repository, e.g. https:...blob/master/README.md, and should not as that's not under control of the source document. Also matches the word in certain other contexts where it might not be appropriate to report - frex we have a Python script containing a list of branch names and metadata for them, which looks like

'master': [ 4000, 4999, 4448 ],

I'll note additional problems here as I encounter them, unless you want separate issues for them. I don't know to what degree you are willing to make this a more general-purpose tool vs. just running on Amber, but we're kinda hoping to leverage off this.

@dj2
Copy link
Collaborator

dj2 commented Aug 30, 2020

I've run into both of those problem myself.

I don't know the best way to find python3 on a system. Maybe I can check if we're under python2 and error out in some fashion? Would at least make it more noticeable (@zoddicus any suggestions?)

For the detecting of master, I see two ways to do it, either we add some kind of suppression mechanism in the file

'master': [4000, 4999, 4448], # NOLANG(master)

or you do something along the lines of running the script, fixing up issues but leaving ones like master and then creating a suppression file that you diff against.

Would either of those options work for you (or do you see another option?)

@oddhack
Copy link
Author

oddhack commented Aug 31, 2020

It is tempting to have an allowlist of regexps as well as a denylist but that could lead to false negatives unless specific expressions in the allowlist are run only for specific expressions that triggered in the denylist, and that would significantly complicate the script.

It might be easiest to just edit the regexps. It is hard to make general statements but any URL containing one of the problematic words is more-than-likely OK no matter the context. Quoted content is less obviously OK though.

@oddhack
Copy link
Author

oddhack commented Aug 31, 2020

Maybe collapse the REGEX_LIST / SUPPRESSION_LIST into a single data structure with (allowed_expression, suppression_expression*)* and then only run the suppression expressions specific to the allowed_expression that just matched? Then could have e.g. [ r"(?i)master", [ r"(?i)[/']master[/']" ] ] for the specific case I noted above.

archimedus pushed a commit to archimedus/amber that referenced this issue Oct 30, 2023
Roll third_party/googletest/ 679bfec6d..34e92be31 (10 commits)

google/googletest@679bfec...34e92be

$ git log 679bfec6d..34e92be31 --date=short --no-merges --format='%ad %ae %s'
2019-11-26 absl-team Googletest export
2019-11-25 absl-team Googletest export
2019-11-25 mate.pek README.md: added Catch2 and Google Test Explorer
2019-11-17 krystian.kuzniarek unify googletest and googlemock main functions
2019-11-17 krystian.kuzniarek remove MSVC workaround: wmain link error in the static library
2019-11-22 krystian.kuzniarek remove Nokia's Symbian compiler workaround: SafeMatcherCastImpl
2019-11-22 krystian.kuzniarek consistency fix for SafeMatcherCastImpl member functions
2019-11-14 krystian.kuzniarek remove MSVC workaround: accessing namespace scope from within nested classes
2019-11-22 krystian.kuzniarek remove g++ 3.3 workaround: using on operator<<
2019-11-05 krystian.kuzniarek change incorrect comments

Roll third_party/shaderc/ ff260580c..1d6155d86 (4 commits)

google/shaderc@ff26058...1d6155d

$ git log ff260580c..1d6155d86 --date=short --no-merges --format='%ad %ae %s'
2019-11-27 rharrison Moving spirv-cross dep from Dawn into shaderc (google#911)
2019-11-26 rharrison Disable shaderc library target in BUILD.gn (google#910)
2019-11-26 9856269+sarahM0 spvc: Fix spvc_spirv_cross_spvc_parser_tests failure by updating known_spvc_filures (google#909)
2019-11-26 rharrison Rolling 5 dependencies (google#908)

Roll third_party/spirv-tools/ 85f3e93d1..52e9cc930 (7 commits)

KhronosGroup/SPIRV-Tools@85f3e93...52e9cc9

$ git log 85f3e93d1..52e9cc930 --date=short --no-merges --format='%ad %ae %s'
2019-11-27 afdx spirv-fuzz: Improve debugging facilities (#3074)
2019-11-27 stevenperron Handle unreachable block when computing register pressure (#3070)
2019-11-27 greg Improve RegisterSizePasses (#3059)
2019-11-26 headlessclayton utils/vscode: Add install.bat (#3071)
2019-11-26 52076061+digit-google build: cmake: Add support for Fuchsia. (#3062)
2019-11-26 dneto Add test with explicit example of stripping reflection info (#3064)
2019-11-26 9856269+sarahM0 Permit the debug instructions in WebGPU SPIR-V (#3063)

Roll third_party/swiftshader/ 663dcefa2..8a6dcf763 (3 commits)

https://swiftshader.googlesource.com/SwiftShader.git/+log/663dcefa22ea..8a6dcf76315c

$ git log 663dcefa2..8a6dcf763 --date=short --no-merges --format='%ad %ae %s'
2019-11-26 sugoi Support sample image instruction operand
2019-11-27 jmadill Fix ICD build when using multiple toolchains.
2019-11-26 chrisforbes gles: Only clamp default block uniform indexes

Roll third_party/vulkan-loader/ fc962d080..2d6f74c6d (2 commits)

KhronosGroup/Vulkan-Loader@fc962d0...2d6f74c

$ git log fc962d080..2d6f74c6d --date=short --no-merges --format='%ad %ae %s'
2019-11-27 tobine build:Add def file for Windows GN build
2019-11-26 shannon build: Update known-good for 1.1.129 header

Roll third_party/vulkan-validationlayers/ 8c954d5a4..567954684 (2 commits)

KhronosGroup/Vulkan-ValidationLayers@8c954d5...5679546

$ git log 8c954d5a4..567954684 --date=short --no-merges --format='%ad %ae %s'
2019-11-22 digit+github build: Fix Vulkan headers detection with CMake
2019-11-25 jbolz layers: Remove MemTrack-FreedMemRef that has moved to best_practices

Created with:
  roll-dep third_party/dxc third_party/glslang third_party/googletest third_party/lodepng third_party/shaderc third_party/spirv-headers third_party/spirv-tools third_party/swiftshader third_party/vulkan-headers third_party/vulkan-loader third_party/vulkan-validationlayers
dneto0 added a commit to dneto0/amber that referenced this issue Jun 6, 2024
dneto0 added a commit that referenced this issue Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants