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

BBC: Blead Breaks PDL #22354

Closed
cjg-cguevara opened this issue Jun 29, 2024 · 13 comments
Closed

BBC: Blead Breaks PDL #22354

cjg-cguevara opened this issue Jun 29, 2024 · 13 comments
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)

Comments

@cjg-cguevara
Copy link

This is a bug report for perl from "Carlos Guevara" carlos@carlosguevara.com,
generated with the help of perlbug 1.43 running under perl 5.41.1.


BBC: Blead Breaks PDL

Please see http://fast-matrix.cpantesters.org/?dist=PDL%202.089


Flags

  • category=core
  • severity=low

Perl configuration

Site configuration information for perl 5.41.1:

Configured by cpan at Fri Jun 28 23:41:57 EDT 2024.

Summary of my perl5 (revision 5 version 41 subversion 1) configuration:
  Commit id: 99e7291a160c07384076cfdcf5b3126143155452
  Platform:
    osname=linux
    osvers=5.14.0-427.22.1.el9_4.x86_64
    archname=x86_64-linux-thread-multi
    uname='linux cjg-rhel9 5.14.0-427.22.1.el9_4.x86_64 #1 smp preempt_dynamic mon jun 10 09:23:36 edt 2024 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Dprefix=/home/cpan/bin/perl -Dscriptdir=/home/cpan/bin/perl/bin -Dusedevel -Duse64bitall -Duseithreads'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O2'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='11.4.1 20231218 (Red Hat 11.4.1-3)'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib /usr/lib64 /usr/local/lib64
    libs=-lpthread -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/lib/../lib64/libc.so.6
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.34'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


---
@INC for perl 5.41.1:
    /home/cpan/bin/perl/lib/site_perl/5.41.1/x86_64-linux-thread-multi
    /home/cpan/bin/perl/lib/site_perl/5.41.1
    /home/cpan/bin/perl/lib/5.41.1/x86_64-linux-thread-multi
    /home/cpan/bin/perl/lib/5.41.1

---
Environment for perl 5.41.1:
    HOME=/home/cpan
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_ALL=C
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/cpan/bin/perl/bin:/home/cpan/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/bash
@jkeenan jkeenan added the BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) label Jun 29, 2024
@jkeenan
Copy link
Contributor

jkeenan commented Jun 29, 2024

Bisecting (which took exactly 1 hour on my fastest machine) with the following invocation:

$ perl Porting/bisect.pl --start=56d5b6ef820802cd4c57fc2cfd8301a3f5d7af04 --end=99e7291a160c07384076cfdcf5b3126143155452 --module=PDL

... pointed to 42cf752

42cf75238c3687c1b1fad9263614036f7059c2dc is the first bad commit
commit 42cf75238c3687c1b1fad9263614036f7059c2dc
Author: Leon Timmermans <fawaka@gmail.com>
Date:   Fri Jun 14 01:38:48 2024 +0200

    Simplify global typemap

@Leont, can you take a look? Thanks.

@Leont
Copy link
Contributor

Leont commented Jun 29, 2024

That is quite a surprise.

It seems PDL has its own typemap evaluator that will need to be adapted to this change.

@jkeenan
Copy link
Contributor

jkeenan commented Jun 30, 2024

That is quite a surprise.

It seems PDL has its own typemap evaluator that will need to be adapted to this change.

Notified upstream of problem: PDLPorters/pdl#487

@jkeenan
Copy link
Contributor

jkeenan commented Jun 30, 2024

That is quite a surprise.

It seems PDL has its own typemap evaluator that will need to be adapted to this change.

Fortunately, that code in PDL's Basic::Gen::PP was developed relatively recently ...

commit 3cfe4bafeee5fea3582d9d39b38984b805b37d60
Author:     Ed J <mohawk2@users.noreply.github.com>
AuthorDate: Fri Aug 5 03:25:45 2022 +0100
Commit:     Ed J <mohawk2@users.noreply.github.com>
CommitDate: Fri Aug 5 04:39:44 2022 +0100

... so hopefully @mohawk2 will be able to help you out on this.

@mohawk2
Copy link
Contributor

