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

make install better #23

Merged
merged 18 commits into from
Mar 3, 2018
Merged

make install better #23

merged 18 commits into from
Mar 3, 2018

Conversation

gabyx
Copy link
Contributor

@gabyx gabyx commented Feb 25, 2018

  • removed include/comp from install (probably not needed, otherwise should be somewhere else)
  • version file is written by cmake
  • lincense,readme is installed as well
  • gnuinstalldirs macro used to properly setup the install directories
  • fixed some wrong includes
install
├── bin
│   └── nodesize_dbg
├── include
│   └── foonathan_memory
│       ├── comp
│       │   └── foonathan
│       │       ├── alignas.hpp
│       │       ├── alignof.hpp
│       │       ├── clz.hpp
│       │       ├── constexpr.hpp
│       │       ├── exception_support.hpp
│       │       ├── get_new_handler.hpp
│       │       ├── hosted_implementation.hpp
│       │       ├── literal_op.hpp
│       │       ├── max_align_t.hpp
│       │       ├── mutex.hpp
│       │       ├── noexcept.hpp
│       │       ├── pmr.hpp
│       │       ├── rvalue_ref.hpp
│       │       └── thread_local.hpp
│       ├── config_impl.hpp
│       ├── container_node_sizes_impl.hpp
│       └── foonathan
│           └── memory
│               ├── aligned_allocator.hpp
│               ├── allocator_storage.hpp
│               ├── allocator_traits.hpp
│               ├── config.hpp
│               ├── container.hpp
│               ├── debugging.hpp
│               ├── default_allocator.hpp
│               ├── deleter.hpp
│               ├── detail
│               │   ├── align.hpp
│               │   ├── assert.hpp
│               │   ├── container_node_sizes.hpp
│               │   ├── debug_helpers.hpp
│               │   ├── ebo_storage.hpp
│               │   ├── free_list.hpp
│               │   ├── free_list_array.hpp
│               │   ├── lowlevel_allocator.hpp
│               │   ├── memory_stack.hpp
│               │   ├── small_free_list.hpp
│               │   └── utility.hpp
│               ├── error.hpp
│               ├── fallback_allocator.hpp
│               ├── heap_allocator.hpp
│               ├── iteration_allocator.hpp
│               ├── joint_allocator.hpp
│               ├── malloc_allocator.hpp
│               ├── memory_arena.hpp
│               ├── memory_pool.hpp
│               ├── memory_pool_collection.hpp
│               ├── memory_pool_type.hpp
│               ├── memory_resource_adapter.hpp
│               ├── memory_stack.hpp
│               ├── namespace_alias.hpp
│               ├── new_allocator.hpp
│               ├── segregator.hpp
│               ├── smart_ptr.hpp
│               ├── static_allocator.hpp
│               ├── std_allocator.hpp
│               ├── temporary_allocator.hpp
│               ├── threading.hpp
│               ├── tracking.hpp
│               └── virtual_memory.hpp
├── lib
│   └── libfoonathan_memory-0.6.1-dbg.a
└── share
    └── foonathan_memory
        ├── LICENSE
        ├── README.md
        └── cmake
            ├── foonathan_memory-config-debug.cmake
            ├── foonathan_memory-config.cmake
            └── foonathan_memory-version.cmake

Multiple version install is not really supported by cmake, but one can always install different version next to each other:
(e.g. /usr/local/opt/{foonathan_memory-0.5.9,foonathan_memory-0.6.1})
and then use one of them with

find_package(foonathan_memory PATH /usr/local/opt/{foonathan_memory-0.5.9)`

if that is a requirement.

- removed include/comp from install (probably not needed, otherwise should be somewhere else)
-  version file is written by cmake
- lincense,readme is installed as well
- gnuinstalldirs macro used to properly setup the install directories
…version.cmake ...

If one wants multiple versions next to each other, there is the install_prefix which should be used for installation (e.g. /usr/local/opt/{foonathan_memory-0.5.9,foonathan_memory-0.61})
Copy link
Owner

@foonathan foonathan left a comment

Choose a reason for hiding this comment

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

I'm trusting the CMake code you wrote, haven't been keeping up to date there.

@@ -0,0 +1,539 @@
{
Copy link
Owner

Choose a reason for hiding this comment

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

You forgot to remove this file from Git.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh good catch!,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think if you squash merge, it will go away (?) (I deleted it in the recent commit)

#include "detail/utility.hpp"
#include "config.hpp"
#include "utility.hpp"
#include "../config.hpp"
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, noticed that one as well, is fixed in a branch currently on CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, will be a small merge conflict probably

@gabyx
Copy link
Contributor Author

gabyx commented Feb 26, 2018

The cmake code is also almost the same as in https://github.com/rttrorg/rttr
where I helped improve the install too. It works very well and is in accordance to a modern cmake approach, which you have already used throughout a lot :-) (most people set global cmake flags and stufff, which is not really good :-))

@foonathan foonathan merged commit 2fed4a4 into foonathan:master Mar 3, 2018
@foonathan
Copy link
Owner

Thank you.

@gabyx
Copy link
Contributor Author

gabyx commented Mar 3, 2018 via email

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.

2 participants