Lua: Add versions and minor clean up by white238 · Pull Request #33037 · spack/spack

white238 · 2022-10-05T23:17:30Z

Added some new versions and removed some hard-coded versions and a standard flag. I also split the build phase out of the install phase.

spackbot-app · 2022-10-05T23:17:37Z

Hi @white238! I noticed that the following package(s) don't yet have maintainers:

lua

Are you interested in adopting any of these package(s)? If so, simply add the following to the package class:

    maintainers = ['white238']

If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with spack blame:

$ spack blame lua

Thank you for your help! Please don't add maintainers without their consent.

You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer.

tldahlgren

I could build 5.2.0 with the old recipe (spack install [email protected] +pcfile) but cannot with this one:

==> lua: Executing phase: 'edit'
==> lua: Executing phase: 'build'
==> Error: ProcessError: Command exited with status 2:
    'make' '-j16' 'MYLDFLAGS=-L/usr/WS1/dahlgren/spack/opt/spack/linux-rhel7-broadwell/gcc-8.3.1/readline-8.1-v63wys4g2e3l6ysgsp6unp77b7pssgcm/lib -L/usr/lib64' 'MYLIBS=-lncurses -ltinfo -lncursesw' 'CC=/usr/WS1/dahlgren/spack/lib/spack/env/gcc/gcc -std=c99 -fPIC' 'linux'

6 errors found in build log:
     63        p->f = lua_popen(L, filename, mode);
     64               ^~~~~~~~~
     65     liolib.c:241:8: warning: assignment to 'FILE *' {aka 'struct _IO_FI
            LE *'} from 'int' makes pointer from integer without a cast [-Wint-
            conversion]
     66        p->f = lua_popen(L, filename, mode);
     67             ^
     68     liolib.c: In function 'f_seek':
  >> 69     liolib.c:79:20: error: unknown type name 'off_t'
     70      #define l_seeknum  off_t
     71                         ^~~~~
     72     liolib.c:543:3: note: in expansion of macro 'l_seeknum'
     73        l_seeknum offset = (l_seeknum)p3;
     74        ^~~~~~~~~
  >> 75     liolib.c:79:20: error: 'off_t' undeclared (first use in this functi
            on); did you mean 'offset'?
     76      #define l_seeknum  off_t
     77                         ^~~~~
     78     liolib.c:543:23: note: in expansion of macro 'l_seeknum'
     79        l_seeknum offset = (l_seeknum)p3;
     80                            ^~~~~~~~~
     81     liolib.c:79:20: note: each undeclared identifier is reported only o
            nce for each function it appears in
     82      #define l_seeknum  off_t
     83                         ^~~~~
     84     liolib.c:543:23: note: in expansion of macro 'l_seeknum'
     85        l_seeknum offset = (l_seeknum)p3;
     86                            ^~~~~~~~~
  >> 87     liolib.c:543:33: error: expected ',' or ';' before 'p3'
     88        l_seeknum offset = (l_seeknum)p3;
     89                                      ^~
     90     liolib.c:77:25: warning: implicit declaration of function 'fseeko';
             did you mean 'fseek'? [-Wimplicit-function-declaration]
     91      #define l_fseek(f,o,w)  fseeko(f,o,w)
     92                              ^~~~~~
     93     liolib.c:546:8: note: in expansion of macro 'l_fseek'

     ...

     96     liolib.c:78:21: warning: implicit declaration of function 'ftello';
             did you mean 'ftell'? [-Wimplicit-function-declaration]
     97      #define l_ftell(f)  ftello(f)
     98                          ^~~~~~
     99     liolib.c:550:35: note: in expansion of macro 'l_ftell'
     100         lua_pushnumber(L, (lua_Number)l_ftell(f));
     101                                       ^~~~~~~
  >> 102    make[2]: *** [liolib.o] Error 1
     103    make[2]: *** Waiting for unfinished jobs....
     104    loslib.c: In function 'os_tmpname':
     105    loslib.c:49:13: warning: implicit declaration of function 'mkstemp'
            ; did you mean 'mktime'? [-Wimplicit-function-declaration]
     106             e = mkstemp(b); \
     107                 ^~~~~~~
     108    loslib.c:107:3: note: in expansion of macro 'lua_tmpnam'

     ...

     125         stm = l_localtime(&t, &tmr);
     126               ^~~~~~~~~~~
     127    loslib.c:203:9: warning: assignment to 'struct tm *' from 'int' mak
            es pointer from integer without a cast [-Wint-conversion]
     128         stm = l_localtime(&t, &tmr);
     129             ^
     130    make[2]: Leaving directory `/tmp/dahlgren/spack-stage/spack-stage-l
            ua-5.2.0-ozfywateqndop2heqilk2az5pnzx57py/spack-src/src'
  >> 131    make[1]: *** [linux] Error 2
     132    make[1]: Leaving directory `/tmp/dahlgren/spack-stage/spack-stage-l
            ua-5.2.0-ozfywateqndop2heqilk2az5pnzx57py/spack-src/src'
  >> 133    make: *** [linux] Error 2

