[elbe-devel] [PATCH v2] dump: remove 2nd extract of archive

[Manuel Traut]

Hi John,

On Wed, Dec 20, 2017 at 04:27:40PM +0100, John Ogness wrote:
> Originally archive extraction occurred only after the finetuning step.
> With commit 33d0e328b74d ("extract archive before finetuning to be
> able to use the files.") extraction was added before the finetuning
> step to allow finetuning to use archive files. However, the extraction
> after finetuning was never removed. This dramatically reduces the
> benefits of finetuning being able to manipulate the archive files.
> 
> Remove archive extraction after the finetuning step.
> 
> Update the logic to determine file origins in the elbe report
> based on this new ordering.
> 
> Add a new "Archive validation" section in validation.txt to list
> and archive files that have been modified or deleted in finetuning.

Basicly i'm fine with these changes.

Just a few comments, see inline.

Can you plesae send a v3?

Thanks,

  Manuel

> Signed-off-by: John Ogness <john.ogness at linutronix.de>
> ---
>  elbepack/dump.py        | 64 +++++++++++++++++++++++++++++++------------------
>  elbepack/elbeproject.py |  5 ++--
>  2 files changed, 44 insertions(+), 25 deletions(-)
> 
> diff --git a/elbepack/dump.py b/elbepack/dump.py
> index 61dfeefd..93f0f6a2 100644
> --- a/elbepack/dump.py
> +++ b/elbepack/dump.py
> @@ -160,7 +160,8 @@ def check_full_pkgs(pkgs, fullpkgs, errorname, cache):
>      if errors == 0:
>          elog.printo( "No Errors found" )
>  
> -def elbe_report( xml, buildenv, cache, reportname, targetfs ):
> +def elbe_report( xml, buildenv, cache, reportname, errorname, targetfs ):
> +    elog = ASCIIDocLog(errorname)
>      outf = ASCIIDocLog(reportname)
>      rfs = buildenv.rfs
>  
> @@ -199,35 +200,30 @@ def elbe_report( xml, buildenv, cache, reportname, targetfs ):
>      # the second extraction is done to ensure that files from the archive
>      # can't be modified/removed in finetuning

please remove the above comments

> -    outf.h2( "archive extract before finetuning" )
> +    index = cache.get_fileindex()
> +    mt_index = targetfs.mtime_snap()
> +
> +    outf.h2( "archive extract" )
>  
>      if xml.has("archive"):
>          with xml.archive_tmpfile() as fp:
>              outf.do( 'tar xvfj "%s" -C "%s"' % (fp.name, targetfs.path) )
> +        mt_index_post_arch = targetfs.mtime_snap()
> +    else:
> +        mt_index_post_arch = mt_index
>  
>      outf.h2( "finetuning log" )
>      outf.verbatim_start()
>  
> -    index = cache.get_fileindex()
> -    mt_index = targetfs.mtime_snap()
>      if xml.has("target/finetuning"):
>          do_finetuning(xml, outf, buildenv, targetfs)
>          #outf.print_raw( do_command( opt.finetuning ) )
>          mt_index_post_fine = targetfs.mtime_snap()
>      else:
> -        mt_index_post_fine = mt_index
> +        mt_index_post_fine = mt_index_post_arch
>  
>      outf.verbatim_end()
>  
> -    outf.h2( "archive extract after finetuning" )
> -
> -    if xml.has("archive"):
> -        with xml.archive_tmpfile() as fp:
> -            outf.do( 'tar xvfj "%s" -C "%s"' % (fp.name, targetfs.path) )
> -        mt_index_post_arch = targetfs.mtime_snap()
> -    else:
> -        mt_index_post_arch = mt_index_post_fine
> -
>      outf.h2( "fileslist" )
>      outf.table()
>  
> @@ -240,16 +236,19 @@ def elbe_report( xml, buildenv, cache, reportname, targetfs ):
>          else:
>              pkg = "postinst generated"
>  
> -        if fpath in mt_index_post_fine and fpath in mt_index:
> -            if mt_index_post_fine[fpath] > mt_index[fpath]:
> -                pkg = "modified finetuning"
>          if fpath in mt_index_post_fine:
> -            if mt_index_post_arch[fpath] > mt_index_post_fine[fpath]:
> -                pkg = "from archive"
> -            elif fpath not in mt_index:
> +            if fpath in mt_index_post_arch:
> +                if mt_index_post_fine[fpath] != mt_index_post_arch[fpath]:
> +                    pkg = "modified finetuning"
> +                elif fpath in mt_index:
> +                    if mt_index_post_arch[fpath] != mt_index[fpath]:
> +                        pkg = "from archive"
> +                    # else leave pkg as is
> +                else:
> +                    pkg = "added in archive"
> +            else:
>                  pkg = "added in finetuning"
> -        else:
> -            pkg = "added in archive"
> +        # else leave pkg as is
>  
>          outf.printo( "|+%s+|%s" % (fpath,pkg) )
>  
> @@ -258,7 +257,7 @@ def elbe_report( xml, buildenv, cache, reportname, targetfs ):
>      outf.h2( "Deleted Files" )
>      outf.table()
>      for fpath in list(mt_index.keys()):
> -        if fpath not in mt_index_post_arch:
> +        if fpath not in mt_index_post_fine:
>              if fpath in index:
>                  pkg = index[fpath]
>              else:
> @@ -286,3 +285,22 @@ def elbe_report( xml, buildenv, cache, reportname, targetfs ):
>      if xml.has("target/pkgversionlist"):
>          f.close ()
>  
> +    if not xml.has("archive"):
> +        return

There is already an issue [0] related to this. Instead of checking the existence
of the attribute it would be better to check if there is content.

> +    elog.h2("Archive validation")
> +
> +    errors = 0
> +
> +    for fpath in list(mt_index_post_arch.keys()):
> +        if fpath not in mt_index or mt_index_post_arch[fpath] != mt_index[fpath]:
> +            if fpath not in mt_index_post_fine:
> +                elog.printo( "- archive file %s was deleted in finetuning" % fpath )
> +                errors += 1
> +            elif mt_index_post_fine[fpath] > mt_index_post_arch[fpath]:
> +                elog.printo( "- archive file %s was modified in finetuning" % fpath )
> +                errors += 1
> +
> +    if errors == 0:
> +        elog.printo( "No Errors found" )
> +
> diff --git a/elbepack/elbeproject.py b/elbepack/elbeproject.py
> index fc4bc412..becd9944 100644
> --- a/elbepack/elbeproject.py
> +++ b/elbepack/elbeproject.py
> @@ -259,9 +259,10 @@ class ElbeProject (object):
>          extract_target( self.buildenv.rfs, self.xml, self.targetfs,
>                  self.log, self.get_rpcaptcache() )
>  
> +        validationpath = os.path.join( self.builddir, "validation.txt" )
> +
>          # Package validation and package list
>          if not skip_pkglist:
> -            validationpath = os.path.join( self.builddir, "validation.txt" )
>              pkgs = self.xml.xml.node( "/target/pkg-list" )
>              if self.xml.has( "fullpkgs" ):
>                  check_full_pkgs( pkgs, self.xml.xml.node( "/fullpkgs" ),
> @@ -295,7 +296,7 @@ class ElbeProject (object):
>          # Elbe report
>          reportpath = os.path.join( self.builddir, "elbe-report.txt" )
>          elbe_report( self.xml, self.buildenv, self.get_rpcaptcache(),
> -                reportpath, self.targetfs )
> +                reportpath, validationpath, self.targetfs )
>  
>          # the current license code raises an exception that interrupts the hole
>          # build if a licence can't be converted to utf-8. Exception handling can
> -- 
> 2.15.1

[0] https://github.com/Linutronix/elbe/issues/135