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

Releases 3.6.0 and 3.6.1 don't build on conda / windows #1536

Closed
SylvainCorlay opened this issue Mar 21, 2019 · 17 comments
Closed

Releases 3.6.0 and 3.6.1 don't build on conda / windows #1536

SylvainCorlay opened this issue Mar 21, 2019 · 17 comments
Assignees
Labels
kind: bug platform: visual studio related to MSVC release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@SylvainCorlay
Copy link

  • What is the issue you have?

I am the maintainer of the nlohman_json package on conda-forge. The package for the latest releases don't build on windows (VS 2015)

MSVC / Windows / (VS 2015)

  • Did you use a released version of the library or the version from the develop branch?

Released version.

The compilation error occurs while building the tests, as per the logs above.

@nlohmann
Copy link
Owner

That is odd, because the MSVC 2015 build runs successfully at our AppVeyor, see https://ci.appveyor.com/project/nlohmann/json/builds/23234117.

@SylvainCorlay
Copy link
Author

Note that this only occurs with 3.6.x. Previous versions of nlohmann_json up until 3.5.x have been packaged succesfully on conda-forge.

@nlohmann
Copy link
Owner

nlohmann commented Mar 21, 2019

The compilation error is in file unit-allocator.cpp. The changes there toward 3.6.0 came with #1492. I'm no expert in MSVC. Maybe @stac47 or @theodelrieu have an idea on this, as they also proposed the fixes for GCC 9.

@theodelrieu
Copy link
Contributor

It would be great to look at the xmemory file to see what the call is. But my guess is that my_allocator only defines some requirements of Allocator, it seems to miss constructors at least.

Maybe using std::allocator<T>::allocator will be enough. If not, adding the constructors should do the trick.

I would suggest removing the inheritance from std::allocator and rewrite it from scratch though.

@nlohmann
Copy link
Owner

@theodelrieu Good point, I'll try the using std::allocator<T>::allocator.

@nlohmann
Copy link
Owner

@SylvainCorlay Could you try whether baaa2a4 builds?

@mistersandman
Copy link

The issue appears only when building test-allocator with MSVC in Debug mode, where the STL is augmented with debugging constructs. Adding the following constructors to my_allocator in unit-allocator.cpp seems to also fix the issue:

my_allocator() = default;

template <class U>
my_allocator(const my_allocator<U>&) {}

@nlohmann
Copy link
Owner

@mistersandman Thanks for the info! As you seem to have access to MSVC (I don't...), could you please check whether baaa2a4 also works?

@mistersandman
Copy link

I just checked and it works! Your fix is cleaner than mine 😃

@mistersandman
Copy link

The question for @SylvainCorlay remains, whether it is even necessary for his case to build the test suite. If the goal is only to install the library, there's no need for it. Building the test suite can be turned off by adding -DBUILD_TESTING=OFF to the command line when invoking CMake.

@SylvainCorlay
Copy link
Author

Hey, thanks for the responses, I am busy for the next couple of hours but will respond after.

@nlohmann
Copy link
Owner

@SylvainCorlay Does baaa2a4 fix the issue?

@SylvainCorlay
Copy link
Author

SylvainCorlay commented Mar 23, 2019

Trying it out now.

@SylvainCorlay
Copy link
Author

Including your patch in the recipe appears to be fixing the compilation issue!

@SylvainCorlay
Copy link
Author

3.6.0 and 3.6.1 are now on conda-forge!

@nlohmann
Copy link
Owner

Thanks a lot for your efforts!

@nlohmann nlohmann self-assigned this Mar 23, 2019
@nlohmann nlohmann added this to the Release 3.6.2 milestone Mar 23, 2019
@nlohmann
Copy link
Owner


🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

  • ✨ New Feature
  • 🐛 Bug Fix
  • ⚡️ Improvement
  • 🔨 Further Change
  • 🔥 Deprecated function

Description


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug platform: visual studio related to MSVC release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

4 participants