iceoryx icon indicating copy to clipboard operation
iceoryx copied to clipboard

Create a flat include structure for `iceoryx_hoofs`

Open mossmaurice opened this issue 3 years ago • 8 comments

Brief feature description

Often headers are moved inside iceoryx_hoofs both in folder and namespaces. This burdens the user with unnecessary code changes, having to adapt the include path.

Detailed information

The idea would be such a structure

include
  |-iox
    |-vector
    |-string
    |-...

with e.g. vector containing

#include "iceoryx_hoofs/cxx/vector.hpp"

namesapce iox
{
    using cxx::vector;
}

On the user side it would look like

#include "iox/vector"

iox::vector<int, 42> foo;

Things to consider

From the developer meetup https://github.com/eclipse-iceoryx/iceoryx/wiki/2023-11-16-Eclipse-iceoryx-developer-meetup#minutes

  1. Relax flat include structure
    • posix is flat include and flat ioxnamespace
    • concurrent is flat include and iox::concurrent namespace
    • units is flat include and iox::units namespace
    • non public API has a iox/detail/foo.hpp include and is in the iox::detail namespace
    • non public API of bigger modules, like cli can opt in for iox/detail/cli and iox::detail::cli
    • public API is in iox/header.hpp

From https://github.com/eclipse-iceoryx/iceoryx/pull/2142#discussion_r1439803183

We can do this also at a later point, it was just my personal taste.

My personal favorite would be:

iox::lock_free::{mpmc|spsc} - for all lockfree code
iox::concurrent for generic thread safe code

But I would suggest that we talk about it in a later meetup and go from there.

Tasks

  • [ ] check which classes should be public
  • [ ] determine how to proceed with namespaces

mossmaurice avatar Aug 26 '22 13:08 mossmaurice

Sync with @elBoberido and @elfenpiff

How to proceeded further with the module structure?

  1. Differentiation between user-view and iceoryx developer-view
    1. First step: #1391 iceoryx developer-view aka new folder structure (possibly without CMake changes)
    2. Second step: #1593 user-view to decouple the user from any future changes

Action items

  • Independently brainstorm for a iceoryx developer-view module structure

mossmaurice avatar Sep 20 '22 15:09 mossmaurice

@mossmaurice @elfenpiff what do you think about this

iceoryx_hoofs
|- concurrent
|- core
|- posix

Everything not concurrent or posix would be part of core and each of these three pillars would have a folder structure with container, memory, etc.?

elBoberido avatar Sep 20 '22 19:09 elBoberido

@elBoberido It might be confusing for developers browsing the repository on the first sight and might seem verbose to some. No hard feelings here, though.

What do you think about?

iceoryx_hoofs
|- container
|- queues
|- etc.
|- concurrent
  |- container
  |- queues
  |- etc.
|- posix
  |- container
  |- queues
  |- etc.

Just to make sure to avoid misunderstanding, I'm only talking about the internal structure not the user-facing one.

mossmaurice avatar Sep 20 '22 22:09 mossmaurice

@mossmaurice I think your structure goes in the right direction but is to fine grained. I would merge queues and containers. If both would contain more than 10 classes than this would be a meaningful distinction but for now it is not required and YAGNI tells us not to do it. Otherwise it is a good structure!

In the first step I would not create the same structure in concurrent and posix. I would introduce them when we have circular dependencies. For concurrent they may make sense but here we would have only queues and other stuff and not a single posix construct would fit into that kind of structure.

The internal structure of those proposed modules could be:

module
|- include
  |- internal or details
|- src
|- tests

I suggest:

iceoryx_hoofs
|- concurrent
  |- leave it as it is
|- conversion
  |- convert in a refactored form (int -> string)
  |- From / Into
  |- Serialization
|- containers
  |- ?buffer? from lockfree_queue as uninitialized_array
  |- ?expected?
  |- list
  |- ?optional?
  |- ?scoped_static?
  |- stack
  |- ?static_storage?
  |- string
  |- ?variant?
  |- ?variant_queue?
  |- vector
|- design
  |- newtype
  |- builder
|- functional
  |- function_ref
  |- function
  |- ?scope_guard?
|- meta
  |- algorithm
  |- attributes
  |- parts of helplets
  |- functional_interface
  |- ?requires? when not replaced with error handler
  |- types
  |- type_traits
|- memory
  |- unique_ptr
  |- relocatable_ptr
  |- allocator (from shared_memory_object)
|- posix
  |- I would add here filesystem.hpp since user/group/other with the permission is defined by the posix standard
  |- rest leave it like it is
|- units
  |- duration

elfenpiff avatar Sep 21 '22 09:09 elfenpiff

@elfenpiff a separate CMakeList.txt for each module would not work, at least not when it is more fine grained.

Example:

  • container/vector uses the logger
  • the logger uses the posix/posixCall
  • posix uses the vector

And now we have the circular dependency

if we would have something like

poxix
|- module with posixCall
|- module with shm?
|- maybe another module

Then it would again be possible to prevent circular dependencies

elBoberido avatar Sep 21 '22 19:09 elBoberido

@mossmaurice I also thought about that and had a slight tendency to move everything into a common folder but your suggestion might make more sense. With my proposal we would walk the dependency tree like this hoofs/core/log -> hoofs/core -> hoofs -> hoofs/posix -> hoofs/posix -> hoofs/posix/foo (contains posixCall) with your suggestion there would be one layer less and it would feel better.

elBoberido avatar Sep 21 '22 20:09 elBoberido

@elBoberido As this issue here is about the user-facing structure, let's move this discussion to #1391 and start implementing it next week once there is a decision.

mossmaurice avatar Sep 23 '22 16:09 mossmaurice

@elBoberido @mossmaurice @elfenpiff

Agreed on the following final structure:

./iceoryx_hoofs
./iceoryx_hoofs/legacy
./iceoryx_hoofs/legacy/include
./iceoryx_hoofs/legacy/include/iceoryx_hoofs
./iceoryx_hoofs/legacy/include/iceoryx_hoofs/cxx
./iceoryx_hoofs/legacy/include/iceoryx_hoofs/cxx/vector.hpp
./iceoryx_hoofs/container
./iceoryx_hoofs/container/source
./iceoryx_hoofs/container/source/vector.cpp
./iceoryx_hoofs/container/include
./iceoryx_hoofs/container/include/iox
./iceoryx_hoofs/container/include/iox/vector.hpp
./iceoryx_hoofs/container/include/iox/detail
./iceoryx_hoofs/container/include/iox/detail/vector.inl
./iceoryx_hoofs/container/unittests
./iceoryx_hoofs/integrationtests
  • the folder legacy is the migration path
  • in the first step we can leave the tests under iceoryx_hoofs/tests but in a final solution we will
    • use iceoryx_hoofs/container/unittests for all unit tests
    • use iceoryx_hoofs/integrationtests for all integration tests which tests the composition of multi-component structures

elfenpiff avatar Sep 26 '22 15:09 elfenpiff

Modules for iceoryx_hoofs were created in:

  • #2143
  • #2142
  • #2061
  • #2133

Hence, closing this issue.

mossmaurice avatar Feb 23 '24 11:02 mossmaurice

@mossmaurice while the work is done, we still need to add a few words to the release notes

elBoberido avatar Feb 23 '24 16:02 elBoberido