mainManeage - Support: sr #110738, configure gets confused with a...

 
 

sr #110738: configure gets confused with a build dir with the same base name as the current project

Submitter:  Victor R. Ruiz <rvr>
Submitted:  Thu 06 Oct 2022 01:01:20 AM UTC
   
 
Category:  None Priority:  5 - Normal
Severity:  3 - Normal Status:  Done
Privacy:  Public Assigned to:  boud
Open/Closed:  Closed Operating System:  GNU/Linux
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 17 Mar 2025 07:19:40 PM UTC, comment #12: 

Looks OK to me in commit 9e7203bbc4b7 .

Boud Roukema <boud>
Group Member
Sun 16 Mar 2025 02:02:49 AM UTC, comment #11: 

Thanks a lot Boud! This has been merged into the Maneage branch as part of Commit 9e7203bbc4b7 (updating the Python environment). So I am closing this support issue.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Thu 27 Feb 2025 09:42:13 PM UTC, comment #10: 

I was wrong: the coding fix was not implemented. I've cherry-picked the full commit onto 17e6bec. Looks ready for merging to me.

https://codeberg.org/boud/maneage_dev/src/branch/fix_subdirectory_bug

https://codeberg.org/boud/maneage_dev/commit/6207daf04470e9b3d53ce8f047bcdd9b8197ce6c

Boud Roukema <boud>
Group Member
Mon 24 Feb 2025 12:12:51 AM UTC, comment #9: 

Thanks Boud. Yes, if you can commit everything related to this bug, I can inspect and merge them all together and hopefully close this bug afterwards.

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 23 Feb 2025 06:23:56 PM UTC, comment #8: 

The coding fix has been implemented, but the wording changes
that aim to clarify the text are not yet implemented:

https://codeberg.org/boud/maneage_dev/commit/fd1a226b54e285b2dae41d510341cb067d2153b7

Should I post the text change as a separate commit?

Boud Roukema <boud>
Group Member
Thu 13 Oct 2022 11:55:59 PM UTC, comment #7: 

Victor - "my" fix is exactly the fix that you proposed in your original report - sorry, I wasn't clear on that in my comment from a few minutes ago. :)

comment #5:

> Please check commit fd1a226 [1]. For me this fixes the bug


> [1] https://codeberg.org/boud/maneage_dev/commit/fd1a226b54e285b2dae41d510341cb067d2153b7

Boud Roukema <boud>
Group Member
Thu 13 Oct 2022 11:50:45 PM UTC, comment #6: 

REMOVAL OF SUBDIRECTORY:

"Another thing: even if the regex fails, the directory is removed (if exists). I don't think any existing directory should be removed without user confirmation."

Maneage has quite a few 'rm' statements - see https://savannah.nongnu.org/task/?15997 for a related task that could reduce the probability of bad 'rm's occurring. And 'makefiles' typically have non-interactive removes with 'make clean' or with some steps in build processes (to remove temporary build directories).

Arguments for retaining the deletion section:

  • In this particular case, the subdirectory is only deleted if it appears to be empty (per 'ls -A').
  • The user should expect Maneage to be responsible for files and directories within its source directory, so deletion of an extra directory inserted there should be acceptable.


Arguments for making the deletion interactive (possibly by default disabled):

  • The user might wish to add some files and directories with the Maneage source directory, and even if a directory is empty, might still be annoyed at having it be deleted.
  • A user could still use '< /dev/null' for remaining inputs and fall back to a default, without being forced to run ./project interactively.


There seem to be arguments both ways. This could possibly be split off as a separate bug or task.


SYMLINK DIRECTORY(IES):

A separate issue is that the current subdirectory test does not check all cases of subdirectoriness. E.g.

cd /home/user
ln -s mysymlink myproj
cd mysymlink
mkdir buildsubdir

In this case, /home/user/myproj/buildsubdir is in reality a subdirectory of /home/user/mysymlink, but 'configure.sh' will not detect that, since it only compares absolute paths (obtained with 'pwd' from the host OS), not dereferenced absolute paths.

This could as much be a feature as a bug :), but someone could split it off as a bug or task if s/he feels that it's worth handling in the long term.


Boud Roukema <boud>
Group Member
Thu 13 Oct 2022 11:22:49 PM UTC, comment #5: 

Please check commit fd1a226 [1]. For me this fixes the bug in which 'configure.sh' (called by './project') incorrectly claims that the build directory is a subdirectory.

To see how this works, try

#!/bin/sh
currentdir=/home/user/myproj
printf "currentdir=$currentdir\n\n"
b="/home/user/myproj-build \
 /home/user/mypro \
 /home/user/myproj/ \
 /home/user/myproj \
 /home/user/myproj/build"
for bdir in $b; do
    printf "bdir=$bdir\n"
    if ! [ x"$bdir" = x ]; then
        if echo "$bdir/" \
                | grep '^'"$currentdir/" 2> /dev/null > /dev/null; then
            printf "subdir? yes\n\n"
        else
            printf "subdir? no\n\n"
        fi
    fi
done


Optionally remove the '/' to get back to the test in maneage commit 4318670.

I'm not sure if this is what you were proposing, Raúl, since the slash is in the regex, not in the parameter given to ./project. In any case, this works in my testing.

[1] https://codeberg.org/boud/maneage_dev/commit/fd1a226b54e285b2dae41d510341cb067d2153b7

Boud Roukema <boud>
Group Member
Thu 06 Oct 2022 07:16:44 PM UTC, comment #4: 

The trailing dots weren't the problem, I also tried with absolute dirs and the result was the same. The problem is the missing slash at the end and the regex used to check, as Boud points out.

Another thing: even if the regex fails, the directory is removed (if exists). I don't think any existing directory should be removed without user confirmation.

Victor R. Ruiz <rvr>
Thu 06 Oct 2022 06:10:58 PM UTC, comment #3: 

Indeed, thanks a lot for reporting this bug Victor, and the suggested fix.

The problem raised by Boud is valid and given my experience with new users, may happen regularly (before obtaining a Maneage-based mind-frame, most scientists just drop their input data, source code and output data in the same directory!).

But I don't think much complexity is needed. We can simply check if the absolute address of the given build directory is exactly equal to the absolute address of the source directory with and without a trailing slash. We already have an 'absolute_dir' function in 'configure.sh' for deriving the absolute address of a given location.

What do you think?

Mohammad Akhlaghi <makhlaghi>
Group administrator
Thu 06 Oct 2022 10:49:11 AM UTC, comment #2: 

Victor, thanks for the spotting this bug! Raúl: the problem is that currentdir

/home/username/niceprojects/project

is a substring of bdir

/home/username/niceprojects/project-build

Lines 923 or 930 convert from the relative directory to the absolute directory, so 'bdir' is absolute.

Victor, my guess is that your solution

grep '^'"$currentdir/"

may be OK, though I haven't checked this... One flaw would be that

/home/username/niceprojects/project/

is not a substring of

/home/username/niceprojects/project

so the project directory itself would be considered to "not be a subdirectory" and would be accepted as a build directory. So a more complex rule would seem to be needed.


Boud Roukema <boud>
Group Member
Thu 06 Oct 2022 08:22:14 AM UTC, comment #1: 

I think this happens because you are setting a relative path to the build directory. I always set the absolute path to the build directory and there is no problem. For example, instead of setting the relative path


../project-build


try setting the absolute path


/home/username/niceprojects/project-build


It is interesting what you propose in a situation that for some reason it is not possible to use the absolute path. Just adding the slash (/) at the end fixes the problem?


Raul Infante-Sainz <infantesainz>
Group Member
Thu 06 Oct 2022 01:01:20 AM UTC, original submission:  

Steps to reproduce


$ git clone https://git.maneage.org/project.git
$ cd project
$ ./project configure
...
Please enter the top build directory: ../project-build/


Expected result:
The directory is accepted, as it is not under the source-directory.

Current result:

 ** The build-directory cannot be under the source-directory.


The problem lies in comparison done in lines 939-940:reproduce/software/shell/configure.sh


 936         # If it is given, make sure it isn't a subdirectory of the source
 937         # directory.
 938         if ! [ x"$bdir" = x ]; then
 939             if echo "$bdir/" \
 940                     | grep '^'"$currentdir" 2> /dev/null > /dev/null; then


Using «grep '^'"$currentdir/"» fixes the problem (at least here).

Victor R. Ruiz <rvr>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by makhlaghi (Posted a comment)
  • -email is unavailable- added by boud (Posted a comment)
  • -email is unavailable- added by infantesainz (Posted a comment)
  • -email is unavailable- added by rvr (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-03-16 makhlaghi StatusWorks For Me Done
        Assigned toNone boud
        Open/ClosedOpen Closed
    2025-02-27 boud StatusNone Works For Me

    Back to the top

    Powered by Savane 3.15-ef91.
    Corresponding source code