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 FortranWrapper.h that enables SerialBox to compile with Fortran features on MacOS #251

Merged
merged 7 commits into from
Jun 23, 2021

Conversation

gmao-ckung
Copy link
Contributor

@gmao-ckung gmao-ckung commented Jun 8, 2021

Compiling the current master branch on MacOS Catalina results in the following undefined symbol errors

Undefined symbols for architecture x86_64: 
  "_serialboxFortranSavepointAddMetainfoInt64", referenced from: 
      ___m_serialize_MOD_fs_add_savepoint_metainfo_l in m_serialize.f90.o 
  "_serialboxFortranSavepointGetMetainfoInt64", referenced from: 
      ___m_serialize_MOD_fs_get_savepoint_metainfo_l in m_serialize.f90.o 
  "_serialboxFortranSerializerAddFieldMetainfoInt64", referenced from: 
      ___m_serialize_MOD_fs_add_field_metainfo_l in m_serialize.f90.o 
  "_serialboxFortranSerializerAddMetainfoInt64", referenced from: 
      ___m_serialize_MOD_fs_add_serializer_metainfo_l in m_serialize.f90.o 
  "_serialboxFortranSerializerGetFieldMetainfoInt64", referenced from: 
      ___m_serialize_MOD_fs_get_field_metainfo_l in m_serialize.f90.o 
  "_serialboxFortranSerializerGetMetainfoInt64", referenced from: 
      ___m_serialize_MOD_fs_get_serializer_metainfo_l in m_serialize.f90.o 

The fix is to change the long and long* parameter types for the Int64 routines in FortranWrapper.h to int64_t and int64_t* respectively.

This was tested using MacOS Catalina v 10.15.7 and built using MacOS clang-based C and C++ compilers and gfortran v 11.1.0 installed via Homebrew.

@jenkins-apn
Copy link
Collaborator

Hi there, this is jenkins continuous integration...
Do you want me to verify this patch?

@gmao-ckung
Copy link
Contributor Author

Yes verify the patch

@havogt
Copy link
Collaborator

havogt commented Jun 21, 2021

launch jenkins

@havogt
Copy link
Collaborator

havogt commented Jun 21, 2021

Thanks, for the contribution. Please merge with the updated master to reflect the fixes in CI.

@havogt
Copy link
Collaborator

havogt commented Jun 21, 2021

Additionally, please add yourself at the bottom of the AUTHORS file with name and affiliation.

@havogt
Copy link
Collaborator

havogt commented Jun 21, 2021

And in case your are motivated, a PR to extending the GitHub action tests to MacOS is very welcome, see #246.

@havogt
Copy link
Collaborator

havogt commented Jun 21, 2021

Hi @gmao-ckung,
for contributions to the GridTools framework, which contains Serialbox, I need to ask you to sign the contributor assignment agreement. (I thought you are employed at Vulcan, which would have been covered already.) Please send the signed document to vogt@cscs.ch. Alternatively, for this particular, obvious contribution, I can merge directly, if you confirm that such a contribution is not copyrightable and you remove yourself again from the AUTHORS list. Sorry, for the confusion.

@gmao-ckung
Copy link
Contributor Author

gmao-ckung commented Jun 21, 2021

Hi @havogt,

I apologize for the confusion about my affiliation. Given that this fix is relatively small, I think it’ll be easiest if I take my name off the “AUTHORS” list. I’ll resubmit a version with my name off the list. You can merge the changes once I make the edit.

@havogt
Copy link
Collaborator

havogt commented Jun 22, 2021

Please give Andrea his n back. ;)

@gmao-ckung
Copy link
Contributor Author

Good catch!

@havogt havogt merged commit 80f0c94 into GridTools:master Jun 23, 2021
@havogt
Copy link
Collaborator

havogt commented Jun 23, 2021

Thanks again for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants