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

[geographiclib] Update to 1.50.1 #12379

Merged
merged 19 commits into from
Jul 14, 2020
Merged

Conversation

cffk
Copy link
Contributor

@cffk cffk commented Jul 11, 2020

Upgrade to geographiclib 1.50.1

I am the author of GeographicLib and am willing to take "ownership" of this vcpkg port.

@ghost
Copy link

ghost commented Jul 11, 2020

CLA assistant check
All CLA requirements met.

@cffk
Copy link
Contributor Author

cffk commented Jul 11, 2020

@NancyLi1013 + @strega-nil: Well, I'm temporarily defeated by the *_uwp failures. The error logs are completely opaque and I have no idea what uwp even means. Any assistance would be appreciated.

@strega-nil
Copy link
Contributor

@cffk uwp is "Universal Windows Platform"; it's this kinda thing https://docs.microsoft.com/en-us/windows/uwp/get-started/universal-application-platform-guide. They used to be the only thing you could publish to the windows store.

@cffk
Copy link
Contributor Author

cffk commented Jul 12, 2020

I tracked down the problem with uwp. Evidently, there's a problem creating executables even if they are only used for testing and are not installed. I will try creating and installing the executables to see if that's allowed.

@cffk
Copy link
Contributor Author

cffk commented Jul 12, 2020

@strega-nil The issue with uwp and compiling executable programs is the error, e.g.,

error APPX0703: Manifest references file 'Planimeter.exe'
which is not part of the payload

The ideal solution would be figure out how to resolve this error. Failing that, what's the cleanest way for cmake to figure out it's compiling for uwp so I can skip the building of executables for this platform?

@cffk
Copy link
Contributor Author

cffk commented Jul 12, 2020

OK, this PR is ready for merging!

I fixed the uwp problem by not building the tools for uwp. But it would be good to know if it is possible to avoid this restriction.

@JackBoosY JackBoosY added the category:port-update The issue is with a library, which is requesting update new revision label Jul 13, 2020
ports/geographiclib/portfile.cmake Outdated Show resolved Hide resolved
ports/geographiclib/portfile.cmake Outdated Show resolved Hide resolved
ports/geographiclib/portfile.cmake Outdated Show resolved Hide resolved
ports/geographiclib/vcpkg.json Outdated Show resolved Hide resolved
@NancyLi1013 NancyLi1013 changed the title Geographiclib/1.50.1 [geographiclib] Update to 1.50.1 Jul 13, 2020
@cffk cffk requested a review from NancyLi1013 July 13, 2020 12:50
@johnco3
Copy link

johnco3 commented Jul 13, 2020

Tried to install the pull request after building locally but it failed. Need to remove the trailing comment in the last line of vcpkg.json file.

It should be:

{
  "name": "geographiclib",
  "version-string": "1.50.1",
  "homepage": "https://geographiclib.sourceforge.io",
  "description": "a small set of C++ classes for performing conversions between geographic, UTM, UPS, MGRS, geocentric, and local cartesian coordinates, for gravity (e.g., EGM2008), geoid height, and geomagnetic field (e.g., WMM2010) calculations, and for solving geodesic problems."
}

C:\Users\johnc\main\extlibs\vcpkg1>.\vcpkg.exe install geographiclib
Computing installation plan...
Error: while loading geographiclib:
Error: C:/Users/johnc/main/extlibs/vcpkg1/ports/geographiclib/vcpkg.json:6:1: Trailing comma in an object
   on expression: {
  "name": "geographiclib",
  "version-string": "1.50.1",
  "homepage": "https://geographiclib.sourceforge.io",
  "description": "a small set of C++ classes for performing conversions between geographic, UTM, UPS, MGRS, geocentric, and local cartesian coordinates, for gravity (e.g., EGM2008), geoid height, and geomagnetic field (e.g., WMM2010) calculations, and for solving geodesic problems.",
}
                                                                                                                                                                                                                                                                                                                                                                                                                                ^

Error: Failed to load port from geographiclib

C:\Users\johnc\main\extlibs\vcpkg1>.\vcpkg.exe search geographiclib --debug
[DEBUG] Feature flag 'binarycaching' unset
[DEBUG] Feature flag 'manifests' unset
[DEBUG] Feature flag 'compilertracking' unset
[DEBUG] Using vcpkg-root: C:\Users\johnc\main\extlibs\vcpkg1
[DEBUG] Using installed-root: C:\Users\johnc\main\extlibs\vcpkg1\installed
[DEBUG] Using buildtrees-root: C:\Users\johnc\main\extlibs\vcpkg1\buildtrees
[DEBUG] Using downloads-root: C:\Users\johnc\main\extlibs\vcpkg1\downloads
[DEBUG] Using packages-root: C:\Users\johnc\main\extlibs\vcpkg1\packages
[DEBUG] Using scripts-root: C:\Users\johnc\main\extlibs\vcpkg1\scripts
Error: while loading geographiclib:
Error: C:/Users/johnc/main/extlibs/vcpkg1/ports/geographiclib/vcpkg.json:6:1: Trailing comma in an object
   on expression: {
  "name": "geographiclib",
  "version-string": "1.50.1",
  "homepage": "https://geographiclib.sourceforge.io",
  "description": "a small set of C++ classes for performing conversions between geographic, UTM, UPS, MGRS, geocentric, and local cartesian coordinates, for gravity (e.g., EGM2008), geoid height, and geomagnetic field (e.g., WMM2010) calculations, and for solving geodesic problems.",
}
                                                                                                                                                                                                                                                                                                                                                                                                                                ^

If your library is not listed, please open an issue at and/or consider making a pull request:
    https://github.com/Microsoft/vcpkg/issues
[DEBUG] C:\Users\johnc\main\extlibs\vcpkg1\toolsrc\src\vcpkg\commands.search.cpp(146)
[DEBUG] Exiting after 341452 us (332706 us)

C:\Users\johnc\main\extlibs\vcpkg1>

ports/geographiclib/portfile.cmake Outdated Show resolved Hide resolved
ports/geographiclib/vcpkg.json Outdated Show resolved Hide resolved
@cffk
Copy link
Contributor Author

cffk commented Jul 13, 2020

@johnco3 Just edit vcpkg.json to remove the last comma and you should be OK. I'm not making this change myself, because I've got other changes to do to this file.

@cffk
Copy link
Contributor Author

cffk commented Jul 13, 2020

@johnco3 I've pushed changes which should fix your problem...

Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for merge once the two comments are addressed!

ports/geographiclib/usage Show resolved Hide resolved
ports/geographiclib/vcpkg.json Outdated Show resolved Hide resolved
@cffk
Copy link
Contributor Author

cffk commented Jul 13, 2020

There's still a problem with the tools because find_package doesn't find them where they were installed (because they got copied to tools/geographiclib). I'll duct-tape around this problem. However, it points out that the vcpkg after-the-fact fixups run somewhat counter to the cmake way of doing things.

@ras0219-msft ras0219-msft merged commit e554608 into microsoft:master Jul 14, 2020
@ras0219-msft
Copy link
Contributor

Thanks for the new library and responding to our reviews so quickly! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants