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

java dependencies test -s -> use test -d #1917

Closed
wants to merge 1 commit into from
Closed

java dependencies test -s -> use test -d #1917

wants to merge 1 commit into from

Conversation

grooverdan
Copy link
Contributor

To correct a build process where the JAVA_TEST_LIBDIR is a symlink to a cache directory.

Test -s (size 0) on symlinks returns true, resulting in a mkdir over the top of the symlink resulting in failure.

As a solution -d checks if it is a directory (or the symlink refers to a directory), which works in the case of real directories and symlinks to directories.

Trivial I know but it was really easy for me to use a symlink here to prevent frequent downloads in a CI environment.

Thanks for your consideration.

Test -s (size 0) on symlinks returns true, resulting
in a mkdir over the top of the symlink resulting in failure.

-d checks if it is a directory (or the symlink refers to
a directory, which works in the case of real directories and
symlinks to directories.

Signed-off-by: Daniel Black <daniel.black@au.ibm.com>
@facebook-github-bot
Copy link
Contributor

@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@grooverdan
Copy link
Contributor Author

I know such a trivial patch is low priority and this is also low risk. Can this be merged and if some unexpected failure occurs on someone's CI then it can be revered. Using this patch the dependence of the travis builds on search.maven.org can be reduced using the cache -> directories option. Happy to do a patch there if merged.

@adamretter
Copy link
Collaborator

@siying sounds good to me.

@grooverdan
Copy link
Contributor Author

thankyou.

@grooverdan grooverdan deleted the test-d branch April 12, 2017 22:21
@siying
Copy link
Contributor

siying commented Apr 12, 2017

@grooverdan sorry for the delay. We migrated the internal system a little bit so I committed to a wrong place. Now I fixed it.

@grooverdan
Copy link
Contributor Author

thanks for letting me know. I'll take a look.

@siying
Copy link
Contributor

siying commented Apr 12, 2017

@grooverdan oh I already fixed it. I just tried to explain why it took me so long.

@grooverdan
Copy link
Contributor Author

all good :-) have a good day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants