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

Update to support and logical_nparens on 5.37.7 and later #6

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

demerphq
Copy link
Contributor

This should fix this module under Perl 5.37.7 and later.

See Perl/perl5#20710

You may want to consider whether Dist::Zilla is appropriate for this module. It took me over an hour to get the Dist::Zilla deps installed, and 3 minutes to fix the bug. :-( And you seem to be missing Dist::Zilla related dependencies, i had to manually install some, and some of them dont install cleanly on blead perl (i need to investigate why Test::Vars breaks.)

I suggest you try building the latest blead perl and then trying to install the deps and build your module. It will show some issues... I had to manually install:

Pod::Weaver::PluginBundle::RJBS
Config::AutoConf

and i had to force install Test::Vars to get Dist::Zilla::Plugin::Test::UnusedVars to install.

Anyway, if you like Dist::Zilla then so be it, but it would be much friendlier to people like me if you included a Makefile.PL that can be used as a fallback.

This should fix this module under Perl 5.37.7 and later.

See Perl/perl5#20710
@jddurand
Copy link
Owner

Many thanks.

Yes, Dist::Zilla is quite overkill here, I have to fix the dependencies at least.

Will work on that asap.

@demerphq
Copy link
Contributor Author

FWIW I just filed Perl/perl5#20723 about Test::Vars

@demerphq
Copy link
Contributor Author

Oh, also my patch fixes the various sprintf warnings during compile. I think you might want to update ppport.h however.

@Grinnz
Copy link

Grinnz commented Jan 18, 2023

FWIW, my recommended contributor-friendly usage of dzil is to regenerate the Makefile.PL (and META.json) into the repository via the dzil regenerate command and on release, such as my https://metacpan.org/pod/Dist::Zilla::PluginBundle::Starter::Git bundle regenerate option provides.

@Grinnz
Copy link

Grinnz commented Jan 18, 2023

Without switching to Starter::Git, you would implement this using [CopyFilesFromRelease] and [Regenerate::AfterReleasers], plus excluding the copied files from your GatherDir plugin.

@jddurand
Copy link
Owner

FWIW, my recommended contributor-friendly usage of dzil is to regenerate the Makefile.PL (and META.json

Hmmm very interesting - thanks. I may apply that to my other packages e.g. MarpaX::ESLIF ;)

@jddurand jddurand merged commit 398f29b into jddurand:master Jan 19, 2023
@demerphq demerphq deleted the yves/fix_perl_20710 branch January 19, 2023 05:05
jddurand added a commit that referenced this pull request Jan 22, 2023
jddurand added a commit that referenced this pull request Jan 23, 2023
 [Jean-Damien Durand <jeandamiendurand@free.fr>]
 - workflows/dzil-build-and-test.yml: dev+
 - workflows/dzil-build-and-test.yml: Remove AUTHOR_TESTING and
   RELEASE_TESTING when testing the distribution
 - workflows/dzil-build-and-test.yml: Set PAUSE identity bis
 - workflows/dzil-build-and-test.yml: Set PAUSE identity
 - workflows/ci.yml -> workflows/dzil-build-and-test.yml
 - workflows/ci.yml: Trying to understand why dzil test --verbose does not
   work on windows workflow...
 - workflows/ci.yml: use perl -MDist::Zilla::App instead of dzil directly
   for windows workflow
 - engine/GNU.pm: Add a section HOW TO CONTRIBUTE
 - dist.ini: Move AUTHOR_TESTING=1 EXTENDED_TESTING=1 RELEASE_TESTING=1 to
   the run instead of the full workflow - this is causing
   List::SomeUtils::XS to fail because AUTHOR_TESTING sets -Werror
   compilation flag that makes clang-13 bail
 - dist.ini: Use cpan to install (and see why it fails on macos)
   List::SomeUtils::XS
 - dist.ini: Removed non-needed git identity, explicit install of
   Test::Perl::Critic (cpanm bug on MacOS ?)
 - Commit dzil regenerated files
 - dist.ini: Generated Changes from git
 - workflows/ci.yml: Run dzil test with --verbose option
 - workflows/ci.yml: Try to fix git identity
 - workflows/ci.yml: dev+
 - engine/GNU.pm: Dummy commit to get actions to run (!?)
 - workflows/ci.yml: Removed a comment
 - workflows/ci.yml: Fetch repo history (no that big fortunately)
 - workflows/ci.yml: Set git identity
 - workflows/ci.yml:  ~ issue
 - engine/GNU.pm: ## no critic when getting $version
 - workflows/ci.yml: ~/.pause file tentative fix
 - workflows/ci.yml: Create a dummy .pause file
 - re/engine/GNU.pm: Bootstrap even when VERSION is not defined
 - workflows/ci.yml: fix syntax
 - workflows/ci.yml: testing github workflow
 - dzil regenerate(d)
 - dist.ini: Add cpanfile
 - dist.ini: Add Changes to Git::Check.allow_dirty
 - Makefile.PL: copied from release
 - dist.ini: Revisit following comments in #6
 - t/re-engine-GNU.t: explicit show array and hash forms
 - weaver.ini
 - weaver.ini: Initial commit
@jddurand
Copy link
Owner

jddurand commented Jan 23, 2023

Without switching to Starter::Git, you would implement this using [CopyFilesFromRelease] and [Regenerate::AfterReleasers], plus excluding the copied files from your GatherDir plugin.

FYI I switched to Starter::Git, that is less heavy that my previous dependencies, adding of course my own things on top of it, that are namely:

  • I use "# VERSION" and "# AUTHORITY" annotations, which forces me to use a combination of [Git::NextVersion], [OurPkgVersion] and [Authority] that is doing very well the job
  • Still using PodWeaver from RJBS, because this the only plugin AFAIK that is able to put correctly a CONTRIBUTOR section into README.pod
  • quite a lot t/xt additional tests

For automatic handy files, I followed your recommendation by adding:

  • the famous Makefile.PL, I admit it is very handy
  • plus cpanfile, very very handy

CI has been added with github workflows, using a lot of perl versions, taking into account the perl minimum version of my package of course (which is an interesting exercise from github workflow point of view ;)), on ubuntu-latest, macos-latest and windows-latest.

I released the 0.026-TRIAL version for this purpose, worflow is nice as per https://github.com/jddurand/re-engine-GNU/actions/runs/3990448514 .

One thing remains open for me: the CI world seems to think gcc only very much, while IMHO it is should never forget cl on Windows at least. So I will add appveyor asap.

Thanks.

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