Re: [PATCH rebase] Add support for Compact OS compression for Cygwin

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

On Jul 14 12:02, Christian Franke wrote:
> [Sorry if this is the wrong list]

Yes, in theorie, but no worries.  However...

> This finally completes '--compact-os' support of Cygwin setup.
> https://sourceware.org/pipermail/cygwin-apps/2021-May/041225.html
> 
> -- 
> Regards
> Christian
> 

> From 807ae9fbaef18491f3aa1e94e66dd21eb6748c3e Mon Sep 17 00:00:00 2001
> From: Christian Franke <christian.franke@t-online.de>
> Date: Thu, 14 Jul 2022 11:59:50 +0200
> Subject: [PATCH] Add support for Compact OS compression for Cygwin
> 
> Preserve compression of manually rebased files.
> Align compression with Cygwin DLL if database is used.
> Only check for writability if file needs rebasing to keep
> compression of unchanged files.
> 
> Signed-off-by: Christian Franke <christian.franke@t-online.de>
> ---
>  rebase.c | 199 +++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 155 insertions(+), 44 deletions(-)
> 
> diff --git a/rebase.c b/rebase.c
> index a403c85..06828bb 100644
> --- a/rebase.c
> +++ b/rebase.c
> @@ -39,6 +39,10 @@
>  #include <errno.h>
>  #include "imagehelper.h"
>  #include "rebase-db.h"
> +#if defined(__CYGWIN__)
> +#include <io.h>
> +#include <versionhelpers.h>
> +#endif
>  
>  BOOL save_image_info ();
>  BOOL load_image_info ();
> @@ -48,6 +52,10 @@ void print_image_info ();
>  BOOL rebase (const char *pathname, ULONG64 *new_image_base, BOOL down_flag);
>  void parse_args (int argc, char *argv[]);
>  unsigned long long string_to_ulonglong (const char *string);
> +#if defined(__CYGWIN__)
> +static int compactos_get_algorithm (const char *pathname);
> +static int compactos_compress_file (const char *pathname, int algorithm);
> +#endif
>  void usage ();
>  void help ();
>  BOOL is_rebaseable (const char *pathname);
> @@ -259,9 +267,19 @@ main (int argc, char *argv[])
>        ULONG64 new_image_base = image_base;
>        for (i = 0; i < img_info_size; ++i)
>  	{
> +#if defined(__CYGWIN__)

Given compactos stuff is a OS thingy and not actually a Cygwin feature,
why do we need an ifdef CYGWIN?

> +	  int compactos_algorithm
> +	      = compactos_get_algorithm (img_info_list[i].name);
> +#endif
>  	  status = rebase (img_info_list[i].name, &new_image_base, down_flag);
>  	  if (!status)
>  	    return 2;
> +#if defined(__CYGWIN__)
> +	  /* Reapply previous compression. */
> +	  if (compactos_algorithm >= 0)
> +	    compactos_compress_file (img_info_list[i].name,
> +				     compactos_algorithm);
> +#endif
>  	}
>      }
>    else
> @@ -269,6 +287,9 @@ main (int argc, char *argv[])
>        /* Rebase with database support. */
>        BOOL header;
>  
> +#if defined(__CYGWIN__)
> +      int compactos_algorithm = compactos_get_algorithm ("/bin/cygwin1.dll");
> +#endif
>        if (merge_image_info () < 0)
>  	return 2;
>        status = TRUE;
> @@ -279,6 +300,14 @@ main (int argc, char *argv[])
>  	    status = rebase (img_info_list[i].name, &new_image_base, FALSE);
>  	    if (status)
>  	      img_info_list[i].flag.needs_rebasing = 0;
> +#if defined(__CYGWIN__)
> +	    /* If Cygwin DLL is compressed, assume setup was used with option
> +	       --compact-os.  Align compression with Cygwin DLL. */
> +	    if (compactos_algorithm >= 0
> +		&& compactos_compress_file (img_info_list[i].name,
> +					    compactos_algorithm) < 0)
> +	      compactos_algorithm = -1;
> +#endif

This ifdef still makes sense, of course and on first glance, the
remainder of the patch LGTM.


Thanks,
Corinna