-
Notifications
You must be signed in to change notification settings - Fork 396
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
Explicit header definitions for Open XL warnings #7321
base: master
Are you sure you want to change the base?
Conversation
dd43d97
to
4f257e0
Compare
4f257e0
to
2e98c96
Compare
6b4576b
to
660db3c
Compare
bb2756c
to
47b5f0f
Compare
jenkins build all |
1 similar comment
jenkins build all |
@Deigue Please investigate the failures in the PR build jobs. I looked at the zOS PR build: https://ci.eclipse.org/omr/job/PullRequest-zos_390-64/4399/console Here are the errors:
|
Sounds good, I can fix some of those. Can I also type the "build all" command to trigger jenkins to start a build to verify changes made after I push a commit against the branch? |
No. For access, you can try requesting through @0xdaryl (project lead) and @AdamBrousseau (devops). You can also message me on Slack; I can launch the builds for you.
One of the changes might have implicitly triggered the above error. Fixing other errors might automatically fix it. If it persists, the following thread might help resolve the above error: |
47b5f0f
to
c1c2105
Compare
jenkins build all |
I see this in the latest jenkin build log Does something still look syntactically wrong? Because I used the method signatures from official documentation and verified against headers for the params so not sure what it is going forward... |
The errors are slightly different in the OSX builds:
|
Only the AIX and zOS builds have the following error:
This looks like a side-effect of the |
On Windows, there is a test failure:
|
Just a quick update re some internal discussions on the right way to fix this problem. On further digging, seems like
After looking at the
Currently discussing a few things with compiler team before finalizing changes..
I think I can make the tweaks once I get a better understanding on the above. |
Are these changes dependent on the make file changes in #7319? |
c1c2105
to
102c197
Compare
Shouldn't be, but Im double checking #7319 changes with Jenkins builds, and will update/comment there once it looks good. Then we can probably merge both at same time. I have a rework for this (which removes those declaration changes from EDIT (2024-06-11): Also to mention I also compiled with java 21 and compiles fine with Open J9 without issues, will need to retest after I pull in the new changes. |
aea165c
to
7986c53
Compare
Changes:
currently working on building xlc jenkins to see if it is all good. |
7986c53
to
9790452
Compare
Unfortunately seeing some errors with XLC z/OS build (j21 extension repo)
as part of jenkins build with Trying to figure out what the problem is that this commit is introducing, sharing here incase someone can add some insight and provide feedback as to where the issue is stemming from. |
jenkins build all |
jenkins build zos(compile:'VERBOSE=1') |
I was able to reproduce this using cd /jit/team/gauravc/repos/omr/build/port && /bin/c89 -D__STDC_LIMIT_MACROS -D_ALL_SOURCE -D_OPEN_THREADS=3 -D_XOPEN_SOURCE=600 -DJ9ZOS390 -DJ9ZOS39064 -DLONGLONG -DOMR_EBCDIC -DOMRPORT_LIBRARY_DEFINE -DSUPPORTS_THREAD_LOCAL -DZOS -I/jit/team/gauravc/repos/omr/port -I/jit/team/gauravc/repos/omr/build/port -I/jit/team/gauravc/repos/omr/include_core -I/jit/team/gauravc/repos/omr/build -I/jit/team/gauravc/repos/omr/port/zos390 -I/jit/team/gauravc/repos/omr/port/unix -I/jit/team/gauravc/repos/omr/port/unix_include -I/jit/team/gauravc/repos/omr/port/common -I/jit/team/gauravc/repos/omr/port/include -I/jit/team/gauravc/repos/omr/port/../nls "-Wc,xplink" "-Wc,rostring" "-Wc,FLOAT(IEEE,FOLD,AFP)" "-Wc,enum(4)" "-Wa,goff" "-Wc,NOANSIALIAS" "-Wc,TARGET(ZOSV2R3)" -Wc,lp64 "-Wa,SYSPARM(BIT64)" "-Wc,ARCH(10)" "-Wc,TUNE(12)" "-Wl,compat=ZOSV2R3" "-Wc,langlvl(extc99)" -Wc,DLL,EXPORTALL -o omrosdump.c.o -c /jit/team/gauravc/repos/omr/port/zos390/omrosdump.c I noticed I didn't run into this because my local build was using |
1de98e1
to
5302e0b
Compare
Good to run this again. Changes have been added for |
jenkins build zos(compile:'VERBOSE=1') |
0532900
to
075c6f1
Compare
|
Open XL 2.1 on z/OS reports some of the implicit declarations as errors, and requires these to be explicitly defined. Most of the underlying changes are to address this concern and some other fixes pertaining to compilation errors. zos.cmake now uses XOPEN_SOURCE=600, which implies/includes already namespaces from POSIX_SOURCE, XOPEN_SOURCE_THREADED, ISOC99_SOURCE. See feature test macros: https://www.ibm.com/docs/en/zos/3.1.0?topic=files-feature-test-macros timer.h - unneeded conditional removed Array.hpp - uninitialized variable addressed Signed-off-by: Gaurav Chaudhari <gaurav.chaudhari@ibm.com>
075c6f1
to
47e80bf
Compare
jenkins build all |
Restarting the z/OS build since the connection with the machine was lost in the previous build: https://ci.eclipse.org/omr/job/PullRequest-zos_390-64/4488/console. jenkins build zos |
@@ -81,7 +81,7 @@ class AIXTimer { | |||
}; | |||
|
|||
typedef AIXTimer PlatformTimer; | |||
#elif defined(LINUX) || defined(OSX) || (defined (__MVS__) && defined (_XOPEN_SOURCE_EXTENDED)) | |||
#elif defined(LINUX) || defined(OSX) || defined (__MVS__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No space before (
in defined (__MVS__)
, please.
@@ -321,7 +326,7 @@ tdump_wrapper(struct OMRPortLibrary *portLibrary, char *filename, char *dsnName) | |||
static intptr_t | |||
tdump(struct OMRPortLibrary *portLibrary, char *asciiLabel, char *ebcdicLabel, uint32_t *returnCode, uint32_t *reasonCode) | |||
{ | |||
struct ioparms_t { | |||
struct ioparmss_t { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to add an s
here (that's not present on line 342)?
-D_ALL_SOURCE | ||
-D_XOPEN_SOURCE=600 | ||
-D_OPEN_THREADS=3 | ||
-D_POSIX_SOURCE | ||
-D_XOPEN_SOURCE_EXTENDED | ||
-D_ISOC99_SOURCE | ||
-D__STDC_LIMIT_MACROS | ||
-DLONGLONG | ||
-DJ9ZOS390 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you're here, could you sort these by name?
-D_ALL_SOURCE
-DJ9ZOS390
-DLONGLONG
-D_OPEN_THREADS=3
-D__STDC_LIMIT_MACROS
-DSUPPORTS_THREAD_LOCAL
-D_XOPEN_SOURCE=600
-DZOS
Open XL 2.1 on z/OS reports some of the implicit header declarations as errors and requires these to be explicitly defined. Most of the underlying changes are to address this concern and some other fixes pertaining to compilation errors.
(This is one part of the multiple changes added for supporting Open XL compilation on OMR)