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 git environment variables if set #42

Merged
merged 2 commits into from Feb 24, 2018

Conversation

rbialon
Copy link
Contributor

@rbialon rbialon commented Feb 22, 2018

Git also uses the GIT_DIR and GIT_WORK_TREE environment variables , which should be preferred if set.
This does not fix #37 yet, but provides a base for vcsh to work. Vcsh only has to export those environment variables and it will work.
In the meantime, other scripts relying on those variables will work right away.

@rhysd
Copy link
Owner

rhysd commented Feb 23, 2018

Thank you for catching it. I didn't know $GIT_WORK_TREE. It's good point.

" Use environment variables if set
if !empty($GIT_DIR) && !empty($GIT_WORK_TREE)
return [$GIT_DIR, $GIT_WORK_TREE]
endif
Copy link
Owner

@rhysd rhysd Feb 23, 2018

Choose a reason for hiding this comment

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

$GIT_WORK_TREE is set by user and the path must be existing. So $GIT_WORK_TREE should be verified as follows:

if !empty($GIT_DIR) && !empty($GIT_WORK_TREE) && isdirectory($GIT_WORK_TREE)
    return [$GIT_DIR, $GIT_WORK_TREE]
endif

Copy link
Owner

Choose a reason for hiding this comment

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

Or should we explicitly raise an error rather than falling back into automatic worktree detection silently?

if !empty($GIT_DIR) && !empty($GIT_WORK_TREE)
    if !isdirectory($GIT_WORK_TREE)
        throw 'Directory specified with $GIT_WORK_TREE is not found: ' . $GIT_WORK_TREE
    endif
    return [$GIT_DIR, $GIT_WORK_TREE]
endif

Either look ok for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I set GIT_WORK_TREE manually, that's correct. I like the variant throwing an error because git cannot operate if the value of GIT_WORK_TREE is invalid:

test master@HEAD$ GIT_WORK_TREE="/tmp/invalid" git status
fatal: This operation must be run in a work tree

Copy link
Owner

@rhysd rhysd left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you.

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.

Does not work for vcsh or fake bare git technique
2 participants