taskGNU Astronomy Utilities - Tasks: task #13765, Convert static libraries to shared...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #13765: Convert static libraries to shared libraries

Submitter:  Mohammad Akhlaghi <makhlaghi>
Submitted:  Thu 08 Oct 2015 01:10:29 AM UTC
   
 
Should Start On:  Wed 07 Oct 2015 03:00:00 PM UTC Should be Finished on:  Wed 07 Oct 2015 03:00:00 PM UTC
Category:  Libraries Priority:  5 - Normal
Item Group:  Enhancement Status:  Done
Privacy:  Public Assigned to:  makhlaghi
Percent Complete:  100% Open/Closed:  Closed
Effort:  0.00

Jump to the original submission

Fri 23 Sep 2016 11:19:46 AM UTC, comment #22: 

All the existing functions in the libraries have now been documented in the book and the documentation has been pushed to the main repository. So I am marking this task as complete and would again like to greatly thank Mosè for his hard work in re-naming all the exported Macros, structures, types and functions into a unified naming convention.

Ofcourse, as I have tried to emphasize in several parts of the book, this is just the first version of the libraries. They will undergo great evolution during the next few releases. Interestingly enough, there are actually practicaly no astronomical operations in them, all the interesting functions are currently in the utilities, not in the libraries.

In time, we will bring them out into the libraries to make Gnuastro's libraries much more useful. Until now the emphasis has been on building the infra-structure. Now that a mostly robust structure is in place, it is really easy to add new functions and headers for the libraries.

So I am now preparing a alpha-release and will release it by tonight. Hopefully by next week (September 30th) we can make the second (version 0.2) release. So please try it out and share your experience for a smooth 0.2 release ;-).

Mohammad Akhlaghi <makhlaghi>
Group administrator
Mon 12 Sep 2016 01:42:21 PM UTC, comment #21: 

Library development (mainly documentation) is ongoing on the master branch of the main repository.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 04 Sep 2016 12:09:26 AM UTC, comment #20: 

I have started work on the documentation of the libraries. You can follow the progress on my personal repository's libs-doc-tests branch. If you get to have a look at the explanations, any comments or suggestions would be great. While writing the documentation probably some more tests will be added.

Hopefully once a first version of the "Libraries" chapter is written (along with more tests), I think Gnuastro will be ready for an alpha-release. Soon after that, we can release version 0.2, ideally before October, but depending on the progress.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sat 03 Sep 2016 04:34:43 PM UTC, comment #19: 

A new library function `gal_gnuastro_version' was added, so we can check the version of Gnuastro within a program that uses Gnuastro's libraries. A small test was also written for it and we can use it as a template to add more library checks.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Tue 30 Aug 2016 09:32:55 AM UTC, comment #18: 

Ok, I now see the warning about the library non installed (it isn't emphasized in the terminal, that's why I missed it).

Mosè Giordano <giordano>
Group Member
Sun 28 Aug 2016 11:24:04 PM UTC, comment #17: 

Sorry, I made a few bad typos: the "error"s should have been "warning" and this line

"... are pushed to stdout (as they should be) ..."

should have been

"... are pushed to stdERR (as they should be) ..."

Sorry ;-).

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 28 Aug 2016 11:20:21 PM UTC, comment #16: 

Thanks for the check, I am very happy it has installed with no problem. It is interesting that you have not seen any errors.

In the previous test, I had not bootstrapped, so this time I cloned Gnuastro in a new directory and bootstrapped it, then ran `make' and `make distcheck'.

However, the libtool warnings still remain. You are right, the libtool error texts are printed in bold and the "warning" phrase is in red.

I am using libtool 2.4.6, with Gnulib v0.1-845-gbea6f3f.

To be perfectly sure you can try directing the `make distcheck' output to a file. If the libtool warnings are pushed to stdout (as they should be), they should print cleanly. Ultimately you can try searching in that file for "libtool: warning:" strings.

But if the warnings have indeed not come up on your system, it might be some configuration beyond Gnuastro (probably while building libtool. It would be interesting if we could find it and somehow include it in Gnuastro's building.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 28 Aug 2016 08:23:37 PM UTC, comment #15: 

Great!  I checked out the latest revision of Gnuastro, clean the directory (make distclean; git clean -fxd) and then built the whole library from scratch: I didn't see any warning.  I tried both "make check" and "make distcheck".  If I remember well, libtool warnings are colored (is it corrected?), otherwise I may have missed them (but I looked at the end of the outputs of all commands I ran).

Mosè Giordano <giordano>
Group Member
Sun 28 Aug 2016 12:54:53 AM UTC, comment #14: 

The library Makefile was updated to combine all the `*.c' sources into one library that is also installable. This update has been pushed to the main repo. See the commit message and diffs for the details.

During `make distcheck', close to the end, two kinds of warnings come up (I have changed the starting of the address to `/topdir/' for readability).

This one comes up once (after trying to install the library)


libtool: warning: remember to run 'libtool --finish /topdir/gnuastro/build/gnuastro-0.1.93-547e/_inst/lib'


and this one comes up after each utility:

+verbose+
libtool: warning: '/topdir/gnuastro/build/gnuastro-0.1.93-547e/_build/sub/lib/libgnuastro.la' has not been installed in '/topdir/gnuastro/build/gnuastro-0.1.93-547e/_inst/lib'
-verbatim-

These are just warnings, so `make distcheck' finished successfully. From this 2006 libtool mailing list post and the reply to it, it appears that this is an expected effect when you change the installation directory after configuration (which `make distcheck' does in its tests). So there is no problem.

If there was something we could do to ignore these warnings it would be great, since other possible warnings would be more clearly seen. But for the time being we will have to accept these warning messages.

We also have to write some tests for the libraries. I personally checked some functions in `txtarray' and `linkedlist' in a tiny C program which I wrote in another directory and they worked. But tests are necessary and have to be added to be checked during `make check'.

As discussed in the commit message, this does not complete this task because the book also has to be updated (a full and probably long chapter has to be added explaining everything about using the libraries and documenting the functions and headers and etc).

But for the time being, this is commit and comment on Savannah are just one step of progress towards these goals. If you get the chance to install libraries and try them out it would be great to share your feedback ;-).

Mohammad Akhlaghi <makhlaghi>
Group administrator
Fri 03 Jun 2016 11:39:16 AM UTC, comment #13: 

