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

Close manifest after writing it so it can be removed later in code #3866

Merged
merged 2 commits into from
May 5, 2023

Conversation

roffe
Copy link
Contributor

@roffe roffe commented May 4, 2023

Fixes a bug where the fyne package command would fail on Windows with error:

failed to remove manifest: remove C:\Users\roffe\go\src\github.com\roffe\t7logger\t7logger.exe.manifest: The process cannot access the file because it is being used by another process.

@coveralls
Copy link

coveralls commented May 4, 2023

Coverage Status

Coverage: 61.982% (-0.002%) from 61.984% when pulling 461258a on roffe:patch-2 into bc2b858 on fyne-io:develop.

Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I left a comment in-line about better placement for the close.

cmd/fyne/internal/commands/package-windows.go Outdated Show resolved Hide resolved
Copy link
Member

@Jacalz Jacalz left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and congratulations on landing your first PR to Fyne :)

I wish that the static analysis would catch unclosed readers and writers. I suspect there might be more places where we are missing close calls.

@Jacalz Jacalz merged commit c3d9318 into fyne-io:develop May 5, 2023
11 checks passed
@roffe roffe deleted the patch-2 branch May 18, 2023 10:46
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.

None yet

3 participants