mohawk2 commented Jun 30, 2024

@Leont Can you guide me on what this change means for PDL? Are you asking me to change the code? Will it work on older EU:PXS? Whose life is better as a result of this "alignment"? When is the new EU:PXS going to be released on CPAN?

I added the "" into that evaluation function because it was necessary to be compatible with EU:PXS typemaps, not out of preference. I couldn't find a method/function I can call in EU:PXS to replace the PDL code. In case it's not obvious, I do not think that this incompatible change is something that PDL should have to adapt for. EDIT Also, even if I did change PDL, older versions of PDL would no longer work on Perl 5.42, which doesn't seem like a great situation to knowingly allow.

@mohawk2
Copy link
Contributor

mohawk2 commented Jul 1, 2024

@Leont To cut to the chase: please revert your change, and find another way if you must to achieve what you were trying to do. Thanks.

@Leont
Copy link
Contributor

Leont commented Jul 1, 2024

Are you asking me to change the code?

Absolutely. Even if this change is reverted you should change the code (but in a different way); it's broken for output maps because those are already using \a quotes, and hence there are output templates (even in the core set) that use double-quotes freely.

Will it work on older EU:PXS?

I'm not sure exactly what you're asking here, but yes. It would break any typemap abusing quotes to get out of the string, but there's only one typemap on all of CPAN that does such a thing and it's easily fixed.

Whose life is better as a result of this "alignment"?

People who actually have to write typemaps.

Previously, unescaped double quotes would break in highly confusing ways in input maps if you ever used them. So one would have to write stuff like:

if (SvIV($arg) < 0))
  croak(\"Something is wrong\");

It's one of those things that makes writing typemaps really confusing and counter-intuitive. After this change you can just do:

if (SvIV($arg) < 0))
  croak("Something is wrong");

It's extra confusing because output templates have always used the latter syntax.

When is the new EU:PXS going to be released on CPAN?

Not sure. Would you want me to do a trial release now?

@mohawk2
Copy link
Contributor

mohawk2 commented Jul 2, 2024

@Leont Are you happy to make the PR to PDL that is backwards-compatible with both currently-released and your updated version of EU:PXS? If there's backward compatibility, then EU:PXS's sometimes-rather-slow release cycle stops being a problem. I don't see that a trial release per se will help with this overall issue, but am open to being wrong.

@Leont
Copy link
Contributor

Leont commented Jul 7, 2024

Yeah I will do that.

@HaraldJoerg
Copy link
Contributor

I ran into this issue today and can confirm that with the fix @Leont suggested in PDLPorters/pdl#487 (comment), PDL tests pass with Perl 5.41.3, and also with my system Perl (5.34).
Shall I prepare a PR with that patch and/or run tests with some old Perl version? The change looks harmless enough.

@mohawk2
Copy link
Contributor

mohawk2 commented Aug 9, 2024

I ran into this issue today and can confirm that with the fix @Leont suggested in PDLPorters/pdl#487 (comment), PDL tests pass with Perl 5.41.3, and also with my system Perl (5.34). Shall I prepare a PR with that patch and/or run tests with some old Perl version? The change looks harmless enough.

If it works with existing Perls and existing EU:PXS (which the CI will show), yes, please do.

@mohawk2
Copy link
Contributor

mohawk2 commented Aug 31, 2024

PDL 2.090 has been released, with @HaraldJoerg's update within it. Please check this against the latest EU:PXS.

@jkeenan
Copy link
Contributor

jkeenan commented Aug 31, 2024

PDL 2.090 has been released, with @HaraldJoerg's update within it. Please check this against the latest EU:PXS.

PDL 2.090 is doing well on CPANtesters: http://fast-matrix.cpantesters.org/?dist=PDL. I believe we should be able to close this ticket after a couple more days of testing.

@jkeenan jkeenan added the Closable? We might be able to close this ticket, but we need to check with the reporter label Aug 31, 2024
@jkeenan jkeenan closed this as completed Sep 17, 2024
@jkeenan jkeenan removed the Closable? We might be able to close this ticket, but we need to check with the reporter label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)
Projects
None yet
Development

No branches or pull requests

5 participants