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.

Sign up for GitHub

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

Jump to bottom

Build scripts rely on $(cd ...; pwd) which doesn't work as expected in case of non-empty CDPATH #3359

Closed
PawelLipski opened this issue Jun 8, 2020 · 8 comments
Closed

Build scripts rely on $(cd ...; pwd) which doesn't work as expected in case of non-empty CDPATH #3359

PawelLipski opened this issue Jun 8, 2020 · 8 comments

Comments

@PawelLipski
Copy link

Setting CDPATH shell variable to anything non-empty (even if it's just one entry, like CDPATH=.) causes cd to output the path where it moved to, which in turn causes errors like this during ./gradlew build:

Entering /home/pawellipski/annotation-tools/.travis-build-without-test.sh in /home/pawellipski/annotation-tools
+ export SHELLOPTS
++ uname
+ '[' Linux = Darwin ']'
+ export JAVA_HOME=/usr/lib/jvm/java-11-openjdk-11.0.6.10-0.fc31.x86_64
+ JAVA_HOME=/usr/lib/jvm/java-11-openjdk-11.0.6.10-0.fc31.x86_64
+ echo JAVA_HOME=/usr/lib/jvm/java-11-openjdk-11.0.6.10-0.fc31.x86_64
JAVA_HOME=/usr/lib/jvm/java-11-openjdk-11.0.6.10-0.fc31.x86_64
++ cd annotation-file-utilities
++ pwd -P
+ export 'AFU=/home/pawellipski/annotation-tools/annotation-file-utilities
/home/pawellipski/annotation-tools/annotation-file-utilities'
+ AFU='/home/pawellipski/annotation-tools/annotation-file-utilities
/home/pawellipski/annotation-tools/annotation-file-utilities'
+ export 'PATH=/home/pawellipski/annotation-tools/annotation-file-utilities
/home/pawellipski/annotation-tools/annotation-file-utilities/scripts:/usr/lib/jvm/java-11-openjdk-11.0.6.10-0.fc31.x86_64/bin:/home/pawellipski/.nvm/versions/node/v12.13.1/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/var/lib/snapd/snap/bin:/bin:/home/pawellipski/go/bin:/home/pawellipski/bin:/bin:/home/pawellipski/go/bin:/home/pawellipski/bin:/bin:/home/pawellipski/go/bin:/home/pawellipski/bin'
+ PATH='/home/pawellipski/annotation-tools/annotation-file-utilities
/home/pawellipski/annotation-tools/annotation-file-utilities/scripts:/usr/lib/jvm/java-11-openjdk-11.0.6.10-0.fc31.x86_64/bin:/home/pawellipski/.nvm/versions/node/v12.13.1/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/var/lib/snapd/snap/bin:/bin:/home/pawellipski/go/bin:/home/pawellipski/bin:/bin:/home/pawellipski/go/bin:/home/pawellipski/bin:/bin:/home/pawellipski/go/bin:/home/pawellipski/bin'
+ cd /home/pawellipski/annotation-tools/annotation-file-utilities /home/pawellipski/annotation-tools/annotation-file-utilities
./.travis-build-without-test.sh: line 24: cd: too many arguments
+ TERM=dumb
+ ./gradlew help

> Task :maybeCloneAndBuildDependencies FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':maybeCloneAndBuildDependencies'.
> Process 'command 'checker/bin-devel/build.sh'' finished with non-zero exit value 127

Esp. see the export 'AFU=... line, here's where the output of both cd and pwd has been assigned, and not just the pwd's output.
@PawelLipski PawelLipski changed the title Build scripts rely on $(cd ...; pwd) which doesn't work in case of non-empty CDPATH Build scripts rely on $(cd ...; pwd) which doesn't work as expected in case of non-empty CDPATH Jun 8, 2020
@mernst
Copy link
Member

mernst commented Jun 8, 2020

@PawelLipski Could you try changing that line to

export AFU="${AFU:-$(cd ../annotation-tools/annotation-file-utilities >/dev/null 2>&1 && pwd -P)}"

? If that works for you, I will commit that fix.

mernst added a commit to mernst/checker-framework that referenced this issue Jun 8, 2020
@mernst
Suppress output of cd command, in case CDPATH environment variable is…
289e097 
… set; fixes typetools#3359
@PawelLipski
Copy link
Author

PawelLipski commented Jun 8, 2020 •
edited
Loading

(btw, as a quick workaround, just setting CDPATH= solves the issue).

Yes, I did change this line... the problem is that:

  1. there are a few other places like that across the checker-framework codebase
  2. the same bug also happens a few times in annotation-tools codebase which is checked out locally by the CF build process:
 15:45 ~/annotation-tools master $ git grep '\$(.*cd'
.travis-build-without-test.sh:echo Entering "$(cd "$(dirname "$0")" && pwd -P)/$(basename "$0")" in `pwd`
.travis-build-without-test.sh:export AFU="${AFU:-$(cd annotation-file-utilities && pwd -P)}"
.travis-build-without-test.sh:echo Exiting "$(cd "$(dirname "$0")" && pwd -P)/$(basename "$0")" in `pwd`
.travis-build.sh:echo Entering "$(cd "$(dirname "$0")" && pwd -P)/$(basename "$0")" in `pwd`
.travis-build.sh:echo Exiting "$(cd "$(dirname "$0")" && pwd -P)/$(basename "$0")" in `pwd`
annotation-file-utilities/dev-scripts/test-allTests.sh:export AFU="${AFU:-$(cd annotation-file-utilities && pwd -P)}"
annotation-file-utilities/dev-scripts/test-allTests.sh:export CHECKERFRAMEWORK="${CHECKERFRAMEWORK:-$(cd .. && pwd -P)/checker-framework}"
annotation-file-utilities/dev-scripts/test-downstream.sh:export AFU="${AFU:-$(cd annotation-file-utilities && pwd -P)}"
annotation-file-utilities/dev-scripts/test-downstream.sh:export CHECKERFRAMEWORK="${CHECKERFRAMEWORK:-$(cd .. && pwd -P)/checker-framework}"
annotation-file-utilities/dev-scripts/test-misc.sh:export AFU="${AFU:-$(cd annotation-file-utilities && pwd -P)}"
annotation-file-utilities/dev-scripts/test-misc.sh:export CHECKERFRAMEWORK="${CHECKERFRAMEWORK:-$(cd .. && pwd -P)/checker-framework}"
annotation-file-utilities/dev-scripts/test-typecheck.sh:export AFU="${AFU:-$(cd annotation-file-utilities && pwd -P)}"
annotation-file-utilities/dev-scripts/test-typecheck.sh:export CHECKERFRAMEWORK="${CHECKERFRAMEWORK:-$(cd .. && pwd -P)/checker-framework}"

@mernst
Copy link
Member

mernst commented Jun 8, 2020

there are a few other places like that across the checker-framework codebase

I saw the occurrences in the annotation-tools and checker-framework-inference repositories, but I don't see other places in the checker-framework repository. Can you point out the places I missed? Thanks.

@PawelLipski
Copy link
Author

Hmmm grep on CF repo shows the following:

➜ 17:21 ~/checker-framework feature/require-prefix-ignore-all $ git grep '\$(.*cd'
.travis-build.sh:echo "Entering $(cd "$(dirname "$0")" && pwd -P)/$(basename "$0") in $(pwd)"
.travis-build.sh:ROOTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
.travis-build.sh:echo Exiting "$(cd "$(dirname "$0")" && pwd -P)/$(basename "$0") in $(pwd)"
checker/bin-devel/test-cf-inference.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-cf-inference.sh:export AFU="${AFU:-$(cd ../annotation-tools/annotation-file-utilities && pwd -P)}"
checker/bin-devel/test-cftests-all.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-cftests-junit.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-cftests-nonjunit.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-daikon-part1.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-daikon-part2.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-daikon.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-downstream.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava-formatter.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava-index.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava-interning.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava-lock.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava-nullness.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava-regex.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava-signature.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-guava.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-misc.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
checker/bin-devel/test-plume-lib.sh:SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"

although checker/bin-devel/test-*.sh files are likely symlinks anyway.

@PawelLipski
Copy link
Author

So yes, the only "unclean" occurrences left are the two in .travis-build.sh (and it's just a message for user), and checker/bin-devel/test-cf-inference.sh. You're right, nothing else left in CF then.

@mernst
Copy link
Member

mernst commented Jun 8, 2020

Great, thanks for confirming. That was my analysis too.

@mernst mernst closed this as completed in b16be6f Jun 8, 2020
@mernst
Copy link
Member

mernst commented Jun 8, 2020

Thanks for the bug report. I'm sorry you encountered this problem. We appreciate users finding the corner cases that we didn't think to test. :-)

@PawelLipski
Copy link
Author

Haha no problem :) I actually wasn't aware of that CDPATH=<non-empty> & cd behavior myself ;)

Speaking of corner cases... I'd be grateful if you (or one of your colleagues) took a look at #3356... it's relatively simple, and likely leads to a quite high number of false negatives among CF&Lombok users. I believe that in most cases the affected users aren't even aware of this unsoundness :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mernst @PawelLipski