GNATCOLL.Projects.Rename_And_Move fails to preserve trailing directory separator

[LordAro]

When calling GNATCOLL.Projects.Rename_And_Move, if you pass in a directory that is missing the trailing slash (such as a path generated by Ada.Directories.Containing_Directory), the resulting renamed gpr file is missing path separators in attributes such as Object_Dir, which can lead to invalid directory/pathnames

Minimal test case

bug6486.adb

with GNATCOLL.Projects; use GNATCOLL.Projects;
with GNATCOLL.VFS;      use GNATCOLL.VFS;
with Ada.Directories;

procedure bug6486 is
   tree : Project_Tree;
   proj_file : String := "bug6486.gpr";
   proj : Project_Type;
   dummy : Boolean;
begin
   tree.Load (Create (Filesystem_String (proj_file)));

   proj := tree.Root_Project;
   proj.Rename_And_Move
      ("foobar_" & proj.Name, Create (Filesystem_String 
         (Ada.Directories.Containing_Directory (+proj.Project_Path.Full_Name))));

   tree.Recompute_View;
   dummy := proj.Save;
end bug6486;

bug6486.gpr

with "gnatcoll_full";

project bug6486 is

   for Exec_Dir use ".";
   for Source_Dirs use (".");
   for Main use ("bug6486.adb");
   type Kind_Type is
      ("dev", "release");
   Kind : Kind_Type := external ("Kind");

   case Kind is

      when "dev" =>
         for Object_Dir use "obj/" & Kind;

      when "release" =>
         for Object_Dir use "obj/" & Kind;
   end case;

end bug6486;

Results

With the above code, run:

gprbuild -p -XKind=dev -Pbug6486 && ./bug6486 && diff -wu bug6486.gpr foobar_bug6486.gpr

which should result in:

--- bug6486.gpr 2018-06-12 15:14:59.566694628 +0100
+++ foobar_bug6486.gpr  2018-06-12 15:15:01.086683191 +0100
@@ -1,6 +1,6 @@
 with "gnatcoll_full";

-project bug6486 is
+project Foobar_Bug6486 is

    for Exec_Dir use ".";
    for Source_Dirs use (".");
@@ -12,11 +12,11 @@
    case Kind is

       when "dev" =>
-         for Object_Dir use "obj/" & Kind;
+         for Object_Dir use "obj" & Kind;

       when "release" =>
-         for Object_Dir use "obj/" & Kind;
+         for Object_Dir use "obj" & Kind;
    end case;

-end bug6486;
+end Foobar_Bug6486;

Hacky fix

The following hack is being used internally. I've not pull requested it, because it's a hack, and I'm not convinced it's the correct fix anyway.

From my own investigations, the issue is that the Relative_Path (File, New_Path) call inside GNATCOLL.Projects.Normalize is stripping the trailing path separator (when New_Path is missing a trailing '/'), compared to the original 'D' string, but I've not got as far as working out quite why that's happening

(even the diff is hacked together, probably won't even apply automatically)

Index: gnatcoll-projects-normalize.adb
===================================================================
--- gnatcoll-projects-normalize.adb     (revision 55361)
+++ gnatcoll-projects-normalize.adb     (revision 55362)
@@ -2875,11 +2875,28 @@
                               Get_Host (New_Path));
                begin
                   if not Is_Absolute_Path (D) then
+                     -- fix here for bug6486
+                     declare
+                        dir_string : constant String := String(D);
+                        -- get whether string ends in directory seperator
+                        end_with_slash : constant Boolean := (dir_string(dir_string'Last) = '/'
+                                                              or else dir_string(dir_string'Last) = '\');
+                        -- if it does make sure that this is not lost
+                        function get_possible_end_dir_sep return String is
+                        begin
+                           if end_with_slash then
+                              return "" & dir_string(dir_string'Last);
+                           else
+                              return "";
+                           end if;
+                        end get_possible_end_dir_sep;
+                        suffix : constant String := get_possible_end_dir_sep;
+                     begin
+
                      Set_String_Value_Of
                        (Node, Tree_Node,
-                        Get_String
-                          (+Relative_Path
-                             (File, New_Path)));
+                           Get_String (+(Relative_Path (File, New_Path)) & suffix));
+                     end;
                   end if;
                end;

(Copy of R612-025)

[Jicquel]

Hello @LordAro, thank you for your issue! It has been years since it was opened, and we apologize for this delay. From my understanding, GNATCOLL.VFS_Utils.Normalize_Pathname relies on System.OS_Lib.Normalize_Pathname. The trailing directory separator is removed by the latter. 

We will keep the "hack" you provided, as removing the trailing directory is the expected behavior of Normalize_Pathname. You can either open a PR or let us do the integration of your patch. I will give you time so you can decide what you would like.

[LordAro]

Best leave this with you - we ended up vendoring a copy of gnatcoll-projects-* and making the necessary modifications ourselves so I'm not best placed to PR this anymore

[Jicquel]

Understood! As you did most of the work, I suggest you open a PR with the code you already provided in this issue. I will take care of updating the code and adding tests, but you will remain the main author of this patch.

[Jicquel]

Hello @LordAro, your patch has been merged with minor changes (d092047). I took the liberty to add the following message to the commit:

This commit is the continuity of the work from Charles Pigott (LordAro) that
can be found here: #32.

Thank you for your contribution!