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.

Sign up for GitHub

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

Jump to bottom

udunits2: update to 2.2.27.14 #8155

Merged
merged 1 commit into from
Sep 21, 2020
Merged

udunits2: update to 2.2.27.14 #8155

merged 1 commit into from
Sep 21, 2020

Conversation

chrstphrchvz
Copy link
Contributor

See: https://trac.macports.org/ticket/61032

Description

Tested on

macOS 10.15.6
Xcode command line tools 12 beta 5

Verification

Have you

  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Notifying maintainers:
@tenomoto for port udunits2.

@chrstphrchvz chrstphrchvz marked this pull request as draft August 20, 2020 00:58
@chrstphrchvz
Copy link
Contributor Author 

[  8%] Building C object lib/CMakeFiles/libudunits2.dir/converter.c.o
cd /opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/build/lib && /usr/bin/clang -Dlibudunits2_EXPORTS -I/opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/build -I/opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/UDUNITS-2-2.2.27.14/lib/. -I/opt/local/include -pipe -Os -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -mmacosx-version-min=10.15 -fPIC -o CMakeFiles/libudunits2.dir/converter.c.o -c /opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/UDUNITS-2-2.2.27.14/lib/converter.c

/opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/UDUNITS-2-2.2.27.14/lib/converter.c:187:12: error: implicitly declaring library function 'snprintf' with type 'int (char *, unsigned long, const char *, ...)' [-Werror,-Wimplicit-function-declaration]

    return snprintf(buf, max, "%s", variable);
           ^
/opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/UDUNITS-2-2.2.27.14/lib/converter.c:187:12: note: include the header <stdio.h> or explicitly provide a declaration for 'snprintf'

I don't know why this error appears, since lib/converter.c already has #include <stdio.h>: https://github.com/Unidata/UDUNITS-2/blob/00ce1e1/lib/converter.c#L25

@ryandesign
Copy link
Contributor

I don't know why this error appears, since lib/converter.c already has #include <stdio.h>

Probably because before that it says:

#   define _XOPEN_SOURCE 500

@chrstphrchvz
Copy link
Contributor Author

I'm not familiar with _XOPEN_SOURCE, but it appears to be common knowledge that there are inconsistencies with using it on macOS (or other BSDs), i.e. it needs to be set to 600 in this case (if at all). What would be the preferred way to address this in MacPorts: patch lib/converter.c using reinplace, or predefine it in configure.cppflags? This is something that should also be addressed upstream, correct?

I observed a compile error rather than a warning on my system because -Werror=implicit-function-declaration will be the default behavior in Xcode 12: https://developer.apple.com/documentation/xcode-release-notes/xcode-12-beta-release-notes/

@ryandesign
Copy link
Contributor

I'm not very familiar with _XOPEN_SOURCE either, but it seems that if it's defined, then it limits the features available in the system headers, and there's disagreement between macOS and Linux and maybe other systems as to which value of that define has which effect. Yes, it would be great if upstream would change their code so that it compiles without changes on macOS. I don't know why they set _XOPEN_SOURCE in this file or what effect removing it or changing what it's defined to would have on other operating systems. If you patch the file, use a patchfile not a reinplace.

@kencu
Copy link
Contributor

kencu commented Aug 20, 2020

https://man7.org/linux/man-pages/man7/feature_test_macros.7.html

@macportsbot
Copy link

Travis Build #13506 Passed.

Lint results 
--->  Verifying Portfile for udunits2
--->  0 errors and 0 warnings found.

Port udunits2 success on xcode10.3. Log
Port udunits2 success on xcode9.4. Log
Port udunits2 success on xcode8.3. Log

@macportsbot
Copy link

Travis Build #13519 Passed.

Lint results 
--->  Verifying Portfile for udunits2
--->  0 errors and 0 warnings found.

Port udunits2 success on xcode10.3. Log
Port udunits2 success on xcode9.4. Log
Port udunits2 success on xcode8.3. Log
Port udunits2 success on xcode7.3. Log

@chrstphrchvz
Copy link
Contributor Author

The remaining problem (for Xcode 12), which I haven't found the cause of:

[ 34%] Building C object lib/CMakeFiles/libudunits2.dir/parser.c.o
cd /opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/build/lib && /usr/bin/clang -Dlibudunits2_EXPORTS -I/opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/build -I/opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/UDUNITS-2-2.2.27.14/lib/. -I/opt/local/include -pipe -Os -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -DNDEBUG -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -mmacosx-version-min=10.15 -fPIC -o CMakeFiles/libudunits2.dir/parser.c.o -c /opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/UDUNITS-2-2.2.27.14/lib/parser.c
parser.c:1394:16: error: implicit declaration of function 'utlex' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
      yychar = yylex ();
               ^
/opt/local/var/macports/build/_opt_local_var_macports_sources_github.com_macports_macports-ports_science_udunits2/udunits2/work/UDUNITS-2-2.2.27.14/lib/parser.c:64:25: note: expanded from macro 'yylex'
#define yylex           utlex
                        ^
1 error generated.

@chrstphrchvz
Copy link
Contributor Author

https://man7.org/linux/man-pages/man7/feature_test_macros.7.html

This is neat, but where would the macOS/Darwin/BSD/libc++ equivalent be?

@macportsbot
Copy link

Travis Build #13532 Passed.

Lint results 
--->  Verifying Portfile for udunits2
--->  0 errors and 0 warnings found.

Port udunits2 success on xcode10.3. Log
Port udunits2 success on xcode9.4. Log
Port udunits2 success on xcode7.3. Log

@chrstphrchvz
Copy link
Contributor Author

The implicit declaration warning in parser.c is because yylex() is used at line 1394 but not defined until scanner.c is included at line 2152. Does this force MacPorts to use -Wno-error=implicit-function-declaration?

Someone tried proposing these files be reorganized upstream, but it's not clear to me whether it would avoid the implicit declaration, and it hasn't been touched in 2 years: Unidata/UDUNITS-2#74

@chrstphrchvz
udunits2: update to 2.2.27.14
b515ac6 
Add patch defining _XOPEN_SOURCE to 600 to allow using
snprintf() and strdup()
Still need -Wno-error=implicit-function-declaration because of
deliberate implicit declaration of yylex() in parser.c
See: https://trac.macports.org/ticket/61032
@chrstphrchvz
Copy link
Contributor Author

Rebased to incorporate change in 3f7d1f6; revised some comments. Marking as ready for review since the build succeeds on Xcode 12.

@chrstphrchvz chrstphrchvz marked this pull request as ready for review September 19, 2020 10:04
@macportsbot
Copy link

Travis Build #14100 Passed.

Lint results 
--->  Verifying Portfile for udunits2
--->  0 errors and 0 warnings found.

Port udunits2 success on xcode10.3. Log
Port udunits2 success on xcode9.4. Log
Port udunits2 success on xcode8.3. Log

@ryandesign ryandesign merged commit ff44f50 into macports:master Sep 21, 2020
@chrstphrchvz chrstphrchvz deleted the udunits-2.2.27.14 branch September 21, 2020 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryandesign ryandesign ryandesign left review comments

Assignees

@tenomoto tenomoto

Labels
maintainer: requires approval maintainer: timeout type: update
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@chrstphrchvz @macportsbot @ryandesign @kencu @tenomoto