Re: new module: relpath

[Akim Demaille]

Le 16 juin 2012 à 17:25, Bruno Haible a écrit :

> Hi Akim,

Hi Bruno,

Thanks for the detailed comments.

> About the module name 'relpath': The GNU standards, section "GNU Manuals",
> say
>   Please do not use the term "pathname" that is used in Unix
>   documentation; use "file name" (two words) instead.  We use the term
>   "path" only for search paths, which are lists of directory names.
> 
> Strictly speaking this is only about manuals, not about code. But the
> more we use the term "file name" also in code, the less we are tempted
> to misuse the term "path" in documentation. Gnulib already has modules
>  concat-filename
>  filename
>  filenamecat
> It would be good to choose a module name in this spirit.

rel-filename? filename-relative?  Tell me what you prefer.  I
like naming related things with a common prefix, so I would be
happy with filename-relative.

>> * lib/relpath.c: New.
>> * lib/relpath.h: New.
>> * modules/relpath: New.
> 
> When you contribute a gnulib module, it's also welcome to write a unit
> test: 2 files tests/test-relpath.c and modules/relpath-tests.

I'll see what I can do.

> 
>> +License:
>> +GPLv3+
> 
> Please write 'GPL' here. It is semantically equivalent to 'GPLv3+', but
> gnulib-tool does not understand 'GPLv3+'.

OK.

>> +++ b/modules/relpath
> 
>> +Depends-on:
>> +canonicalize
>> +dirname
>> +error
>> +pathmax
>> +stdbool
>> +xalloc
>> +...
>> +License:
>> +GPLv3+
> 
> This module may produce error messages and be under GPL. Then the module
> is not usable in shared libraries. If you want a module that is more widely
> usable, consider an interface that returns error codes  or error strings,
> remove the dependencies to 'error' and 'xalloc', and change the license
> to 'LGPL'.

I don't feel entitled to change the license :(  That's the license the
code was under in coreutils.

>> +/* Return FROM represented as relative to the dir of TARGET.
>> +   The result is malloced.  */
>> +
>> +char *
>> +convert_abs_rel (const char *from, const char *target);
> 
> It is a bit strange here that if I want to compute a relative filename,
> relative to a given directory, I will have to add "/." to this TARGET
> directory. How about either
>  - adding a second function that takes a directory as second argument
>    (as opposed to a file in this directory), or
>  - change the specification of convert_abs_rel in this way, outright?
>    (Then many callers will have to perform a dirname() themselves.)

OK.

> 
>> +#include <config.h>
>> +
>> +#include <stdbool.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <errno.h>
>> +
>> +#include "gettext.h"
>> +#define _(msgid) gettext (msgid)
>> +
>> +#include "canonicalize.h"
>> +#include "dirname.h"
>> +#include "error.h"
>> +#include "relpath.h"
>> +#include "xalloc.h"
> 
> By convention, the specification header ("relpath.h" in this case) should
> be included right after <config.h>. This helps verifying that the include
> file is self-contained, i.e. does not need a prerequisite of <sys/types.h>
> or similar.

Nice trick, thanks!