Re: [Test patch] Re: Duplicate files /boot/grub/locale/*.mo~ in iso file made by grub-mkrescue

Hi, I'm having some difficulties compiling grub from git (my own
deficiencies) - I do not see any .mo files on iso with it, the locale
dir is empty, somehow the localization is off. But that happens
even without your patch, so I am doing something wrong.

I was able to apply your patches to grub-2.12 in gentoo through
the use of user patches and your patch works - there are no more
*.mo~ files in the iso made by grub-mkrescue, and all the .mo
files are there.

But looking at the patch let me say: it would be better if the
*.mo~ weren't (temporarily) made during the process of creating
the iso at all, so it would be better to prevent their creation,
rather than fix their deletion. Just a personal opinion,
I don't know how to do that.

Anyway, again, your patch works for me well, so far. Good job!

Best regards,
Adam



On 5/29/25 2:58 PM, Thomas Schmitt wrote:
> 
> This is my preliminary proposal for a grub-util API call
>    void grub_install_delete_backup_files (void);
> which gets called by grub-mkrescue instead of
>    grub_set_install_backup_ponr (void);
> 
> In my Debian 12 setup with X86_64_EFI and I386_PC this prevents the
> presence of .mo~ files in the ISO.
> 
> Polishing this proposal for submission on grub-devel will be some more
> work. So i ask you to apply it to GRUB source and to test whether it
> does what we want without spoiling anything that worked before.
> 
> Change details beyond just calling the atexit cleanup code:
> 
> The functionality to remove backup files depended on the availability
> of system call atexit(3). Now only the ability to perform this removal
> automatically at exit time depends on HAVE_ATEXIT.
> 
> The changed grub-mkrescue was tested with valgrind which reports 6610
> lost allocated bytes. I did not investigate this further since my
> changes do not allocate additional memory.
> 
> 
> Have a nice day :)
> 
> Thomas
> 
> =======================================================================
> 
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 5c0a52ca2..22c214a20 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -146,6 +146,9 @@ void
>   grub_install_copy_files (const char *src,
>   			 const char *dst,
>   			 enum grub_install_plat platid);
> +void
> +grub_install_delete_backup_files (void);
> +
>   char *
>   grub_install_get_platform_name (enum grub_install_plat platid);
>   
> @@ -290,6 +293,9 @@ grub_install_is_short_mbrgap_supported (void);
>    * to set the grub_install_backup_ponr flag. This way failure to perform
>    * subsequent actions will not result in reverting new modules to the
>    * old ones, e.g. in case efivars updates fails.
> + * Independently of HAVE_ATEXIT it is possible to set the flag and to perform
> + * immediate removal of the backup files by
> + *   void grub_install_delete_backup_files (void);
>    */
>   #ifdef HAVE_ATEXIT
>   extern void
> diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> index 22bccb6a3..ee26dc91b 100644
> --- a/util/grub-install-common.c
> +++ b/util/grub-install-common.c
> @@ -287,17 +287,16 @@ clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
>     grub_util_fd_closedir (d);
>   }
>   
> -#ifdef HAVE_ATEXIT
>   static void
> -restore_backup_atexit (void)
> +restore_backup (int by_any_pid)
>   {
>     size_t i;
>   
>     /*
> -   * Some child inherited atexit() handler, did not clear it, and called it.
> -   * Thus skip clean or restore logic.
> +   * If not by_any_pid, check whether some child inherited atexit() handler,
> +   * did not clear it, and called it. Thus skip clean or restore logic.
>      */
> -  if (backup_process != getpid ())
> +  if (backup_process != getpid () && !by_any_pid)
>       return;
>   
>     for (i = 0; i < backup_dirs_size; i++)
> @@ -318,7 +317,15 @@ restore_backup_atexit (void)
>   
>     backup_dirs_size = 0;
>   
> -  free (backup_dirs);
> +  if (backup_dirs != NULL)
> +      free (backup_dirs);
> +  backup_dirs = NULL;
> +}
> +
> +static void
> +restore_backup_atexit (void)
> +{
> +  restore_backup (0);
>   }
>   
>   static void
> @@ -329,16 +336,12 @@ append_to_backup_dirs (const char *dir)
>     backup_dirs_size++;
>     if (!backup_process)
>       {
> +#ifdef HAVE_ATEXIT
>         atexit (restore_backup_atexit);
> +#endif
>         backup_process = getpid ();
>       }
>   }
> -#else
> -static void
> -append_to_backup_dirs (const char *dir __attribute__ ((unused)))
> -{
> -}
> -#endif
>   
>   static void
>   clean_grub_dir (const char *di)
> @@ -1189,3 +1192,11 @@ grub_util_unlink_recursive (const char *name)
>     grub_util_rmdir (name);
>     grub_util_fd_closedir (d);
>   }
> +
> +
> +void
> +grub_install_delete_backup_files (void)
> +{
> +  grub_install_backup_ponr = 1;
> +  restore_backup (1);
> +}
> diff --git a/util/grub-mkrescue.c b/util/grub-mkrescue.c
> index 6dc71a8a1..a2e5149a2 100644
> --- a/util/grub-mkrescue.c
> +++ b/util/grub-mkrescue.c
> @@ -555,7 +555,7 @@ main (int argc, char *argv[])
>         source_dirs[plat] = xstrdup (grub_install_source_directory);
>       }
>   
> -  grub_set_install_backup_ponr ();
> +  grub_install_delete_backup_files ();
>   
>     if (system_area == SYS_AREA_AUTO || grub_install_source_directory)
>       {