Bug 1327

Summary: Version installed ns-3 files
Product: ns-3 Reporter: Vedran Miletić <vedran>
Component: build systemAssignee: Gustavo J. A. M. Carneiro <gjcarneiro>
Status: REOPENED ---    
Severity: normal CC: ns-bugs, tomh, wzssyqa
Priority: P5    
Version: pre-release   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 1317    
Attachments: Proposed patch
another patch
patchy to version the library file name and pcfiles
fixed patch
patch v3
Modified Gustavo's patch
Modified Gustavo's patch version 2
Patch addendum (disable VNUM)
Patch addendum 2 (name -l arguments properly in generated .pc files)

Description Vedran Miletić 2011-12-29 10:54:26 EST
Currently:

$ readelf -a build/libns3-core.so | grep SONAME
 0x000000000000000e (SONAME)             Library soname: [libns3-core.so]

It should be:

Library soname: [libns3-core.so.3.13]

or even

Library soname: [libns3-core.so.3.13.0]

so the .0 can be incremented in case of binary incompatibility.

However, when I talked to Fedora guys, one of them mentioned only first number being significant/used. I don't know more about this and Google didn't help me much; if that's true, we should probaby use ns-3 minor version as major version (e.g. ns-3.13 should have libns3-core.so.13).
Comment 1 Vedran Miletić 2011-12-30 19:06:21 EST
Interesting change with waf 1.6.10:

$ readelf -a lib/libns3-core.so | grep SONAME
 0x000000000000000e (SONAME)             Library soname: [libns3-core.so.3]

If we could make it use minor version instead of major, this would actually be perfect.
Comment 2 Vedran Miletić 2012-01-02 07:27:01 EST
Created attachment 1294 [details]
Proposed patch

This fixes it for me.

I prefer [2:] solution to ".".join(VERSION.split(".")[1:]) because:
1) it looks a lot cleaner
2) it will not break with 3-dev as VERSION
3) it's highly unlikely that ns-3 will have major number increased above 3, and even more unlikely that it will be increased above 9.

This produces libns3-core.so.14 with same SONAME, which is perfect for my needs. It's based on the assumptions:
1) every major release breaks binary compatibility (this is the assumption that boost also uses)
2) none of the minor releases break binary compatibility (this looks safe to assume from what has been the case so far, but I will CC Tom to see if it's really so)
Comment 3 Vedran Miletić 2012-01-02 07:28:19 EST
By the way, looking for waf + VNUM instructions, I found these two issues:
- http://code.google.com/p/waf/issues/detail?id=270
- http://code.google.com/p/waf/issues/detail?id=721
Both are closed; can you check if darwin workaround regarding VNUM is still needed in wscript?
Comment 4 Vedran Miletić 2012-01-03 17:33:16 EST
CC'ing Tom, forgot to do it earlier.
Comment 5 Tom Henderson 2012-01-04 00:31:44 EST
(In reply to comment #2)
> 
> This produces libns3-core.so.14 with same SONAME, which is perfect for my
> needs. It's based on the assumptions:
> 1) every major release breaks binary compatibility (this is the assumption that
> boost also uses)
> 2) none of the minor releases break binary compatibility (this looks safe to
> assume from what has been the case so far, but I will CC Tom to see if it's
> really so)

If you are asking about binary compatibility in the COM/GUID sense, we do not guarantee this even across minor releases.
Comment 6 Gustavo J. A. M. Carneiro 2012-01-04 07:18:59 EST
Created attachment 1296 [details]
another patch

This patch makes MINOR and MICRO version form the VNUM for the SOLIB, so that a micro version upgrade causes a new VNUM to be applied.  For instance, with a version like ns-3.14.1, we get:

[1530/1536] vnum: build/libns3-spectrum.so -> build/libns3-spectrum.so.1401

Is this what we want?
Comment 7 Vedran Miletić 2012-01-04 08:20:53 EST
Works for me, but looks weird; in this case I would really prefer something like

libns3-core.so.3.14.1

(this is the way boost does it), or even

libns3-core.so.14.1

in case 3 will not change. If waf can do it, it would be great. If not, we can perhaps file a bug on it as well.

From what I read (e.g. in [1]), strcmp is used, so this solution would work, and it would allow us to ship 3.14 with .so.3.14.0, and then release 3.14.1. If it turns out to be binary compatible with 3.14, we don't need to bump that .0 to .1.

[1] http://lists.fedoraproject.org/pipermail/devel/2011-June/153145.html
Comment 8 Gustavo J. A. M. Carneiro 2012-01-04 08:39:17 EST
(In reply to comment #7)
> Works for me, but looks weird; in this case I would really prefer something
> like
> 
> libns3-core.so.3.14.1
> 
> (this is the way boost does it), or even
> 
> libns3-core.so.14.1
> 
> in case 3 will not change. If waf can do it, it would be great. If not, we can
> perhaps file a bug on it as well.
> 
> From what I read (e.g. in [1]), strcmp is used, so this solution would work,
> and it would allow us to ship 3.14 with .so.3.14.0, and then release 3.14.1. If
> it turns out to be binary compatible with 3.14, we don't need to bump that .0
> to .1.
> 
> [1] http://lists.fedoraproject.org/pipermail/devel/2011-June/153145.html

If I understand correctly, if we have installed both libns3-core.so.14.0 and libns3-core.so.14.1, then both these libraries are supposed to be binary compatible.  But ns 3.14.1 does not promise to be binary compatible to 3.14.0.  The '14' component in libns3-core.so.14.1 is the number that indicates the binary compatibility version, and it is the same in both cases.  We are saying that both libraries are compatible, but that is a lie.

Waf does support dotted version numbers, we just have to change this line in the patch:
    wutils.VNUM = "%i.%i" % (MINOR, MICRO)

But if we can't promise binary compatibility, then this would be wrong...
Comment 9 Vedran Miletić 2012-01-04 09:24:52 EST
From what I understand, if strcmp is used for SONAME, it doesn't matter how many minor versions you have. But I might be wrong here.

If I'm correct, then .so.14.1 and .so.14.0 are not binary compatible and can coexist nicely (assuming we don't install symlinks to .so and .so.14).

And I'm not sure that you are correct on waf (sorry, I can't check now); if I recall correctly, waf will only take first number in VNUM for SONAME, and the rest will only be used for file name.
Comment 10 Gustavo J. A. M. Carneiro 2012-01-04 10:30:55 EST
(In reply to comment #9)
> From what I understand, if strcmp is used for SONAME, it doesn't matter how
> many minor versions you have. But I might be wrong here.
> 
> If I'm correct, then .so.14.1 and .so.14.0 are not binary compatible and can
> coexist nicely (assuming we don't install symlinks to .so and .so.14).
> 
> And I'm not sure that you are correct on waf (sorry, I can't check now); if I
> recall correctly, waf will only take first number in VNUM for SONAME, and the
> rest will only be used for file name.

OK, I won't claim to really understand much about the version numbers, in that case.  But, is this the final result that you want?  If so, I have the code ready to do it, I just need to commit it:

libns3-wimax.so -> libns3-wimax.so.14.1
libns3-wimax.so.14 -> libns3-wimax.so.14.1
libns3-wimax.so.14.1

However, I see a potential problem.  I don't think it becomes easy to link a program or module to a specific ns-3 version, this way.  If you add the -lns3-wimax link option, it just links to the latest ns-3 version installed.  I am not sure this is very good.

Maybe a better option is to just forget the VNUM and encode the version into the library name?  Alternatively, create a subdirectory for ns-3 libraries for each separate version?
Comment 11 Vedran Miletić 2012-01-04 10:54:42 EST
(In reply to comment #10)
> Maybe a better option is to just forget the VNUM and encode the version into
> the library name?  Alternatively, create a subdirectory for ns-3 libraries for
> each separate version?

It seems so indeed, so Mathieu was quite right when he suggested this approach.

We have couple of options for library names and SONAMEs:
1) libns3-core-3.14.1.so (and libns3-core-3.14.so, no .0 neeeded)
2) libns3.14.1-core.so
3) libns3-core-14.1.so

I vote for option 1). With debug/release in the name (bug 1328), it would become libns3-core-3.14.1-debug.so and libns3-core-3.14.1-release.so which isn't short, but it's logical.
Comment 12 Gustavo J. A. M. Carneiro 2012-01-04 11:00:39 EST
(In reply to comment #11)
> (In reply to comment #10)
> > Maybe a better option is to just forget the VNUM and encode the version into
> > the library name?  Alternatively, create a subdirectory for ns-3 libraries for
> > each separate version?
> 
> It seems so indeed, so Mathieu was quite right when he suggested this approach.
> 
> We have couple of options for library names and SONAMEs:
> 1) libns3-core-3.14.1.so (and libns3-core-3.14.so, no .0 neeeded)
> 2) libns3.14.1-core.so
> 3) libns3-core-14.1.so
> 
> I vote for option 1). With debug/release in the name (bug 1328), it would
> become libns3-core-3.14.1-debug.so and libns3-core-3.14.1-release.so which
> isn't short, but it's logical.

I like 2) better, personally.  It's shorter and logical.  libns3.14.1-debug.so.  But 1) is not bad either.
Comment 13 Vedran Miletić 2012-01-04 11:22:59 EST
I'm OK with 2) as well, but with debug/release after module name, i.e.

libns3.14.1-core-debug.so and libns3.14.1-core-release.so

not

libns3.14.1-debug-core.so and libns3.14.1-release-core.so.
Comment 14 Gustavo J. A. M. Carneiro 2012-01-06 14:03:33 EST
Created attachment 1297 [details]
patchy to version the library file name and pcfiles

This changes the file names to libnsVERSION-module-PROFILE.so and libnsVERSION-module-PROFILE.pc, where PROFILE is the build profile, e.g. debug/optimized.
Comment 15 Vedran Miletić 2012-01-07 06:39:20 EST
Great work, almost perfect. Two things that are bothering me:
1) PROFILE is always debug, e.g.

$ ./waf configure -d optimized --prefix=~/whatever

gives

$ ls build/
(...)
libns3-dev-*-debug.so
(...)

and

$ readelf -a build/libns3-dev-core-debug.so | grep SONAME 
 0x000000000000000e (SONAME)             Library soname: [libns3-dev-core-debug.so]

By the way, I like 3-dev stuff. I hope it's intentional. Please keep it.

2) Name and description is wrong with .pc files other than core, e.g.

$ cat build/src/network/libns3-dev-network-debug.pc 
prefix=/home/vedranm/whatever
libdir=/home/vedranm/whatever/lib
includedir=/home/vedranm/whatever/include

Name: libcore
Description: ns-3 module core
Version: devel
Libs: -L${libdir} -Wl,-Bdynamic -lns3-network
Cflags: -I${includedir}
Requires: libns3-dev-core-debug
Comment 16 Vedran Miletić 2012-01-07 06:49:35 EST
And while at it, it would be awesome if you can also add versioning of include path along with library versioning and fix .pc file accordingly. So, instead of $INCLUDEPATH/ns3 we should have $INCLUDEPATH/ns3.14/ns3 (or $INCLUDEPATH/ns3-dev/ns3).
Comment 17 Gustavo J. A. M. Carneiro 2012-01-08 11:24:52 EST
Created attachment 1299 [details]
fixed patch

Thanks for testing, this patch should be better now.
Yes, the libns-3-dev-xxx is kind of intentional, I mean, I noticed it, and why not? So I suggest we leave it like this.
Comment 18 Vedran Miletić 2012-01-08 12:17:04 EST
Just tested it, works great.

Should I file a separate bug for include-related stuff?
Comment 19 Gustavo J. A. M. Carneiro 2012-01-08 12:20:03 EST
Created attachment 1300 [details]
patch v3

This patch adds versioning to installed headers and programs.
Comment 20 Vedran Miletić 2012-01-08 14:42:52 EST
Almost perfect, two things:
1) I vote for ns3-dev-bench-packets-optimized instead of ns3-dev-optimized-bench-packets to be consistent with library naming.
2) Also, on a similar note, I vote for include/ns3-dev instead of include/ns-3-dev (even though both conventions are used by libs) so if you prefer to do dashed one it's also good.
3) And a bug; I get this:
$ ls include/
ns3  ns-3-dev
$ ls include/ns-3-dev/ns3/
aodv-module.h          network-module.h
applications-module.h  nix-vector-routing-module.h
bridge-module.h        olsr-module.h
click-module.h         openflow-module.h
config-store-module.h  point-to-point-layout-module.h
core-module.h          point-to-point-module.h
csma-layout-module.h   propagation-module.h
csma-module.h          spectrum-module.h
dsdv-module.h          stats-module.h
emu-module.h           tap-bridge-module.h
energy-module.h        tools-module.h
flow-monitor-module.h  topology-read-module.h
internet-module.h      uan-module.h
lte-module.h           virtual-net-device-module.h
mesh-module.h          visualizer-module.h
mobility-module.h      wifi-module.h
mpi-module.h           wimax-module.h
netanim-module.h

Everything else is in include/ns3.
Comment 21 Vedran Miletić 2012-01-10 12:41:22 EST
Gustavo, I really don't want to pressure you, but is there any chance that we could solve these things tomorrow or the day after? I would prepare RPMs and specs immediately following your patch, as I have a group of students working with ns-3 for the first time this friday, and I would love to use new naming conventions if possible.
Comment 22 Vedran Miletić 2012-01-10 12:42:48 EST
Just to add: there is absolutely no rush to get patch committed to Mercurial repo, as I can patch the source manually.
Comment 23 Gustavo J. A. M. Carneiro 2012-01-10 12:49:55 EST
(In reply to comment #21)
> Gustavo, I really don't want to pressure you, but is there any chance that we
> could solve these things tomorrow or the day after? I would prepare RPMs and
> specs immediately following your patch, as I have a group of students working
> with ns-3 for the first time this friday, and I would love to use new naming
> conventions if possible.

Things are looking bad on my side, too little time. I just have my day work to do, plus a couple of papers to review for yesterday. :(

Though not a lot of work is left, I think.  If you are in a real hurry, I bet you can do some tweaks on my patch to fix the remaining issues!... ;-) Hint: look for bld.install_files(...) calls in src/wscript.
Comment 24 Vedran Miletić 2012-01-10 13:31:44 EST
Thanks Gustavo, all done. Will post a patch soon.
Comment 25 Vedran Miletić 2012-01-10 13:54:31 EST
Created attachment 1302 [details]
Modified Gustavo's patch

Here it is.
Comment 26 Vedran Miletić 2012-01-10 14:48:15 EST
*** Bug 1328 has been marked as a duplicate of this bug. ***
Comment 27 Vedran Miletić 2012-01-10 16:27:11 EST
Created attachment 1303 [details]
Modified Gustavo's patch version 2

Updated patch, previous one is broken.
Comment 28 Gustavo J. A. M. Carneiro 2012-01-11 09:35:01 EST
Thanks, looks great.

changeset:   7681:0873edb445e8
tag:         tip
user:        Gustavo Carneiro / Vedran Miletić
date:        Wed Jan 11 14:34:14 2012 +0000
summary:     Bug 1327 - Version installed ns-3 files
Comment 29 Tom Henderson 2012-01-11 12:41:14 EST
the latest patch breaks test.py, which expects test-runner and the executables to be named in the old fashion.

Mitch and I will work on a change to test.py to make it smarter as a result, but just wanted to reopen this to alert people that this problem is recognized and being worked on.
Comment 30 Gustavo J. A. M. Carneiro 2012-01-11 13:13:16 EST
(In reply to comment #29)
> the latest patch breaks test.py, which expects test-runner and the executables
> to be named in the old fashion.

Apologies, I forgot about test.py.
Comment 31 Vedran Miletić 2012-01-11 14:30:45 EST
Apologies from me as well, I have tried only the functionality I need for making RPMs.
Comment 32 Vedran Miletić 2012-01-11 14:32:42 EST
Also, we forgot to disable VNUM altogether. Patch in a second.
Comment 33 Vedran Miletić 2012-01-11 14:36:26 EST
Created attachment 1305 [details]
Patch addendum (disable VNUM)

Patch disables VNUM and provides reference to this bug for explanation.
Comment 34 Vedran Miletić 2012-01-11 14:42:26 EST
Created attachment 1306 [details]
Patch addendum 2 (name -l arguments properly in generated .pc files)

And one more; this was actually supposed to be in patch version 2 I uploaded, but for some reason (either a failure on my or on Bugzilla's side) version 2 == version 1. Sorry for that as well.
Comment 35 Vedran Miletić 2012-01-16 03:17:58 EST
Gustavo, any comments on these patches?
Comment 36 Gustavo J. A. M. Carneiro 2012-01-16 11:04:38 EST
(In reply to comment #35)
> Gustavo, any comments on these patches?

I'm fine with both patches.
Comment 37 Vedran Miletić 2012-01-16 14:07:07 EST
Can they be checked in or we need more review?
Comment 38 Gustavo J. A. M. Carneiro 2012-01-16 14:42:04 EST
(In reply to comment #37)
> Can they be checked in or we need more review?

Yes.


changeset:   7688:6f5c9b870e45
tag:         tip
user:        Vedran Miletić
date:        Mon Jan 16 19:39:45 2012 +0000
summary:     Still Bug 1327: disable VNUM in libraries, fix the .pc file deps
Comment 39 Vedran Miletić 2012-01-16 14:53:33 EST
Thanks.
Comment 40 YunQiang Su 2012-06-13 09:58:47 EDT
As we named .so as libns3.14.1-core.so, 
it is not the same library with libns3.14.2-core.so at all.
So, no "compatible" can be break.

Then why not give it an VNUM to meet LSB?
such as  libns3.14.1-core.so.0.0.0
Comment 41 Gustavo J. A. M. Carneiro 2012-06-13 10:29:19 EDT
(In reply to comment #40)
> As we named .so as libns3.14.1-core.so, 
> it is not the same library with libns3.14.2-core.so at all.
> So, no "compatible" can be break.
> 
> Then why not give it an VNUM to meet LSB?
> such as  libns3.14.1-core.so.0.0.0

To be clear, the VNUM would have to be hardcoded 0.0.0, unrelated to the ns-3 release.  It seems a bit silly, what does it accomplish?
Comment 42 Vedran Miletić 2012-06-13 11:04:54 EDT
Gustavo, we specifically made ns-3 take into account minor revision numbers because Tom said there is no waranty about binary compatibility. So, I don't see a problem here and vote to leave it as it is.