MDSplus / mdsplus

The MDSplus data management system
https://mdsplus.org/
Other
74 stars 44 forks source link

Building on conda forge OSX fails #2736

Open smithsp opened 7 months ago

smithsp commented 7 months ago

Affiliation GA

Version(s) Affected alpha-7.139.65

Platform Mac OSX

Describe the bug The full log of the attempted build is in https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=909581&view=logs&jobId=15f8dc28-9f6f-5e3a-5b3c-2b169071e5be&j=15f8dc28-9f6f-5e3a-5b3c-2b169071e5be&t=63fa899e-c160-5185-75c4-c5fd30c90374 The build is based on the PR for conda-forge at https://github.com/conda-forge/mdsplus-feedstock/pull/54/files The last several lines of the actual report is

                    ^
/Users/runner/miniforge3/conda-bld/mdsplus_1712259065520/work/_include/mdsmsg.h:62:5: note: expanded from macro '__MDSMSG'
    clock_gettime(CLOCK_REALTIME, &ts);            \
    ^
MdsEvents.c:123:3: error: use of undeclared identifier 'CLOCK_REALTIME'
/Users/runner/miniforge3/conda-bld/mdsplus_1712259065520/work/_include/_mdsshr.h:21:3: note: expanded from macro 'MDSSHR_LOAD_LIBROUTINE_LOCAL'
  MDSSHR_LOAD_LIBROUTINE(method, lib, method, on_error)
  ^
/Users/runner/miniforge3/conda-bld/mdsplus_1712259065520/work/_include/_mdsshr.h:15:7: note: expanded from macro 'MDSSHR_LOAD_LIBROUTINE'
      MDSERR("Failed to load " #lib "->" #method "()");           \
      ^
/Users/runner/miniforge3/conda-bld/mdsplus_1712259065520/work/_include/mdsmsg.h:96:21: note: expanded from macro 'MDSERR'
#define MDSERR(...) __MDSMSG(MSG_ERROR, __VA_ARGS__)
                    ^
/Users/runner/miniforge3/conda-bld/mdsplus_1712259065520/work/_include/mdsmsg.h:62:19: note: expanded from macro '__MDSMSG'
    clock_gettime(CLOCK_REALTIME, &ts);            \
                  ^
MdsEvents.c:124:18: warning: passing arguments to a function without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
  return MdsValue(id, exp, d1, d2, d3);
                 ^
MdsEvents.c:466:41: warning: passing arguments to a function without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
          ((void (*)())(*event->astadr))(event->astprm, 12, event->data);
                                        ^
MdsEvents.c:830:11: error: call to undeclared function 'clock_gettime'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
          clock_gettime(CLOCK_REALTIME, &abstime);
          ^
MdsEvents.c:830:25: error: use of undeclared identifier 'CLOCK_REALTIME'
          clock_gettime(CLOCK_REALTIME, &abstime);
                        ^
13 warnings and 16 errors generated.
make[1]: *** [<builtin>: MdsEvents.o] Error 1         

To Reproduce Steps to reproduce the behavior:

  1. Attempt to build on conda forge OSX machine

Expected behavior Able to build without problems

Additional context I think it is failing because _include/pthread_port.h is not being included in MdsEvent.c or one of its include files, as that is where clock_gettime is defined. If you have a suggested patch, I am happy to try it in the conda build system.

smithsp commented 7 months ago

Mention @wdeshazer @modestmc .

smithsp commented 7 months ago

This is lower priority than #2731

fsciortino commented 6 months ago

Jumping in to say that resolution of this PR is currently required to unblock several other projects relating to Aurora, which is connected to omfit_classes, which is connected to MDSplus. Efforts to bring this higher in priority are appreciated.

mwinkel-dev commented 6 months ago

Hi @fsciortino and @smithsp,

The fix for Issue #2731 (namely PR #2740) should be released in a few days. Whereupon this issue becomes the top priority.

NOTE: -- MDSplus for Mac presently only compiles on computers that use Intel CPUs. The port of MDSplus to the Apple Silicon computers is underway but has not yet been completed.

mwinkel-dev commented 6 months ago

Hi @smithsp,

I just noticed that this bug report states alpha_7.139.65. However, the GA branch for Atlas is based on alpha_7.139.59. Which version should be used for this Conda Forge fix? (Stated another way, how many versions of MDSplus will GA be using?)

smithsp commented 6 months ago

@mwinkel-dev The conda-forge builds are mainly used for a robust python package of MDSplus, including for OMFIT and toksearch. They are separate from our other atlas and omega builds. The fix for the conda-forge build can be built off of the usual alpha branch and merged back into the alpha branch. I think the fix should be as easy as putting a #include <_pthread_port> (or similar) in the appropriate file. I wondered if @WhoBrokeTheBuild also might chime in, and I know that there is a new build method to be released at some point that may resolve this type of issue.

WhoBrokeTheBuild commented 6 months ago

(You actually responded while we were typing this, which is pretty funny)

@santorofer and I attempted to replicate this in a local environment with miniforge3, the steps from your build.sh and versions matching (as well as we could manage). However, we weren't able to replicate it, and the build succeeded anyway. I've poured over the azure build log for differences, but wasn't able to find any, so I'm not sure how to replicate it to test a fix. That being said, I agree with you that including pthread_port.h should fix this issue, if you want to include that in a patch and let us know how it goes, we would definitely appreciate it. I don't think the new cmake build system will change this, it tries to replicate the compiler flags from the autotools build system as close as possible.

Any ideas you have to help us replicate it or do further testing are also welcome.

smithsp commented 6 months ago

@WhoBrokeTheBuild I agree that I can't reproduce the issue locally. I think it has to do with the Xcode developer tools available to the conda-forge build bots, which has something to do with redistribution rights. Do you think the pthread_port include belongs in _mdsshr.h or mdsmsg.h or MdsEvents.c?

WhoBrokeTheBuild commented 6 months ago

I would put it in mdsmsg.h, since it has the macro that calls clock_gettime, that should cover all the bases

mwinkel-dev commented 6 months ago

Hi @smithsp,

I'm curious whether including the pthread_port.h in the mdsmsg.h fixed the problem that GA encountered with the Conda Forge build of MDSplus for Intel MacOS.

https://github.com/conda-forge/mdsplus-feedstock/pull/54

smithsp commented 6 months ago

@mwinkel-dev I haven't had time yet to try. I have to inject it via a patch to the source, push it, then wait for the build, which is a little cumbersome, since we haven't figured out how to reproduce the problem on a local build.

mwinkel-dev commented 6 months ago

Hi @smithsp -- Thanks for the update. I will therefore keep this issue open. When you report success with the Conda Forge build, then we will close this issue.