8000 Improve reliability of header file generation using context manager by Doekin · Pull Request #681 · p11-glue/p11-kit · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Improve reliability of header file generation using context manager #681

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 11, 2025
Merged

Improve reliability of header file generation using context manager #681

merged 3 commits into from
Jun 11, 2025

Conversation

Doekin
Copy link
Contributor
@Doekin Doekin commented Mar 28, 2025

This PR addresses an issue where the gen-fixed-closures.py script fails to write to virtual-fixed-closures.h on Windows, causing p11-kit to not build properly. The problem occurs when using Meson 1.7.0 and Python 3.13.2.

The following minimal reproduction example demonstrates the issue:

import argparse

def foo(output):
    output.write("OK\n")

if __name__ == "__main__":
    parser = argparse.ArgumentParser()
    parser.add_argument("--outfile", type=argparse.FileType("w"))
    args = parser.parse_args()
    foo(args.outfile)

Running meson runpython foo.py --outfile t.h does not write "OK" to t.h.

kalvdans
kalvdans approved these changes Mar 28, 2025
View reviewed changes
Copy link
@kalvdans kalvdans left a comment

Choose a reason for hiding this comment

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

This is in line with the recommendation at https://docs.python.org/3/library/argparse.html#type
Approve! (but I have no merge rights)

ueno
ueno approved these changes Mar 28, 2025
View reviewed changes
Copy link
Member
@ueno ueno left a comment

Choose a reason for hiding this comment

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

Thank you; I wonder if other scripts (e.g., p11-kit/gen-wrappers.py) need the same treatment?

@coveralls
Copy link
coveralls commented Mar 28, 2025 •
edited
Loading

Coverage Status

coverage: 69.075% (-0.02%) from 69.097%
when pulling 72f9b5f on Doekin:gen-fixed-closures
into cd76113 on p11-glue:master.

@Doekin
Copy link
Contributor Author
Doekin commented Mar 28, 2025 •
edited
Loading

The other scripts currently work fine, but I'm not completely sure they'll always perform as expected. I'll update them too.

@Doekin Doekin mentioned this pull request Mar 28, 2025
Merged
@Doekin Doekin changed the title gen-fixed-closures.py: Use with statement to ensure file contents are written Improve reliability of header file generation using context manager Mar 28, 2025
@ZoltanFridrich ZoltanFridrich merged commit 1bbd6db into p11-glue:master Jun 11, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ueno ueno ueno approved these changes

@kalvdans kalvdans kalvdans approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Doekin @coveralls @ueno @kalvdans @ZoltanFridrich
0