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

Merge equivalent require blocks #2406

Merged
merged 2 commits into from
Aug 16, 2024
Merged

Merge equivalent require blocks #2406

merged 2 commits into from
Aug 16, 2024

Conversation

Waffl3x
Copy link
Contributor

@Waffl3x Waffl3x commented Aug 14, 2024

Addresses #2404
Merges extension require blocks that have identical children, extending the depends attribute with a logical or.

Addresses KhronosGroup#2404
Merges extension require blocks that have identical children,
extending the depends attribute with a logical or.
@CLAassistant
Copy link

CLAassistant commented Aug 14, 2024

CLA assistant check
All committers have signed the CLA.

@Waffl3x
Copy link
Contributor Author

Waffl3x commented Aug 14, 2024

Here is a script I made in python that finds these cases, I'm not really sure how to handle license stuff, and I'm not sure where it should go so I didn't add it with this pull request. I'm also not very familiar with python so it has some flaws, namely that it just looks for a vk.xml in the same directory as the script, so I don't feel like it's good enough to be included as-is.

import xml.etree.ElementTree as etree
import itertools

tree = etree.parse('vk.xml')
doc = tree.getroot()

extensions = doc.find('extensions')

def check_if_children_equal(req1, req2):
    if len(req1) != len(req2):
        return False
    for child1, child2 in zip(req1, req2):
        if child1.tag != child2.tag:
            return False
        if child1.attrib != child2.attrib:
            return False
    return True
# def check_if_children_equal

for ext in extensions:
    req_with_depends = []
    # put all candidates into a list
    for req, count in zip(ext, itertools.count()):
        if 'depends' in req.attrib:
            req_with_depends.append((req, count))
    
    # compare candidates in a pairwise manner
    for tup1, it2_start in zip(req_with_depends[:-1], range(1, len(req_with_depends))):
        for tup2 in req_with_depends[it2_start:]:
            req1, req_count1 = tup1
            req2, req_count2 = tup2
            all_match = check_if_children_equal(req1, req2)
            if all_match:
                print('Found matching require block in extension {}:'.format(ext.attrib['name']))
                print('blocks {} and {} are equal'.format(req_count1, req_count2))
                print('require block {} attributes: {}'.format(req_count1, req1.attrib))
                print('require block {} attributes: {}'.format(req_count2, req2.attrib))
                print('  block children:')
                for child1, child2 in zip(req1, req2):
                    print('    ', req_count1, ': ', child1.attrib)
                    print('    ', req_count2, ': ', child2.attrib)

@oddhack
Copy link
Contributor

oddhack commented Aug 15, 2024

Thanks! If you are comfortable putting this at the start:

#!/usr/bin/env python3
#
# Copyright 2024 <your actual name, or other organization holding copyright>
# SPDX-License-Identifier: Apache-2.0

in the header, that will place it under Apache 2.0 on the same terms as other scripts in the repo and allow us to redistribute it while still giving you credit. It should go in the "scripts/" directory.

@Waffl3x
Copy link
Contributor Author

Waffl3x commented Aug 15, 2024

Should be good now I think

@@ -0,0 +1,46 @@
#!/usr/bin/env python3
#
# Copyright 2024 <your actual name, or other organization holding copyright>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace "your actual name, or other organization holding copyright" with, well, what it says :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, xd, yeah let me fix that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe my pseudonym should be fine, I contributed to GCC under this name, the Free Software Foundation has the paperwork for that. I don't know if that's compatible or whatnot, but hopefully it's fine. If not I guess I can use my legal name but I would like to avoid that if possible.

This script prints out require blocks that are eligible to be
merged together.
@oddhack oddhack added this to the Signed-off to Merge milestone Aug 16, 2024
@oddhack oddhack merged commit 8f60e57 into KhronosGroup:main Aug 16, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

3 participants