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

Fix some build issues with clang in Mingw-w64 #18879

Closed
wants to merge 5 commits into from
Closed

Fix some build issues with clang in Mingw-w64 #18879

wants to merge 5 commits into from

Conversation

Biswa96
Copy link
Contributor

@Biswa96 Biswa96 commented Jun 11, 2021

No description provided.

@jkeenan
Copy link
Contributor

jkeenan commented Jun 11, 2021

Thank you for this pull request. I suspect that this p.r. may be attempting to do too much and that p.r.s addressing separate problems may be better.

Specifically, until we can see the build-time warnings you are getting when compiling with clang on Mingw-w64, we can't evaluate your proposed changes to perl.h. Because perl.h is very fundamental to compilation, we have to be very careful before changing it.

We keep track of build-time warnings with a GitHub "label" by that name. Can you please open a separate Issue for the build-time warnings, pasting in the relevant parts of the make (or Windows equivalent) output and the relevant configuration options?

Thank you very much.
Jim Keenan

@Biswa96
Copy link
Contributor Author

Biswa96 commented Jun 11, 2021

I think you've already opened the issue here #18780

@jkeenan
Copy link
Contributor

jkeenan commented Jul 12, 2021

I think you've already opened the issue here #18780

As noted in #18780 (comment) today, I tried out your pull request and found that it eliminated the vast majority of build-time warnings discussed in #18780. The warnings it does not eliminate appear to come from files under dist/ rather than from perl.h.

I'm not in a position to evaluate the mingw-related parts of this pull request. Nor can I say whether this is the best way to eliminate approximately 28,000 build-time warnings; only that is a way to eliminate such warnings. I don't have access to more recent versions of clang (clang13, I think, is relevant).

So a qualified +1 to this p.r.

@jkeenan
Copy link
Contributor

jkeenan commented Jul 13, 2021

@Biswa96, can you confirm that the changes in this commit are consistent with your proposed changes to perl.h and help to eliminate build-time warnings when compiling with clang12?

Reference: #18780

Thank you very much.
Jim Keenan

@jkeenan jkeenan requested a review from xenu July 14, 2021 12:14
@jkeenan
Copy link
Contributor

jkeenan commented Jul 14, 2021

I think you've already opened the issue here #18780

As noted in #18780 (comment) today, I tried out your pull request and found that it eliminated the vast majority of build-time warnings discussed in #18780. The warnings it does not eliminate appear to come from files under dist/ rather than from perl.h.

I'm not in a position to evaluate the mingw-related parts of this pull request. Nor can I say whether this is the best way to eliminate approximately 28,000 build-time warnings; only that is a way to eliminate such warnings. I don't have access to more recent versions of clang (clang13, I think, is relevant).

So a qualified +1 to this p.r.

@xenu, are you in a position to review the mingw-related parts of this p.r.?

Thank you very much.
Jim Keenan

@jkeenan
Copy link
Contributor

jkeenan commented Jul 15, 2021

@Biswa96,

Thank you for providing this pull request, which helped focus discussion of the problem of build-time warnings with clang12.

In #18984, @tonycoz has provided a more focused version of your patch, which we have applied to blead.

That leaves the 3 mingw-focused commits which were part of your p.r. If you would like to pursue those further, please open a new p.r. with just mingw-focused commits.

Closing this pull request. Thank you very much.

@jkeenan jkeenan closed this Jul 15, 2021
@Biswa96 Biswa96 deleted the fix-clang-mingw branch July 16, 2021 14:16
jeremyd2019 added a commit to jeremyd2019/MINGW-packages that referenced this pull request Jul 20, 2021
utilize @Biswa96's patches from Perl/perl5#18879

fix ExtUtils-CBuilder for clang, by updating dll creation to this
century.

For now, punt on linker script and just disable it.  This may be an
issue if some perl module has a ton of input files/libraries.

copy clang runtime dlls in addition to gcc's during test, required for
taint test, as comment above test-prep-gcc rule indicates.
jeremyd2019 added a commit to jeremyd2019/MINGW-packages that referenced this pull request Jul 20, 2021
utilize @Biswa96's patches from Perl/perl5#18879

fix ExtUtils-CBuilder for clang, by updating dll creation to this
century.

For now, punt on linker script and just disable it.  This may be an
issue if some perl module has a ton of input files/libraries.

copy clang runtime dlls in addition to gcc's during test, required for
taint test, as comment above test-prep-gcc rule indicates.
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.

2 participants