Re: [Bug-tar] [PATCH] PAX format defaults to omitting PID

[Pavel Raiskup]

Hi Zachary, thanks for the patch.

On Monday 17 of August 2015 18:06:19 Zachary Vance wrote:
> The intent is to make binary equivalent PAX archives easy to create. If POS
> IXLY_CORRECT is set, the POSIX standard default is used, which embeds the p
> id.
> 
> * src/buffer.c (add_chunk_header): Change filenames of multipart files to o
> mit
>    the pid
> * src/xheader.c (xheader_xhdr_name, xheader_ghdr_name): Change default form
>    at
>    of PAX headers unless POSIXLY_CORRECT is set

Those lines are somehow broken in my mailbox -- maybe thats problem on my
side.  But you should use full sentences like

  '* some/file.c (topic): Full sentence and period.'

> Signed-off-by: Zachary Vance <address@hidden>
> ---
>  doc/tar.texi  | 34 ++++++++++++++++------------------
>  src/buffer.c  |  2 +-
>  src/xheader.c | 20 +++++++++++++++-----
>  3 files changed, 32 insertions(+), 24 deletions(-)
> 
> diff --git a/doc/tar.texi b/doc/tar.texi
> index 96f6aeb..747f2f1 100644
> --- a/doc/tar.texi
> +++ b/doc/tar.texi
> @@ -9975,7 +9975,7 @@ If no option @samp{exthdr.name=string} is specified, 
> @command{tar}
>  will use the following default value:
>  
>  @smallexample
> -%d/PaxHeaders.%p/%f
> +%d/PaxHeaders/%f
>  @end smallexample
>  
>  @item address@hidden
> @@ -10007,7 +10007,7 @@ If no option @samp{globexthdr.name=string} is 
> specified, @command{tar}
>  will use the following default value:
>  
>  @smallexample
> -$TMPDIR/GlobalHead.%p.%n
> +$TMPDIR/GlobalHead.%n
>  @end smallexample

In documentation, there is not mentioned the POSIXLY_CORRECT variable in
this context.  I'm not sure whether this is expected.

