Project

General

Profile

Feature #2272

clarify: why does common_cs.h get installed by debian packaging?

Added by neels 3 months ago. Updated 2 days ago.

Status:
New
Priority:
High
Assignee:
Category:
-
Target version:
-
Start date:
05/18/2017
Due date:
% Done:

10%

Resolution:
Spec Reference:

Description

openbsc/include/openbsc/common_cs.h is listed as noinst_HEADER.
However, it also appears in debian/openbsc-dev.install and is apparently installed.
This seems to magically work, although it shouldn't.

Related: gsm_data_shared.h includes <openbsc/common_cs.h>.
Related: until recently, the installed files were actually put in '$prefix/openbsc /*', i.e. with a trailing space in 'openbsc '. How did that not break our builds?
Related: when we added rest_octets.h to gsm_data_shared.h, the osmo-bts nightly packages build broke. https://gerrit.osmocom.org/2677

There appears to be some magic going on there that should not happen. Clarify what's going on there and why stuff works despite being broken. When this is done, all installed headers should be listed as such in the Makefile.am, debian should be able to package only installed headers, and a space in the 'openbsc ' installed dir should lead to build failures.

History

#1 Updated by msuraev 3 months ago

  • Priority changed from Normal to Low

This seems to magically work, although it shouldn't.

Why shouldn't? The obvious answer is that entries in debian/*.install have higher priority than noinst_HEADER. I don't have any reference from debian.org to back this up though - links are appreciated.

until recently, the installed files were actually put in '$prefix/openbsc /*'

Could you be more specific? In which files(s) was extra space? Which commits added/removed it?

all installed headers should be listed as such in the Makefile.am

What for? Aesthetic reasons or there'll be some improvement/fix for some scenario?

debian should be able to package only installed headers

Not sure I'm following this. Isn't it already package installed headers?

Setting priority to low as it might be made obsolete by BSC/MSC split in few months anyway.

#2 Updated by neels 3 months ago

  • Priority changed from Low to Normal

I think you could find out these things from looking at it more closely, but if you need it spelled out:

  • In my understanding, debian packaging does a 'make install' to a temp dir and can only possibly include files in the package that were installed there during 'make install'. How then can we install a header file that is not listed in Makefile.am as installable. I might be wrong but we should be able to explain what is going on.
  • A 'make install' should essentially be similar to a debian package installation. If we install files in the deb package that we don't from 'make install', that is confusing and wrong IMO. It should match. It should be obvious and not "magic", avoid as much obscurity as you can. I hope you agree there.
  • The gerrit patch that fixes the trailing space: https://gerrit.osmocom.org/2649; correction, missing 'include' path element: $prefix/include/openbsc<space>.
    But the actual question is: why does it not cause the build to fail when the installed headers are basically missing (aka at the wrong path). In other words, it seems that these headers don't even need to be installed in the first place??
  • Finally, common_cs.h wasn't intended to be installed. Why does it need to be installed?

Putting priority back to normal because it should not take long to answer these questions. It is not clear yet when exactly we will move to the split git repositories. I would appreciate if you would clarify this instead of discarding the questions.

#3 Updated by neels 3 months ago

  • Priority changed from Normal to High

Any news here please? To be able to split up the openbsc.git repository, I need to know which dependencies are needed where. Please sort out this uncertainty and let me know the intention, instead of me trying to guess why things are the way they are and possibly breaking things without being aware.

#4 Updated by laforge 8 days ago

ping? what's the status here?

#5 Updated by msuraev 7 days ago

I haven't double checked for references regarding my guess that "entries in debian/*.install have higher priority than noinst_HEADER" due to other work. I'm not sure what uses common_cs.h yet.

#6 Updated by msuraev 6 days ago

  • % Done changed from 0 to 10

The common_cs.h is included by Neels (in ac1f1436e9d380f632dd850fcd253d3480f0fc2d) into gsm_data_shared.h so it have to be installed (even if initially it wasn't intended to).

#7 Updated by msuraev 6 days ago

It feels kinda odd that I'm answering to Neels' question regarding the header introduced by Neels with the ref to Neels' commit. Shall we cut the middle-man and just assign ticket to Neels?

#8 Updated by neels 2 days ago

msuraev wrote:

The common_cs.h is included by Neels (in ac1f1436e9d380f632dd850fcd253d3480f0fc2d) into gsm_data_shared.h so it have to be installed (even if initially it wasn't intended to).

common_cs.h added to gsm_data_shared.h was a mistake, i.e. I added the dependency by accident, thanks for spotting that. Including common_cs.h in the debian installation may have seemed like the logical consequence but was not intended, we should probably have had a chat about that back then... I reverted My commit ac1f1436 in https://gerrit.osmocom.org/3570.

@msuraev, please drop common_cs.h from the debian installation.

Also please fix or clarify the other issues raised here.

Also available in: Atom PDF