don't export GIT_DIR

[ruro]

See #8288 for more info. Fixes #8288.

[humitos]

Hi @ruro and thanks for your PR. I haven't tested the code locally yet (*), but it seems there are 16 tests failing after this change. Can you take a look into that first?

(*) a good test would be to build test-builds with the git-lfs branch without modifications and it should pass and show the image in the page.

[ruro]

Sorry for the delay. I was really busy last week. Also, I had some issues with installing the readthedocs developer setup, but I was able to resolve them.

---

Managing the different environment variables "properly" is kind of a huge pain with the current readthedocs architecture. Environment variables are stored inside the LocalEnvironment/BuildEnvironment/etc objects, but these objects seem to have longer lifetime than a single BaseVCS (e.g. git.py:Backend) and the vcs backend just .updates the environment dict, effectively leaking the environment variable to later steps. Additionally, in this situation, not exporting GIT_WORK_TREE breaks any use cases, which attempt to use git to modify the worktree, but exporting it seems to break submodules.

So, after spending a lot of time trying to fix this "properly", I decided to try just removing the GIT_DIR environment export altogether instead of adding GIT_WORK_TREE. It seems, that the GIT_DIR export wasn't actually used anywhere since the cwd is always inside the relevant repo (at least it seems to be the case in all the tests).

I've also built the git-lfs branch from the test-builds repo and the image is displayed correctly. Although the is some error in the latexmk step (I don't use latex myself, so I am not 100% sure, what is the issue there).

[stsewd]

So, we always set the cwd to self.working_dir

readthedocs.org/readthedocs/vcs_support/base.py

Lines 101 to 104 in babbdf0

	kwargs.update({
	'cwd': self.working_dir,
	'shell': False,
	})

So it makes sense to just remove that env var 👍

[ericholscher]

I do worry that this might break things -- but let's try it and see if something else blows up :/ We might end up reverting pretty quickly, but it does seem like it isn't needed. I imagine some users could be depending on this somehow though :/

[ruro]

I imagine some users could be depending on this somehow though :/

I think, that prior to #8263, these environment variables weren't even visible in the step that ran the users conf.py. In fact, that's why git-lfs broke in the first place. So, in theory, any breakage should be entirely in the readthedocs codebase, and it is hopefully covered by all the tests. Hopefully. 😅

[ericholscher]

I'm guessing this is actually breaking from expand_vars=False (https://github.com/readthedocs/readthedocs.org/pull/8263/files#diff-2fc9f70fc71269000cbbc336a139bcb62fe63f5121d299eb5e207d8aa593bad1R83) being passed, and not the GIT_DIR? I'm wondering if git-lfs is depending on some kind of env variable in the backend of git to do this work.