Project

General

Profile

Task #4782

Install implementation detail headers

Added by Junxiao Shi 4 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Base
Target version:
Start date:
12/06/2018
Due date:
% Done:

100%

Estimated time:

Description

So far the headers in detail directory are not installed to the target system.
Some headers, such as common.hpp, are implementation detail but their headers should be installed so that other headers can include them.

This task is to introduce a directory for installed implementation detail headers:

  • impl directory (renamed from detail) contains implementation detail with non-installed headers.
  • detail directory contains implementation detail with installed headers.

Related issues

Blocks ndn-cxx - Feature #3919: Scoped prefix registrationCode review

Blocked by ndn-cxx - Task #3084: Rename "src" folder to "ndn-cxx"Closed

History

#1 Updated by Junxiao Shi 4 months ago

#2 Updated by Junxiao Shi 4 months ago

  • Blocked by Task #3084: Rename "src" folder to "ndn-cxx" added

#3 Updated by Junxiao Shi 3 months ago

  • Status changed from New to In Progress
  • Assignee set to Junxiao Shi

#4 Updated by Junxiao Shi 3 months ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 100

#5 Updated by Junxiao Shi 3 months ago

Question was raised in https://gerrit.named-data.net/#/c/ndn-cxx/+/5066/1/ndn-cxx/detail/common-pch.hpp about whether common-pch.hpp should be installed.
I don't understand how common-pch.hpp works. Can someone explain?

#6 Updated by Alex Afanasyev 3 months ago

pch = precompiled header. None of the precompiled headers should be installed, as they are only used to "speed up" compilation and only within the build system itself, they are not (and should not) be explicitly included anywhere.

#7 Updated by Junxiao Shi 3 months ago

It has been suggested that there are more headers that can go into 'detail' now that it's installed:

  • net/asio-fwd.hpp: yes
  • util/cf-releaser-osx.hpp: yes
  • util/cf-string-osx.hpp: yes
  • util/sqlite3-statement.hpp: no, used by repo-ng
  • packet-base.hpp: yes

I'll move the "yes" ones in another commit, as moving common.hpp is already complex enough.

#8 Updated by Davide Pesavento 3 months ago

Probably you can add transport/stream-transport{,-with-resolver}-impl.hpp to the list.
I'm not sure about tag-host.hpp

#9 Updated by Junxiao Shi 3 months ago

https://gerrit.named-data.net/5069 PacketBase and TagHost
I don't see TagHost being used elsewhere so I moved it. I'll confirm with ndn-cxx-breaks.

https://gerrit.named-data.net/5070 asio-fwd
I'm moving it to ndn-cxx/detail/ rather than ndn-cxx/net/detail/ because it's being used by more than ndn-cxx/net/*.

https://gerrit.named-data.net/5071 StreamTransportImpl
This should be safe.

https://gerrit.named-data.net/5072 CFReleaser and cfstring
I can't test it because I no longer have account on monaco.

#10 Updated by Junxiao Shi 3 months ago

ndn-cxx-breaks logs before and after.
ndns is broken by these changed, and it's fixed in https://gerrit.named-data.net/5073 .
repo-ng has problems in unit testing, which is unrelated.
NLSR gained a new dependency and is not working in ndn-cxx-breaks at the moment.

#11 Updated by Davide Pesavento 3 months ago

  • Status changed from Code review to Closed

Also available in: Atom PDF