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

Fix: Allow git worktree in version detection; drop svn, hg. #7707

Merged
merged 1 commit into from
Aug 31, 2019

Conversation

kernigh
Copy link
Contributor

@kernigh kernigh commented Aug 28, 2019

If $ROOT_DIR is a linked working tree from git-worktree(1), then
$ROOT_DIR/.git is a regular file instead of a directory. Allow this
when deciding whether to use git to detect OpenTTD's version.

Drop checks for svn and hg in config.lib, because findversion.sh
hasn't used svn nor hg since 192770e.

If $ROOT_DIR is a linked working tree from git-worktree(1), then
$ROOT_DIR/.git is a regular file instead of a directory.  Allow this
when deciding whether to use git to detect OpenTTD's version.

Drop checks for svn and hg in config.lib, because findversion.sh
hasn't used svn nor hg since 192770e.
@kernigh
Copy link
Contributor Author

kernigh commented Aug 28, 2019

(Also, git commit from a linked tree fails, because the openttd_hooks can't find the hooks directory. I used HOOKS_DIR=$(git rev-parse --git-path hooks) git commit to make this commit.)

@nielsmh
Copy link
Contributor

nielsmh commented Aug 29, 2019

Looks fine at first glance, might have to be redone for #7270 (CMake) at some point.

@LordAro
Copy link
Member

LordAro commented Aug 29, 2019

Seems to me that both -f and -d checks are unnecessary? Just -e should do

@kernigh
Copy link
Contributor Author

kernigh commented Aug 29, 2019

Yes, -e also works. I put both -d and -f to show that .git can be a regular file. If I put -e, I fear that someone would later change the -e back to -d.

I see that #7270 CMake uses IS_DIRECTORY "${CMAKE_SOURCE_DIR}/.git". I would propose to change IS_DIRECTORY to EXISTS (because CMake has no IS_FILE), and add a comment that .git may be a directory or a regular file. I like the CMake script better than our shell scripts, because the check for .git is in 1 place, not 2.

Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, it'll do for now, especially if it's about to be rewritten Soon(tm)

@LordAro LordAro merged commit c655f89 into OpenTTD:master Aug 31, 2019
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

Successfully merging this pull request may close these issues.

3 participants