>  @noindent
> @@ -10074,7 +10074,7 @@ archives created using it, will be binary equivalent 
> if they have the
>  same contents:
>  
>  @smallexample
> ---pax-option=exthdr.name=%d/PaxHeaders/%f,atime:=0
> +--pax-option=atime:=0
>  @end smallexample
>  
>  @noindent
> @@ -10083,14 +10083,14 @@ from them, you will also need to eliminate changes 
> due to ctime, as
>  shown in examples below:
>  
>  @smallexample
> ---pax-option=exthdr.name=%d/PaxHeaders/%f,atime:=0,ctime:=0
> +--pax-option=atime:=0,ctime:=0
>  @end smallexample
>  
>  @noindent
>  or
>  
>  @smallexample
> ---pax-option=exthdr.name=%d/PaxHeaders/%f,atime:=0,delete=ctime
> +--pax-option=atime:=0,delete=ctime
>  @end smallexample
>  
>  @node Checksumming
> @@ -10216,7 +10216,7 @@ GNU extensions.  More specifically, the very first 
> part retains its
>  original name, and all subsequent parts are named using the pattern:
>  
>  @smallexample
> -%d/GNUFileParts.%p/%f.%n
> +%d/GNUFileParts/%f.%n
>  @end smallexample
>  
>  @noindent
> @@ -10229,19 +10229,17 @@ have the following meaning:
>  result of the @command{dirname} utility on its full name.
>  @item %f @tab  The file name of the file, equivalent to the result
>  of the @command{basename} utility on its full name.
> address@hidden %p @tab  The process @acronym{ID} of the @command{tar} process 
> that
>  created the archive.
>  @item %n @tab  Ordinal number of this particular part.
>  @end multitable
>  
>  For example, if the file @file{var/longfile} was split during archive
> -creation between three volumes, and the creator @command{tar} process
> -had process @acronym{ID} @samp{27962}, then the member names will be:
> +creation between three volumes then the member names will be:
>  
>  @smallexample
>  var/longfile
> -var/GNUFileParts.27962/longfile.1
> -var/GNUFileParts.27962/longfile.2
> +var/GNUFileParts/longfile.1
> +var/GNUFileParts/longfile.2
>  @end smallexample
>  
>  When you extract your archive using a third-party @command{tar}, these
> @@ -10252,9 +10250,9 @@ the proper order, for example:
>  @smallexample
>  @group
>  $ @kbd{cd var}
> -$ @kbd{cat GNUFileParts.27962/longfile.1 \
> -  GNUFileParts.27962/longfile.2 >> longfile}
> -$ rm -f GNUFileParts.27962
> +$ @kbd{cat GNUFileParts/longfile.1 \
> +  GNUFileParts/longfile.2 >> longfile}
> +$ rm -f GNUFileParts
>  @end group
>  @end smallexample
>  
> @@ -10280,17 +10278,17 @@ more warnings and more files generated on your 
> disk, e.g.:
>  @smallexample
>  @group
>  $ @kbd{tar xf vol-1.tar}
> -var/PaxHeaders.27962/longfile: Unknown file type 'x', extracted as
> +var/PaxHeaders/longfile: Unknown file type 'x', extracted as
>  normal file
>  Unexpected EOF in archive
>  $ @kbd{tar xf vol-2.tar}
> -tmp/GlobalHead.27962.1: Unknown file type 'g', extracted as normal file
> -GNUFileParts.27962/PaxHeaders.27962/sparsefile.1: Unknown file type
> +tmp/GlobalHead.1: Unknown file type 'g', extracted as normal file
> +GNUFileParts/PaxHeaders/sparsefile.1: Unknown file type
>  'x', extracted as normal file
>  @end group
>  @end smallexample
>  
> -Ignore these warnings.  The @file{PaxHeaders.*} directories created
> +Ignore these warnings.  The @file{PaxHeaders} directories created
>  will contain files with @dfn{extended header keywords} describing the
>  extracted files.  You can delete them, unless they describe sparse
>  members.  Read further to learn more about them.
> diff --git a/src/buffer.c b/src/buffer.c
> index 1a96595..ba41927 100644
> --- a/src/buffer.c
> +++ b/src/buffer.c
> @@ -1692,7 +1692,7 @@ add_chunk_header (struct bufmap *map)
>        st.stat.st_uid = getuid ();
>        st.stat.st_gid = getgid ();
>        st.orig_file_name = xheader_format_name (&st,
> -                                               "%d/GNUFileParts.%p/%f.%n",
> +                                               "%d/GNUFileParts/%f.%n",
>                                                 volno);
>        st.file_name = st.orig_file_name;
>        st.archive_file_size = st.stat.st_size = map->sizeleft;
> diff --git a/src/xheader.c b/src/xheader.c
> index a5452a1..be57f81 100644
> --- a/src/xheader.c
> +++ b/src/xheader.c
> @@ -362,13 +362,17 @@ xheader_format_name (struct tar_stat_info *st, const 
> char *fmt, size_t n)
>  char *
>  xheader_xhdr_name (struct tar_stat_info *st)
>  {
> +  bool posixly_correct = (getenv ("POSIXLY_CORRECT") != NULL);

I *personally* would rather see 'posixly_correct' variable somewhere in
global scope (or at least somewhere else) -- without the need to check for
POSIXLY_CORRECT env variable for each file.

>    if (!exthdr_name)
> -    assign_string (&exthdr_name, "%d/PaxHeaders.%p/%f");
> +    {
> +      if (posixly_correct)
> +        assign_string (&exthdr_name, "%d/PaxHeaders.%p/%f");
> +      else
> +        assign_string (&exthdr_name, "%d/PaxHeaders/%f");
> +    }
>    return xheader_format_name (st, exthdr_name, 0);
>  }
>  
> -#define GLOBAL_HEADER_TEMPLATE "/GlobalHead.%p.%n"
> -
>  char *
>  xheader_ghdr_name (void)
>  {
> @@ -378,10 +382,16 @@ xheader_ghdr_name (void)
>        const char *tmp = getenv ("TMPDIR");
>        if (!tmp)
>       tmp = "/tmp";
> -      len = strlen (tmp) + sizeof (GLOBAL_HEADER_TEMPLATE); /* Includes nul 
> */
> +      bool posixly_correct = (getenv ("POSIXLY_CORRECT") != NULL);
> +      const char *global_header_template;
> +      if (posixly_correct)
> +        global_header_template = "/GlobalHead.%p.%n";
> +      else
> +        global_header_template = "/GlobalHead.%n";

Perhaps place for ternary operator?

> +      len = strlen (tmp) + sizeof (global_header_template); /* Includes nul 
> */
>        globexthdr_name = xmalloc (len);
>        strcpy(globexthdr_name, tmp);
> -      strcat(globexthdr_name, GLOBAL_HEADER_TEMPLATE);
> +      strcat(globexthdr_name, global_header_template);
>      }
>  
>    return xheader_format_name (NULL, globexthdr_name, global_header_count + 
> 1);

Note also that Sergey will probably need copyright assignment for patch of
this size.

Pavel