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.

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

Fix #8920 - m4/1.4.19: Runtime assertion windows pops up on MSVC in Debug mode #8984

Merged
merged 7 commits into from
Mar 7, 2022
Merged

Fix #8920 - m4/1.4.19: Runtime assertion windows pops up on MSVC in Debug mode #8984

merged 7 commits into from
Mar 7, 2022

Conversation

jmarrec
Copy link
Contributor

@jmarrec jmarrec commented Jan 19, 2022

Specify library name and version: m4/1.4.19

Fix #8920 - m4/1.4.19: Runtime assertion windows pops up on MSVC in Debug mode by define HAVE_MSVC_INVALID_PARAMETER_HANDLER

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@prince-chrismc prince-chrismc mentioned this pull request Jan 19, 2022
Open
@jmarrec jmarrec mentioned this pull request Jan 19, 2022
Closed
@conan-center-bot

This comment has been minimized.

Sign in to view
SSE4
SSE4 previously approved these changes Jan 21, 2022
View reviewed changes
SSE4
SSE4 reviewed Jan 21, 2022
View reviewed changes
recipes/m4/all/conanfile.py
@@ -62,6 +62,9 @@ def _configure_autotools(self):
elif self.settings.compiler == "clang":
if tools.Version(self.version) < "1.4.19":
autotools.flags.extend(["-rtlib=compiler-rt", "-Wno-unused-command-line-argument"])
if self.settings.os == 'Windows':
Copy link
Contributor

Choose a reason for hiding this comment

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

comparing compiler might be a better condition here, but that's fine as well

@madebr
Copy link
Contributor

madebr commented Jan 21, 2022 •
edited

I just built with Visual Studio 2019 in Debug mode, but the assertion is still there.
image

@jmarrec
Copy link
Contributor Author

jmarrec commented Jan 24, 2022 •
edited

@madebr Can you post your settings/profile (especially compiler.runtime I suppose) here so I can try to reproduce please? I've tested this

$ conan create . m4/1.4.19@ -b m4 -b missing -s build_type=Debug
[settings]
arch=x86_64
arch_build=x86_64
build_type=Debug
compiler=Visual Studio
compiler.runtime=MDd
compiler.version=17
os=Windows
os_build=Windows
[options]
[build_requires]
[env]

Edit: ANNNNND I'm seeing the assertion. My bad!

@jmarrec
Copy link
Contributor Author

jmarrec commented Jan 24, 2022

I have no clue why it was working on my other machine the other day. Clearly this isn't working.

I've tweaked by patch-approach now, I was using the wrong macro. That seemed to work locally, so I pulled it here. Could you give it another test @madebr please?

I also wrote to the bug-m4 mailing list (hopefully clearly enough...)

@jmarrec
Copy link
Contributor Author

jmarrec commented Jan 26, 2022

My other machine has in profiles/default the compiler.runtime=MD and not MDd, and that's why it was working I suppose.

@madebr
Copy link
Contributor

madebr commented Jan 26, 2022

It works now! Great job!

It's very weird though how a windows warning dialog sound is player every time after the test package has finished executing.

@jmarrec
Copy link
Contributor Author

jmarrec commented Jan 26, 2022

@madebr see the thread on the bug-m4 mailing list I have going: https://lists.gnu.org/archive/html/bug-m4/2022-01/msg00000.html

@madebr
Copy link
Contributor

madebr commented Jan 26, 2022

Thanks for the link. And +1 for the bug report, it looks like somebody is looking into it.

madebr
madebr previously approved these changes Jan 26, 2022
View reviewed changes
@madebr
Copy link
Contributor

madebr commented Jan 26, 2022

@jmarrec
Looks like they posted a patch: https://lists.gnu.org/archive/html/bug-m4/2022-01/txtSzXResQgKX.txt
It requires running gnulib.
To avoid that (since it takes a long of time for it to finish), the gnulib script can be run on your system and a patch created with the only the fclose diff (or at least reduced to the minimum).

@SSE4
Copy link
Contributor

SSE4 commented Jan 27, 2022

it would be nice if someone can test this patch:

 	xstrtoimax, xstrtoumax: depend on inttypes-incomplete
diff --git a/modules/close-stream b/modules/close-stream
index 78ed207992..4fce2ea233 100644
--- a/modules/close-stream
+++ b/modules/close-stream
@@ -6,6 +6,7 @@ lib/close-stream.h
 lib/close-stream.c
 
 Depends-on:
+fclose
 fpending
 stdbool

I'd prefer to have this one, so we diverge less from the upstream and don't need to constantly rebase custom patches

prince-chrismc
prince-chrismc previously approved these changes Feb 2, 2022
View reviewed changes
@jmarrec
Copy link
Contributor Author

jmarrec commented Feb 9, 2022

@SSE4 Based on the latest conversations on the mailing list, it seems the patch will have no effect. What they recommend is to (force) build with /MD instead of /MDd.

SSE4
SSE4 previously approved these changes Feb 9, 2022
View reviewed changes
@conan-center-bot

This comment has been minimized.

Sign in to view
@SSE4 SSE4 dismissed stale reviews from prince-chrismc, madebr, and themself via 4b39370 February 9, 2022 17:55
@conan-center-bot

This comment has been minimized.

Sign in to view
@jmarrec
Copy link
Contributor Author

jmarrec commented Feb 11, 2022

I don't know what's going on with the help2man on gcc 5...

This was referenced Feb 25, 2022
Closed
Closed
@ericLemanissierBot
Copy link

I detected other pull requests that are modifying m4/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@SSE4
Copy link
Contributor

SSE4 commented Mar 5, 2022

m4.1 (man page) depends on m4.c on the Makefile (which is modified by the patch).

@SSE4
- workaround make calling help2man (don't care, as we are not interes…
7018aba 
…ted in man pages)

Signed-off-by: SSE4 <tomskside@gmail.com>
@conan-center-bot

This comment has been minimized.

Sign in to view
@SSE4
- fix MSVC
5c694ed 
Signed-off-by: SSE4 <tomskside@gmail.com>
@conan-center-bot

This comment has been minimized.

Sign in to view
@SSE4 SSE4 closed this Mar 5, 2022
@SSE4 SSE4 reopened this Mar 5, 2022
@conan-center-bot
Copy link
Collaborator

All green in build 7 (5c694ed6ab3d51dc47046fe6bf66adcef607ef54):

  • m4/1.4.19@:
    All packages built successfully! (All logs)

  • m4/1.4.18@:
    All packages built successfully! (All logs)

@SSE4 SSE4 requested a review from uilianries March 6, 2022 06:11
jmarrec
jmarrec commented Mar 7, 2022
View reviewed changes
recipes/m4/all/conanfile.py
Comment on lines +95 to +98
with tools.chdir(self._source_subfolder):
tools.save("help2man", '#!/usr/bin/env bash\n:')
if os.name == 'posix':
os.chmod("help2man", os.stat("help2man").st_mode | 0o111)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clever! thanks for the workaround @SSE4 !

uilianries
uilianries approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

@conan-center-bot conan-center-bot merged commit b00bb73 into conan-io:master Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SSE4 SSE4 SSE4 approved these changes

@madebr madebr madebr approved these changes

@uilianries uilianries uilianries approved these changes

@prince-chrismc prince-chrismc prince-chrismc left review comments

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

Successfully merging this pull request may close these issues.

[package] m4/1.4.19: Runtime assertion windows pops up on MSVC in Debug mode
7 participants
@jmarrec @conan-center-bot @madebr @SSE4 @ericLemanissierBot @uilianries @prince-chrismc