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 to solaris build problem re quiet_nan #135

Merged
merged 2 commits into from Aug 11, 2015
Merged

Conversation

devel-chm
Copy link
Member

It appears that the logic is already in the code,
just that the NANARG determined in mconf.h was not
used to declare the local quiet_nan version.

Works for me, seems simple. Ok with you?

@mohawk2
Copy link
Member

mohawk2 commented Aug 9, 2015

Looks good to me, so if it works on Solaris, merge away! (and release)

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

With this change I get:

/opt/solarisstudio12.3/bin/cc -c  "-I/export/home/vagrant/.cpanm/work/1439227413.3233/PDL-2.012_01/Basic/Core"  -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DPERL_USE_SAFE_PUTENV -O    -DVERSION=\"2.012_01\"  -DXS_VERSION=\"2.012_01\" -KPIC "-I/export/home/vagrant/perl5/perlbrew/perls/perl-5.22.0/lib/5.22.0/i86pc-solaris/CORE"  -DMY_ERFI -DMY_INFINITY -DMY_NAN -DMY_NDTRI -DMY_POLYROOTS quiet_nan.c
"quiet_nan.c", line 3: syntax error before or at: 1L
"quiet_nan.c", line 3: warning: parameter mismatch: 1 declared, 0 defined
cc: acomp failed for quiet_nan.c
*** Error code 2
make: Fatal error: Command failed for target `quiet_nan.o'
Current working directory /export/home/vagrant/.cpanm/work/1439227413.3233/PDL-2.012_01/Basic/Math

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

I'll continue to look into it.

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

I made a change to how the function signature is defined here. With that change everything continues compiling until:

cp pdldoc blib/script/pdldoc
"/export/home/vagrant/perl5/perlbrew/perls/perl-5.22.0/bin/perl" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/pdldoc
cp perldl blib/script/perldl
"/export/home/vagrant/perl5/perlbrew/perls/perl-5.22.0/bin/perl" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/perldl
/opt/solarisstudio12.3/bin/cc  -o pdl
usage: cc [ options ] files.  Use 'cc -flags' for details
*** Error code 1
make: Fatal error: Command failed for target `pdl'
FAIL
! Installing ./PDL-2.012_01.tar.gz failed. See /export/home/vagrant/.cpanm/work/1439229062.7054/build.log for details. Retry with --force to force install it.

There need to be input files to cc, right?

@devel-chm
Copy link
Member Author

Try a completely clean build. See if that makes a difference...

On Mon, Aug 10, 2015 at 2:07 PM, Zaki Mughal [sivoais] <
notifications@github.com> wrote:

I made a change to how the function signature is defined. With that change
everything continues compiling until:

