Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use 'builtin' command in setup.bash #542

Closed
baxelrod-bdai opened this issue Oct 21, 2013 · 6 comments
Closed

use 'builtin' command in setup.bash #542

baxelrod-bdai opened this issue Oct 21, 2013 · 6 comments
Assignees

Comments

@baxelrod-bdai
Copy link

The setup.bash script should use "builtin cd" instead of just "cd". This fixes a bug for people who have created a function called "cd" to perform extra functionality in addition to changing directories, (for example doing an "ls" upon entering the new directory). The new line should be:

_CATKIN_SETUP_DIR=$(builtin cd "`dirname "${BASH_SOURCE[0]}"`" && pwd)

This will not change any behavior for those people who have not changed the functionality of the "cd" command.

I understand that the person who overrode "cd" should have the onus on them to fix any side-effects caused by their customization. However, the error you get is very non-intuitive and can take a long time to figure out why that command is not working.

@dirk-thomas
Copy link
Member

Besides the setup.bash I would assume that it would also be impossible to build anything with that kind of customized cd command since the since the same code is also in env.sh (which can not use builtin). Have you tried that or can you give it a try and report back if that is an issue too?

@baxelrod-bdai
Copy link
Author

I can compile fine with both rosbuild and catkin. (with custom cd command). Is this what you wanted me to test?

@dirk-thomas
Copy link
Member

Can you please post your custom cd function?

@baxelrod-bdai
Copy link
Author

It used to be pretty straightforward like this:

function cd {
    builtin cd $@
    ls
}

But has now grown to this:

export BACK_HISTORY=""
export FORWARD_HISTORY=""

function cd {

    GOPATH=$1
    DOTS=0

    # loop through entire path
    for (( i=0; i<${#1}; i++ ))
    do
        # count dots
        if [[ "${GOPATH:$i:1}" == "." ]]
        then
            let "DOTS+=1"
        else
            break
        fi
    done

    if [[ $DOTS -gt 2 ]]
    then
        # cut off old dots
        GOPATH=${GOPATH:$DOTS}

        # maybe cut off slash that will be duplicated
        if [[ "${GOPATH:0:1}" == "/" ]]
        then
            GOPATH=${GOPATH:1}
        fi

        # add real up paths: "../"
        for (( i=0; i<$DOTS-1; i++ ))
        do
           GOPATH="../"$GOPATH
        done
    fi

    # for some reason, the above code breaks cd with no parameters
    # going to the home directory, so put it back in
    if [[ ${#GOPATH} -eq 0 ]]
    then
        GOPATH="$HOME"
    fi

    # mimic browser forward and back buttons 
    BACK_HISTORY=$PWD:$BACK_HISTORY
    FORWARD_HISTORY=""

    builtin cd "$GOPATH"
    ls
}

# mimic browser forward and back buttons 
function back {
    DIR=${BACK_HISTORY%%:*}
    if [[ -d "$DIR" ]]
    then
        BACK_HISTORY=${BACK_HISTORY#*:}
        FORWARD_HISTORY=$PWD:$FORWARD_HISTORY
        builtin cd "$DIR"
        ls
    fi
}

# mimic browser forward and back buttons 
function forward {
    DIR=${FORWARD_HISTORY%%:*}
    if [[ -d "$DIR" ]]
    then
        FORWARD_HISTORY=${FORWARD_HISTORY#*:}
        BACK_HISTORY=$PWD:$BACK_HISTORY
        builtin cd "$DIR"
        ls
    fi
}

@dirk-thomas
Copy link
Member

Thanks for pointing it out. I fixed it for bash and zsh.

@baxelrod-bdai
Copy link
Author

Great! Thanks!

cwecht pushed a commit to cwecht/catkin that referenced this issue Mar 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants