Skip to content

llvm-windres handling of unescaped quotes broke building xz #65122

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

Closed
mstorsjo opened this issue Aug 30, 2023 · 3 comments · Fixed by llvm/llvm-project-release-prs#681
Closed
Labels
llvm-tools All llvm tools that do not have corresponding tag regression release:backport release:merged

Comments

@mstorsjo
Copy link
Member

The recent change (in 17.x) for llvm-windres handling of unescaped quotes in 0f4c6b1 (which in itself fixed one aspect of #57334) instead broke use of llvm-windres in building xz (an llvm-windres specific workaround in tukaani-project/xz@6b117d3, which worked before no longer works with the current llvm-windres).

We shouldn't be trading one bug for another, as the change from 0f4c6b1 is correct in itself (making llvm-windres behaviour closer to GNU windres), we can implement the GNU windres flag --use-temp-file properly, which should allow the GNU windres codepath in xz to work for llvm-windres as well.

A "fix" for this issue is up at https://reviews.llvm.org/D159223 - but to properly take that fix into use, xz will need to remove its llvm-windres specific handling.

@mstorsjo mstorsjo added llvm-tools All llvm tools that do not have corresponding tag regression labels Aug 30, 2023
@mstorsjo mstorsjo added this to the LLVM 17.0.X Release milestone Aug 30, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Aug 30, 2023
@mstorsjo
Copy link
Member Author

mstorsjo commented Sep 1, 2023

/cherry-pick 2bcc0fd

The implementation of --use-temp-file in that commit should provide a way forward for xz to use llvm-windres with the same parameters as they use for GNU windres.

@llvmbot
Copy link
Member

llvmbot commented Sep 1, 2023

/branch llvm/llvm-project-release-prs/issue65122

@llvmbot
Copy link
Member

llvmbot commented Sep 1, 2023

/pull-request llvm/llvm-project-release-prs#681

@tru tru moved this from Needs Triage to Needs Review in LLVM Release Status Sep 2, 2023
@tru tru moved this from Needs Review to Done in LLVM Release Status Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm-tools All llvm tools that do not have corresponding tag regression release:backport release:merged
Projects
Development

Successfully merging a pull request may close this issue.

3 participants