var/spack/repos/builtin/packages/lua/package.py

tldahlgren · 2022-10-06T01:27:28Z

var/spack/repos/builtin/packages/lua/package.py

+    def build(self, spec, prefix):
        if spec.satisfies("platform=darwin"):
            target = "macosx"
        else:
            target = "linux"
        make(
-            "INSTALL_TOP=%s" % prefix,
            "MYLDFLAGS="
            + " ".join((spec["readline"].libs.search_flags, spec["ncurses"].libs.search_flags)),
            "MYLIBS=%s" % spec["ncurses"].libs.link_flags,
-            "CC=%s -std=gnu99 %s" % (spack_cc, self.compiler.cc_pic_flag),
+            "CC={0} {1} {2}".format(spack_cc, self.compiler.c99_flag, self.compiler.cc_pic_flag),
            target,
        )
+
+    def install(self, spec, prefix):


I can build 5.2.0 if I "undo" these changes.

I was on quartz BTW.

I personally don't care about this change. I was trying to be less hard-coded but there is no gnu99 flag function on though the xl compiler does toggle between -std=gnu99 and -qlanglvl=extc99 in the c99_flag property.

Would you prefer I toggle between the hard-coded vs this property based on the Lua version? I am willing to test until it fails and figure out the cut-off. Or move back to a hard-coded -std=gnu99?

I don't like hard-coded flags either and was curious to see if this worked. Perhaps @trws has a preference?

I'm not sure what c99_flag is normally populated with, but lua does require some gnu extensions so going to c99 is likely to cause problems. If we had an option for enabling the extended mode that's what we would want. If gnu99 works and we don't have a fail case for it, I'd leave it hardcoded since that's a surprisingly portable option (except windows), but it would be nice to clean up if the support stuff were there.

Alternatively, I could add a gnu99_flag to the compilers make sense.

Another idea... leave hard-coded -std=gnu99 for previous versions and i could see if c11_flag would work on the newer ones.

If that works it would be nice, I have a feeling they'll want gnu11 though.

I decided this would be a scope creep of this PR and is going to be a rather large PR/effort alone. I reverted back to the original hardcoded standard flag and will create an issue.

…standard flag

…ersions above 5.4

trws · 2022-10-12T22:15:57Z

Thanks @white238!

tldahlgren

Confirmed I can build 5.2.0 with this version.

spackbot-app bot added new-version update-package virtual-dependencies labels Oct 5, 2022

white238 requested review from tldahlgren and trws October 5, 2022 23:17

tldahlgren reviewed Oct 6, 2022

View reviewed changes

var/spack/repos/builtin/packages/lua/package.py Show resolved Hide resolved

tldahlgren reviewed Oct 6, 2022

View reviewed changes

tldahlgren assigned tldahlgren and trws Oct 6, 2022

white238 added 3 commits October 12, 2022 14:53

add new lua releases

ec48d66

split install phase and move it into a build phase, remove hardcoded …

70f5ef5

…standard flag

revert back to the original hardcoded std flag, guard patch against v…

3121f5d

…ersions above 5.4

white238 force-pushed the white238/lua_versions branch from 927b194 to 3121f5d Compare October 12, 2022 21:53

trws approved these changes Oct 12, 2022

View reviewed changes

trws enabled auto-merge (squash) October 12, 2022 22:15

tldahlgren approved these changes Oct 12, 2022

View reviewed changes

white238 mentioned this pull request Oct 12, 2022

Compiler classes are missing standard flags #33272

Open

2 tasks

trws merged commit 5167eed into develop Oct 13, 2022

trws deleted the white238/lua_versions branch October 13, 2022 17:51

Conversation

white238 commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spackbot-app bot commented Oct 5, 2022

Uh oh!

tldahlgren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trws commented Oct 12, 2022

Uh oh!

tldahlgren left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

white238 commented Oct 5, 2022 •

edited

Loading