cp pdldoc blib/script/pdldoc
"/export/home/vagrant/perl5/perlbrew/perls/perl-5.22.0/bin/perl" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/pdldoc
cp perldl blib/script/perldl
"/export/home/vagrant/perl5/perlbrew/perls/perl-5.22.0/bin/perl" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/perldl
/opt/solarisstudio12.3/bin/cc -o pdl
usage: cc [ options ] files. Use 'cc -flags' for details
*** Error code 1
make: Fatal error: Command failed for target `pdl'
FAIL
! Installing ./PDL-2.012_01.tar.gz failed. See /export/home/vagrant/.cpanm/work/1439229062.7054/build.log for details. Retry with --force to force install it.

There need to be input files to cc, right?


Reply to this email directly or view it on GitHub
#135 (comment).

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

This is from a clean build. I use make dist and build the tarball (for some reason that's the only way it compiles).

If I type make pdl, it seems that the $< in Makefile is not expanded. Which is rather confusing.

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

Running make -dd pdl to get the debugging information gives the following state for the automatic variables:

@= pdl
* =
< =
% =
? = pdl.c
^ = pdl.c

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

So there are two possibilities:

I either change the toplevel Makefile (from Makefile.PL)

 pdl: pdl.c
-        $(CC) $< -o $@
+        $(CC) $^ -o $@

or run GNU make: /usr/sfw/bin/gmake pdl

Both solutions allow the build to complete and tests all pass.

@devel-chm
Copy link
Member Author

I vote for changing the $&lt; to $^ in the Makefile.PL line since that is our
code and not EUMM stuff.

--Chris

On Mon, Aug 10, 2015 at 3:01 PM, Zaki Mughal [sivoais] <
notifications@github.com> wrote:

So there are two possibilities:

I either change

pdl: pdl.c- $(CC) $&lt; -o $@+ $(CC) $^ -o $@

or run GNU make: /usr/sfw/bin/gmake pdl


Reply to this email directly or view it on GitHub
#135 (comment).

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

According to POSIX make:

$<

In an inference rule, the $&lt; macro shall evaluate to the filename whose existence allowed the inference rule to be chosen for the target. In the .DEFAULT rule, the $&lt; macro shall evaluate to the current target name. The meaning of the $< macro shall be otherwise unspecified.

For example, in the .c.a inference rule, $< represents the prerequisite .c file.

which means that in this case a POSIX-compliant make might not expand $<. So do we want a more portable Makefile or want to use a portable make (GNU make)?

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

OK, I agree with that @devel-chm. Changing it now.

@zmughal
Copy link
Member

zmughal commented Aug 10, 2015

Another way is to be explicit by using pdl.c in the command directly and this should work pretty much everywhere. We'll see if the CIs are happy and I believe it should be good. I'd like to squash the first two commits before I merge.

@devel-chm
Copy link
Member Author

I don't know if you seen the PDL bugs tracker for sf#390 but
Marek replied that he will try out the PDL-2.012_01 release
tomorrow.

--Chris

On Mon, Aug 10, 2015 at 4:02 PM, Zaki Mughal [sivoais] <
notifications@github.com> wrote:

Another way is to explicit, but this should work pretty much everywhere.
We'll see if the CIs are happy and I believe it should be good. I'd like to
squash the first two commits before I merge.


Reply to this email directly or view it on GitHub
#135 (comment).

zmughal and others added 2 commits August 11, 2015 09:12
This is more portable since some `make`s only expand `$<` in an
inference rule rather than an explicit command. See the definition in
POSIX <http://pubs.opengroup.org/onlinepubs/009695399/utilities/make.html>.

This came up with the make(1) in Solaris 11.2.
The quiet_nan() function on Solaris has the signature:

    double quiet_nan(long n);

and this adds a define to set the `long` parameter in the function
prototype and function definition.

It appears that the logic is already in the code,
just that the NANARG determined in mconf.h was not
used to declare the local quiet_nan version.
@zmughal
Copy link
Member

zmughal commented Aug 11, 2015

On Solaris 11.2 (uname -a "SunOS solaris 5.11 11.2 i86pc i386 i86pc"):

All tests successful.

Test Summary Report
-------------------
t/bad.t                       (Wstat: 0 Tests: 82 Failed: 0)
  TODO passed:   53-54
t/fits.t                      (Wstat: 0 Tests: 90 Failed: 0)
  TODO passed:   13, 17, 26, 30, 39, 45, 51, 60
t/inline-comment-test.t       (Wstat: 0 Tests: 3 Failed: 0)
  TODO passed:   3
t/iotypes.t                   (Wstat: 0 Tests: 8 Failed: 0)
  TODO passed:   1-8
t/minmax-behavior.t           (Wstat: 0 Tests: 3 Failed: 0)
  TODO passed:   1, 3
t/op-eq-warn-for-non-numeric.t (Wstat: 0 Tests: 5 Failed: 0)
  TODO passed:   5
t/ops.t                       (Wstat: 0 Tests: 53 Failed: 0)
  TODO passed:   48-50
t/pdl_from_string.t           (Wstat: 0 Tests: 113 Failed: 0)
  TODO passed:   37-39
t/ufunc.t                     (Wstat: 0 Tests: 35 Failed: 0)
  TODO passed:   14-35
Files=132, Tests=1480, 48 wallclock secs ( 0.76 usr  0.54 sys + 32.26 cusr  8.16 csys = 41.72 CPU)
Result: PASS
...
OK
Successfully installed PDL-2.012_01
1 distribution installed

All tests pass and once Travis-CI is happy, I'll merge this in.

@zmughal zmughal added this to the PDL v2.013 milestone Aug 11, 2015
@zmughal zmughal merged commit 6a320a2 into master Aug 11, 2015
@zmughal
Copy link
Member

zmughal commented Aug 12, 2015

@devel-chm, the changes have been merged and PDL v2.013 should be ready to go!

@zmughal zmughal deleted the solaris-quiet-nan-conflict branch August 12, 2015 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants