Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

Remove log4tango from tango.pc & makefiles; add dummy liblog4tango.so #33

Conversation

mliszcz
Copy link

@mliszcz mliszcz commented May 17, 2019

As discussed during meeting at ESRF, following changes has to be made after log4tango has been removed (#30):

  • assets/lib/cpp/tango.pc.in - remove log4tango as tango dependency from pkg-config file. This file is used by templates generated by Pogo on linux
  • assets/cppserver/*/Makefile.am - remove -llog4tango linker option from all device servers. NOTE: in each device server source repository there is another makefile which should be aligned as well (but this is out of scope of this change)
  • provide dummy liblog4tango.so that exports no symbols at all but is left in 9.3.3 for backwards compatibility. It shall be removed in 9.4.0.

I've tried to install tango with these changes in an empty docker container (so there were no liblog4tango from previous installation). It compiles and installs fine.

Such libraries are generated:

root@472df231a8e6:/tango/tango-9.3.3# tree /prefix/lib/
/prefix/lib/
|-- liblog4tango.a
|-- liblog4tango.la
|-- liblog4tango.so -> liblog4tango.so.5.0.1
|-- liblog4tango.so.5 -> liblog4tango.so.5.0.1
|-- liblog4tango.so.5.0.1
|-- libtango.a
|-- libtango.la
|-- libtango.so -> libtango.so.9.3.3
|-- libtango.so.9 -> libtango.so.9.3.3
|-- libtango.so.9.3.3
`-- pkgconfig
    |-- log4tango.pc
    `-- tango.pc

1 directory, 12 files
root@472df231a8e6:/tango/tango-9.3.3# nm -g /prefix/lib/libtango.so.9.3.3 | grep -i log4tango  | wc -l
267
root@472df231a8e6:/tango/tango-9.3.3# nm -g /prefix/lib/liblog4tango.so.5.0.1 | grep -i log4tango | wc -l
0

Swapping liblog4tango with a dummy one works without a need to recompile the program:

$ g++ -I/usr/local/include/tango main.cpp -ltango -llog4tango -lomniDynamic4 -lCOS4 -lomniORB4 -lomnithread -lzmq; echo $?
0

$ ./a.out; echo $?
0

$ sudo rm /usr/local/lib/liblog4tango.*

$ ./a.out; echo $?
./a.out: error while loading shared libraries: liblog4tango.so.5: cannot open shared object file: No such file or directory
127

$ sudo cp ~/TangoSourceDistribution/build/lib/liblog4tango.* /usr/local/lib/

$ ./a.out; echo $?
./a.out: symbol lookup error: /usr/local/lib/libtango.so.9: undefined symbol: _ZTIN9log4tango14LayoutAppenderE
127

$ sudo rm /usr/local/lib/libtango.*
$ sudo cp ~/TangoSourceDistribution/build/lib/libtango.* /usr/local/lib/

$ ./a.out; echo $?
0

@bourtemb
Copy link
Member

@mliszcz , thanks for the work done!
I think the empty log4tango library should not have the same revision number as the one distributed in tango 9.2.5a source distribution (5.0.1).

@mliszcz
Copy link
Author

mliszcz commented May 17, 2019

What number should we use then? 5.0.2, 5.1.0 or 6.0.0? I'm not sure if any semantic versioning rules cover such a scenario.

@bourtemb
Copy link
Member

I would use 5.0.2, just to avoid to overwrite a previous version which could have been already there in the install folder. This is just for a specific use case (the user wants to install in a folder where there is already log4tango.so.5.0.1 and does not want the previous version to be overwritten).

@mliszcz mliszcz force-pushed the fix-32-remove-log4tango-from-tango-pc branch from adbea80 to 089b9f9 Compare May 19, 2019 08:12
@mliszcz
Copy link
Author

mliszcz commented May 19, 2019

Thanks @bourtemb . liblog4tango version is specified in kernel repository, which is already tagged for 9.3.3 release. I'm updating the version with sed+autoconf in build.xml after kernel checkout.

@@ -76,6 +76,13 @@
<arg value="-B${workdir}/cpp/cmake-build"/>
<arg value="-DIDL_BASE=${workdir}/idl/install"/>
</exec>
<exec executable="sed" failonerror="true" failifexecutionfails="true">
<arg value="-i"/>
<arg value="1s/5\.0\.1/5.0.2/;18s/5:1:0/5:2:0/"/>
Copy link
Member

Choose a reason for hiding this comment

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

@mliszcz, I am not an expert in the usage of sed and I'm curious...
What is the advantage of adding '1' and '18' in front of 's/' on this line?
What does it do?

Copy link
Author

Choose a reason for hiding this comment

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

The number (1 or 18) shows the lines for which the operation (s=substitute) applies. I just wanted to prevent accidental modifications from other lines (just in case "5.0.1" would be used in a different context).

Please check: https://www.gnu.org/software/sed/manual/sed.html#sed-script-overview

[addr]X[options]

X is a single-letter sed command. [addr] is an optional line address. If [addr] is specified, the command X will be executed only on the matched lines. [addr] can be a single line number, a regular expression, or a range of lines (see sed addresses).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanations.

Copy link
Member

@bourtemb bourtemb left a comment

Choose a reason for hiding this comment

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

Looks good to me.
I've done simple tests and it seems to work as expected.
Thanks!

@@ -76,6 +76,13 @@
<arg value="-B${workdir}/cpp/cmake-build"/>
<arg value="-DIDL_BASE=${workdir}/idl/install"/>
</exec>
<exec executable="sed" failonerror="true" failifexecutionfails="true">
<arg value="-i"/>
<arg value="1s/5\.0\.1/5.0.2/;18s/5:1:0/5:2:0/"/>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanations.

@bourtemb bourtemb merged commit 9d9883f into tango-controls:prepare-9.3.3 May 24, 2019
bourtemb added a commit that referenced this pull request Jun 5, 2019
* Update cpptango to release 9.3.3

* Update assets/README...
... to show changes between 9.2.5a and 9.3.3

* Update assets/TANGO_CHANGES for 9.3.3
Minimal text explanation in this version.

* Update TangoAccessControl to Release 2.14

* Update to latest available release versions
Update AtkPanel to      Release 5.8
Update JSSHTerminal to  Release 1.13
Update tango_admin to   Release 1.14

* Fix missing separator error in common_target.opt

Fix the following error (a tab was missing):
common_target.opt:166: *** missing separator.

* Update build.xml
Use copy ant task instead of deprecated copydir task

* Cleanup POGO (#20)

Remove obsolete POGO templates
Remove pogo-6

* Update astor to release 7.2.5

* rename atkpanel to ATKPanel in java applications (fixes #25) (#27)

* Pass correct arguments to TangoRestServer (fixes #24) (#26)

* Remove zmq.hpp (#23) (#28)

* Add logback configuration file (#21) (#29)

* Merge liblog4tango into libtango (change to convenience lib) (#30) (#31)

Change log4tango into a libtool convenience library.
liblog4tango.so is no longer generated, but log4tango objects are still compiled and are now linked into libtango.so

* Update README

* Remove my.cnf config file (tango-controls/TangoDatabase#16) (#35)

* Remove log4tango from tango.pc & makefiles; add dummy liblog4tango.so (#33)

* Add CMake opt files (#34) (#36)

Add cmake_common_target.opt and cmake_tango.opt to the distribution
in order to be able to generate CMakeLists.txt files for Device
Servers from Pogo.

* Compile with -std=c++0x when using g++ < 6.1

* Update to latest available release versions (#37)

Update to latest available release versions
Update Jive to          Release 7.22
Update ATK to           Release 9.3.6
Update Pogo to          Release 9.6.23
Update rest-server to   Release 1.14
Update TangoDatabase to Release 5.11
Update JTango to 9.5.14
@bourtemb bourtemb mentioned this pull request Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants