search

Perl / metaconfig

Perl 5 Metaconfig
15 stars 19 forks source link

wcrtomb probe added, perl changes need to be done; need advice and README changes #52

Open khwilliamson opened 6 years ago

khwilliamson commented 6 years ago

Again, I've apparently pushed something directly without doing a PR. I followed the instructions. I think I need to create a branch and then push that. But I haven't changed my workspace since the last time when it did work.

Anyway, the perl changes are at https://perl5.git.perl.org/perl.git/shortlog/refs/heads/wcrtomb

Tux commented 6 years ago

For commits like this I do not really care. Sane and simple!

BUT ...

  1. There is a format error in the ?S: section (spaces instead of tabs). I'll commit the fix.
  2. Why did you choose the threads/ folder? Reading the probe does not hint me to it being thread related
  3. Copyright 2017? Shouldn't that be 2018?
khwilliamson commented 6 years ago

On 01/12/2018 07:58 AM, H.Merijn Brand wrote:

For commits like this I do not really care. Sane and simple!

BUT ...

  1. There is a format error in the ?S: section (spaces instead of tabs). I'll commit the fix.

I ran the tools. Did they show an error for you?

  1. Why did you choose the threads/ folder? Reading the probe does not hint me to it being thread related

No one needs reentrancy unless it is a threaded-build.

  1. Copyright 2017? Shouldn't that be 2018?

Indeed.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/perl5-metaconfig/metaconfig/issues/52#issuecomment-357260151, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGhn9Ioc1p31b8VbgF1tHZm3DdgMsAqks5tJ3MmgaJpZM4Rb2pR.

Tux commented 6 years ago

• I saw no errors, but I remember from earlier days they will appear. Not sure if it was in ?S: or another section. Anyway, there should be a test for that, if not for any error, then at least for consistency • Ok, clear • Feel free to change

khwilliamson commented 6 years ago

On 01/12/2018 07:58 AM, H.Merijn Brand wrote:

For commits like this I do not really care. Sane and simple!

BUT ...

  1. There is a format error in the ?S: section (spaces instead of tabs). I'll commit the fix.

I copied that .U, and just changed some words around. My guess is that my editor expanded the tabs, so we do need a test to prevent this. I know you're busy, but I don't know what it should do, should it be combined with an existing test, etc. If you can lay out some quick requirements, I could implement it.

And I need to know if Win32 has this function implemented: configthreadlocale().

According to MS docs, it came along in VC8. According to our docs, we support back to VC6. That means I have to #ifdef code. I don't know if a Configure probe should be written, or if the code should somehow look at the VC version. What do you suggest, and can you point me to a template as to how to do it?

My probe for thread-safe nl_langinfo_l is defective because some platforms require xlocale.h to be included, and some don't even have that hdr. It's not clear to me how to write a probe that handles this. Again, I bet there is a template probe that does something similar, and that you can quickly point me to one.

Tux commented 6 years ago

I already have that check. I'm currently waiting for an answer from Rafael on what I propose