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

get_git_root_path only checks three levels up #168

Closed
dscorbett opened this issue Nov 24, 2021 · 5 comments · Fixed by #171
Closed

get_git_root_path only checks three levels up #168

dscorbett opened this issue Nov 24, 2021 · 5 comments · Fixed by #171

Comments

@dscorbett
Copy link
Contributor

I’m generating fonts/otf/unhinted/instance_otf/NotoSansDuployan-Regular.otf in a Git repository and trying to use set_state_git_commit_sha1, but it can’t determine the commit hash because it can’t find the .git directory. Why is get_git_root_path limited to looking three levels up?

@chrissimpkins
Copy link
Member

I don't recall the decision but I am sure that it was arbitrary and only to establish a sanity check on the dir traversal distance. I would be happy to take a PR to expand the range so that it meets your requirements.

@dscorbett
Copy link
Contributor Author

dscorbett commented Nov 24, 2021

Thanks, I’ll do that. Ideally, I would remove the limit entirely. Is there a reason to keep a limit, e.g. to avoid a symlink loop or something like that?

@chrissimpkins
Copy link
Member

related #169

@chrissimpkins
Copy link
Member

We may be able to use a better (i.e. not custom home baked dir traversal) approach.

I am reading this https://stackoverflow.com/a/957978 at the moment.

We should consider @madig's finding in #169 as we address this.

@chrissimpkins
Copy link
Member

We may be able to use a better (i.e. not custom home baked dir traversal) approach.

I am reading this https://stackoverflow.com/a/957978 at the moment.

We should consider @madig's finding in #169 as we address this.

Don't let this scare you away from a PR though if it is more work than you planned. We can begin by simply extending the traversal range.

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

Successfully merging a pull request may close this issue.

2 participants