Two commits were just pushed to master that were not directly related to this task but still relevant: the two libraries related to txt and fits files were renamed to `txtarray' (from `txtarrayvv') and `fits' (from `fitsarrayvv'). In the latter case, the `_fits_' string would appear two times (redundantly) in some function names, so such functions were renamed to satisfy the new library file name.

The commit body thoroughly explains the reasons behind this rename, in short this is a summary:

  • During the work on renaming the functions, the string for the filename for these two libraries didn't have the `vv's (which was a very good move). So to avoid confusion, it was also necessary for the `vv's to be removed from the file names too.


  • The `array' in the fits library was an ancient artifact which was no longer correct (since the functions in this library now do a large variety of operations on FITS files, not just array operations).


  • Functions that were purely using WCS (and not related to FITS images) were moved to a new library called wcs.
Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 29 May 2016 12:22:26 PM UTC, comment #12: 

The "refactor" branch was merged into master and is now on the main repository. Thank you very much for the hard work.

I had initially decided to leave the merging of this branch into master after the 0.1 release. However, I recognized that any further modifications to the file/function names (as mentioned in the previous comment) can also be done after the merge! My mind was too focused on all the other more important aspects at that time. Infact changing the names would be easier because of the prefixes. For the time being all these changes are internal, and the libraries aren't installed, so they don't need to be documented in the manual as library functions. Finally, if this standard is included in the 0.1 release of Gnuastro, interested developers will start using it from the beginning and there will be less friction later.

The standard place to install the header files is Automake's '$(pkgincludedir)', which is `$(includedir)/$(PACKAGE)', where $(PACKAGE) is `gnuastro'. So to be standard, the directory where all the headers should be kept must be named `gnuastro'. In order to make the utility source codes similar to what a user would write in their own programs, all the headers were moved to the `lib/gnuastro' directory (previously they were in `include/'). Also, the `-I$(top_srcdir)/lib' directory replaced the previous `-I$(top_srcdir)/include' directory in the `Makefile.am's.

Another issue was that header files depend on each other (for example `commonargs.h' currently depends on 'checkset.h', 'commonparams.h' and 'fixedstringmacros.h'). After checking, I noticed that GSL's headers also depend on each other (for example `eigen/gsl_eigen.h' includes `gsl/gsl_vector.h' and `gsl/gsl_matrix.h'). So if a user does really intend to not have a `gnuastro' (or 'gsl') directory in their installed 'include/' directory, programs that include such headers from Gnuastro (or GSL) will not be able to find the headers and crash. It might be possible to define a `./configure' option to allow the user to specify a macro to use as directory, but I feel that would be too much, and would be hardly ever used!

Since that hypothetical situation cannot be used in Gnuastro (or GSL), and since unlike GSL, we have no technical necessity to have this filename convention, I decided to remove the prefix for the header file names. The directory keeping the header file names is also now `gnuastro', so it is very unlikely to have have an overlap with other packages (at least in file names). However, the `gal_' prefix for the installed functions, macros and variables still remains (since `gnuastro_' would be too long! It will also be very clear and non-cryptic if someone is looking into their `$prefix/include' directory.

After the 0.1 release (in the next day or two), we can now start making the libraries installable.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sat 30 Apr 2016 11:00:15 AM UTC, comment #11: 

As discussed before, nearly all the libraries were initially part of a single utility. When they were needed by more than one utility, they were moved to the lib/ directory. So various issues about them are not perfectly ideal for a library: for example filenames, function names, even the function operations (they might be more useful merged with each other or further modularized).

The main issue now (names of the functions) is that the names were not necessarily the best possible. So be going through the libraries (file and function names), we can choose better names, to be both descriptive and make a good over-all function name. Applying the definition discussed below immediately will make some bad names (very long sometimes). While the gal_libraryfilename_function_name can be very useful to find the actual file and function, but such long names will make the code harder to read. So we have to choose good libraryfilename and functionname that can fit nicely with this format, while also being descriptive.

So I suggest to start implementing the library creation slowly and one by one, first with the smaller libraries (with fewer functions) to check our library models in practice and test them before implementing them over the full package. If we find out that any of our choices were not ideal, we can then change them easily.


Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 08 Nov 2015 02:08:39 AM UTC, comment #10: 

The use-case you mentioned is indeed valid and does allow for higher customization. So lets stick to the GSL guidelines. Thanks for clarifying the issue.

This idea just came to me since unlike GSL in the current design, we don't have any operational necessity for this naming convention in the current way the libraries are organized.

Since having a "gal/" directory filled with files starting with "gal_" is highly redundant and we have decided to keep the "gal_" in the header file names, we can move all the header files to the same folder as the libraries.

This will make the coding much more easier since the library .c and .h files will be in the same directory and are often necessary to be open together. At build time, we can use GSL's method of copying them all in a final directory before building/installing the utilities.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 08 Nov 2015 01:08:38 AM UTC, comment #9: 

I agree we shouldn't implement other conventions uncritically, for example I like how the files are organized in Gnuastro (and this is different from the GSL project), but I think having the prefix in the base name of the header files makes sense.

Someone may want, for some reasons, to include the header files without the directory name (for example by  adding the "gal/" directory to the list of directories searched by the compiler with the "-I" flag, or simply installing the headers in /usr/include), but having header files with somewhat generic names might cause problems.  The prefix "gal_" for all header files would limit them, and in addition would identify the headers as part of the GNU Astronomy Library even without looking at the directory they're in ;-)

Not installing the headers in a "gal/" directory wouldn't be the default behavior and one should do that at own risk (for example the GSL manual doesn't even mention this possibility, still they have the prefix "gsl_" for the headers), but I don't think we should prevent this either.

This is the reason why I suggest to have a prefix also in the file base name, but I leave the final decision to you :-)

Mosè Giordano <giordano>
Group Member
Sun 08 Nov 2015 12:37:01 AM UTC, comment #8: 

I have still not mastered the markup guidelines here! I had put an asterisk to mark wildcards in the shell. But the markup took each pair of them as the start (and end) of a bold section! So when ever you see the bold test start and end, there was an asterisk!

Sorry for that ;-)!!!

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 08 Nov 2015 12:31:59 AM UTC, comment #7: 

While GSL's guidelines are very useful and we are learning from them, we should not implement them un-critically. As discussed below, the library header filename convention of GSL is not useful in Gnuastro's design for example.

The library header files in GSL have the following naming convention: "gsl/gsl_specific_name.h".

The reason for this is that the "gsl_*.h" files are defined in the same folders as the .c and other .h files for each sub-library directory. When you look at the source code, you will see that the "gsl" directory only contains Makefiles. The Makefile will search all the directories and put a copy of all the "gsl_*.h" files there to install later. There are other *.h files in each directory that are not installed, so this naming convention is completely design-based: based on the fact that the libraries are completely independent entities.

But Gnuastro's source code is designed in another fashion. We have both executables and libraries where the libraries are made first and then the executables use those libraries. Therefore we have a separate "include/" directory (later to be named "gal") that contains all the to-be-installed ".h" files from the beginning. So for us the prefix before the to-be-installed header files surves no purpose.

In other words, we don't need to hand-pick some files automatically at build time and thus set a specific filename convention to do that. They are already separated from the start.

So I suggest to drop this part of the GSL guidelines since it does not satisfy our design making this prefix is operationally redundant for us. It only makes it harder to find a file that you want in the "include/" (later "gal/") directory in the source code or after installation by lengthening the filenames. It also makes the inclusion commands at the top of source files unnecessarily long and possibly confusing: since GSL will often be used alongside Gnuastro and the only difference between "gsl" and "gal" is the middle letter!

So if my suggestion is agreed upon, Gnuastro's header files will be included with: "gal/specific_name.h".

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sat 07 Nov 2015 04:16:31 AM UTC, comment #6: 

Sorry, I meant to suggest renaming the "include" directory within Gnuastro to "gal" for the headers, not the "lib" directory.

The linkings can be different from a Gnuastro library user, since they are managed by a Makefile. But the source code which includes the header file should be identical.

With this correction, it might not be too hard to make a symbolic link named "gal" at build time (in the build directories) which will link to the "include" directory. In this way the code of the executables can satisfy the conditions explained before and the "include" directory can keep its name which is very descriptive for someone inspecting the code. Ofcourse this will make the unfamiliar reader a little confused because they will be looking for a "gal" directory but won't find it in the original source code. So it might be easier to just rename the "include" directory to "gal".

These are just very unmature suggestions and open to discussion, so please weigh in ;-).

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sat 07 Nov 2015 04:05:54 AM UTC, comment #5: 

This just occurred to me: following GSL, after the libraries are installed, the library headers should go in a "gal" directory of the installed libraries directory like GSL.

But in the actual Gnuastro programs, currently we are using them within the "lib" directory. So the library headers are called differently within the executable's codes and how a library user of Gnuastro would call them.

So we can either change the "lib" directory's name to "gal" so all the shared libraries can be built there and the different programs can link with them the same way a library user would use the libraries. Alternatively, we can make such a directory at build time and put all the shared libraries there, but I think the second approach might be a little more complicated in the Makefile.

It is best that the code of the executables in Gnuastro be exactly identical to how a Gnuastro user would want to use the libraries. In this way, the executables can be a good demonstration for the usage of the libraries.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sat 07 Nov 2015 03:39:44 AM UTC, comment #4: 

I vote for the "gal_" prefix.

Both because it is easier to pronounce (remember) and that it keeps the "library" part (It is also the first three letters of Galaxy, which are my research topic ;-)).

The "Utilities" part of the name was taken from GNU Coreutils that provides command line executables. So I used it to mean executables since they were what I had planned at first with one or two programs that didn't share too many common functions. As more common functions were created for the different executables, this idea of making them installable grew.

Fortunately the short name "Gnuastro" doesn't contain "Utilities", so it can be applied to both the utilities (executables in particular) and libraries. Once the libraries are also installed, we can redefine "utilities" to mean both executable and libraries.

Just as a brainstorm and radical alternative, we might also be able to change the third word in the official name to something that encompasses both executables and libraries. Since the short name (Gnuastro) doesn't contain the third word, none of the URLs or email addresses have to change. But this is just a very unmature suggestion, to think about for the long run...

Mohammad Akhlaghi <makhlaghi>
Group administrator
Wed 04 Nov 2015 11:40:52 AM UTC, comment #3: 

Thanks for the complete explanation Mosè, the GSL experience is indeed very useful and I agree with implementing it.

This concept of making shared libraries can also be expanded to nearly all functions in Gnuastro, not just those in lib/ (each Makefile can do the library making and installation separately).

Currently most of the functions that are not in the lib/ directory depend on the main.h structure of the particular program they are part of. If necessary, this dependancy can easily be removed. Each function in a program only needs a small set of those parameters. Infact this is how all the functions in the lib/ directory were created: whenever a function was found to be needed in more than one program, the argument was changed to only accept the variables it needed and it was placed in the lib/ directory.

This concept can even be extended to a whole program: The main progname function in progname.c can be installed as a library function so the users can write programs to mix the operations of separate programs in one program if they want to. In that case, we should think of what we should do with the main.h parameter structure. We can either completely remove it, or add it to the library header so the caller can simply fill in their values, but then using the structure in other languages might prove difficult, I am not sure.

The fact that all the sanity checks have been done in ui.c makes things easier in this case, since as a library, such checks should be done by the programmer that wants to call that function. But this is just a brainstorm now and unless there is demand, we don't have to invest too much time in it.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Wed 04 Nov 2015 11:21:00 AM UTC, comment #2: 

To complete this task, we should start using a namespace convention.  GSL uses this convention (https://www.gnu.org/software/gsl/design/gsl-design.html#SEC15):

  • Use gsl_ as a prefix for all exported functions and variables.


  • Use GSL_ as a prefix for all exported macros.


  • All exported header files should have a filename with the prefix gsl_.


  • All installed libraries should have a name like libgslhistogram.a


  • Any installed executables (utility programs etc) should have the prefix gsl- (with a hyphen, not an underscore).


  • All function names, variables, etc should be in lower case. Macros and preprocessor variables should be in upper case.


I think we can use all of these conventions, specifying in addition this rule for naming functions/variables: <prefix>_<file name>_<function/variable name>, using underscores to separate words in the last part of the name, as also suggested by GNU Coding Standards (https://www.gnu.org/prep/standards/standards.html#Names).  In this way one can easily guess that a function/variable comes from this library and from which particular file, without having to do a grep search in order to find where it's defined.

As per the prefix to use, I'd suggest either "gau" (Gnu Astronomy Utilities) or "gal" (Gnu Astronomy Library).  In order to keep separated the underlying libraries from the actual utilities).

The point of having a namespace prefix is to avoid clashes with other libraries (in addition to have a clearly identification of the origin of that function), so using the same prefix of other libraries would defeat this goal.  Doing a quick search on the Internet, I found no reference to "gau" and one reference to a CGAL library (https://github.com/CGAL/cgal), but they use "CGAL" as prefix, so there should be no problem for us to use "gal" as well.

Mosè Giordano <giordano>
Group Member
Thu 08 Oct 2015 01:14:04 AM UTC, comment #1: 

This issue is mentioned in the "Internal libraries" section of the manual, but I had forgot to list it here. I am grateful to Mosè for bringing up the issue and so I added it here so it can be tracked.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Thu 08 Oct 2015 01:10:29 AM UTC, original submission:  

There are many functions within the libraries that can be used by other programs. But currently the libraries are only linked statically while building and are not installed.

So if they are changed into shared libraries, more advanced users can use them in their own programs.

Mohammad Akhlaghi <makhlaghi>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by giordano (Posted a comment)
  • -email is unavailable- added by makhlaghi (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-09-23 makhlaghi StatusIn Progress Done
        Percent Complete50% 100%
        Assigned togiordano makhlaghi
        Open/ClosedOpen Closed
    2016-09-03 makhlaghi Percent Complete40% 50%
    2016-08-28 makhlaghi Percent Complete30% 40%
    2016-05-29 makhlaghi Percent Complete10% 30%
    2015-11-07 makhlaghi StatusPostponed In Progress
    2015-11-07 makhlaghi Assigned toNone giordano
    2015-11-05 giordano Percent Complete0% 10%

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code