Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@

# We don't want CRLF conversion or any automatic change in there
tests/python/third_party/** binary

# LFS
requirements/**/*.zip filter=lfs diff=lfs merge=lfs -text
Comment thread
julien-lang marked this conversation as resolved.
13 changes: 13 additions & 0 deletions docs/descriptor.rst
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,12 @@ descriptor is defined as the most recent commit for a given branch.
app download. The git executable is, however, not needed during descriptor
resolve and normal operation.

.. note:: If a repository uses `Git LFS <https://git-lfs.com>`_ to store some of its files
(declared via ``filter=lfs`` entries in its ``.gitattributes``), the machine
downloading the descriptor also needs ``git-lfs`` installed and initialized
(``git lfs install``). If it isn't, Toolkit will raise an error rather than
silently checking out files that still contain unresolved LFS pointer text.


Tracking against releases on Github
===================================
Expand Down Expand Up @@ -357,6 +363,13 @@ A token must be set as environment variable that is specific to the organization

.. note:: For private repos, it's recommended that you use a personal access token (classic) with read-only access to Content. Fine-grained tokens are not yet supported. For more information, see the `Github Documentation on Personal Access Tokens <https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token>`_.

.. note:: If the repository uses `Git LFS <https://git-lfs.com>`_, its
`"Include Git LFS objects in archives" <https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/managing-git-lfs-objects-in-archives-of-your-repository>`_
setting must be enabled on Github. This descriptor downloads a Release's zip archive rather than
doing a git clone, and Github only includes real Git LFS content in that archive when this setting
is turned on; otherwise the downloaded files will contain unresolved LFS pointer text. This setting
isn't exposed through the Github API and must be checked/enabled manually per repository.


Pointing to a path on disk
==========================
Expand Down
50 changes: 50 additions & 0 deletions python/tank/descriptor/io_descriptor/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# By accessing, using, copying or modifying this work you indicate your
# agreement to the Shotgun Pipeline Toolkit Source Code License. All rights
# not expressly granted therein are reserved by Shotgun Software Inc.
import json
import os
import subprocess
import tempfile
Expand Down Expand Up @@ -202,9 +203,58 @@
)
log.debug("Execution successful. stderr/stdout: '%s'" % output)

self._validate_lfs_content(target_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just thinking out loud, not a real request from my side.

What if we validate the lfs content earlier to prevent doing a lot of steps that will end up meaningless when LFS is not installed? That way we can warn the user in advance.

I'm not sure if that will be possible to be honest.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's not possible unfortunately. We first need to clone the repo before indentifying if it's using LFS.


# return the last returned stdout/stderr
return output

def _validate_lfs_content(self, repo_path: str) -> None:
"""
Checks that Git LFS tracked files in the checked out repo were
actually resolved to their real content, rather than left as
literal pointer text. This happens silently when git-lfs isn't
installed/registered on this machine - git checks out the pointer
text with no error of its own.

:param repo_path: path to a checked out git repository.
:raises TankGitError: if the repo uses Git LFS but git-lfs isn't
available, or some LFS content wasn't downloaded.
"""
Comment thread
julien-lang marked this conversation as resolved.
gitattributes_path = os.path.join(repo_path, ".gitattributes")
try:
with open(gitattributes_path, "r") as fh:
uses_lfs = "filter=lfs" in fh.read()
Comment thread
julien-lang marked this conversation as resolved.
Comment thread
julien-lang marked this conversation as resolved.
except OSError:
uses_lfs = False

if not uses_lfs:
return

try:
output = _check_output(
"git lfs ls-files --json",
cwd=repo_path,
shell=True,
)

Check warning on line 238 in python/tank/descriptor/io_descriptor/git.py

View check run for this annotation

ShotGrid Chorus / security/bandit

B604: any_other_function_with_shell_equals_true

Function call with shell=True parameter identified, possible security issue. secure coding id: SEC-PY-INV-003.
except SubprocessCalledProcessError as err:
raise TankGitError(
f"{self} uses Git LFS to store some of its files, but git-lfs "
"does not appear to be installed on this machine. Install "
"it from https://git-lfs.com, run `git lfs install`, and "
"try again."
) from err

files = json.loads(output).get("files") or []
missing = [f["name"] for f in files if not f.get("checkout")]
if missing:
Comment thread
julien-lang marked this conversation as resolved.
raise TankGitError(
"Git LFS content for the following file(s) in %s was not "
"downloaded correctly - they still contain pointer text "
"instead of their real content: %s. Make sure git-lfs is "
"installed (https://git-lfs.com) and run `git lfs install`, "
"then try again." % (self, ", ".join(missing))
)

def _tmp_clone_then_execute_git_commands(self, commands, depth=None, ref=None):
"""
Clone into a temp location and executes the given
Expand Down
Binary file modified requirements/3.10/pkgs.zip
Binary file not shown.
Binary file modified requirements/3.11/pkgs.zip
Binary file not shown.
Binary file modified requirements/3.13/pkgs.zip
Binary file not shown.
Binary file modified requirements/3.9/pkgs.zip
Binary file not shown.
Binary file modified requirements/any/flow_data_sdk-beta.zip
Binary file not shown.
141 changes: 141 additions & 0 deletions tests/descriptor_tests/test_git_lfs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
# Copyright (c) 2026 Shotgun Software Inc.
#
# CONFIDENTIAL AND PROPRIETARY
#
# This work is provided "AS IS" and subject to the Shotgun Pipeline Toolkit
# Source Code License included in this distribution package. See LICENSE.
# By accessing, using, copying or modifying this work you indicate your
# agreement to the Shotgun Pipeline Toolkit Source Code License. All rights
# not expressly granted therein are reserved by Shotgun Software Inc.

import os
import shutil
import subprocess

Check notice on line 13 in tests/descriptor_tests/test_git_lfs.py

View check run for this annotation

ShotGrid Chorus / security/bandit

B404: blacklist

Consider possible security implications associated with the subprocess module. secure coding id: SEC-PY-INV-003.
import tempfile
import unittest.mock

import sgtk
from tank_test.tank_test_base import (
ShotgunTestBase,
_is_git_lfs_missing,
_is_git_missing,
setUpModule, # noqa
skip_if_git_lfs_missing,
skip_if_git_missing,
)

LFS_FILE_NAME = "sample.dat"
LFS_FILE_CONTENT = "hello lfs content for tk-core tests\n"


@skip_if_git_missing
@skip_if_git_lfs_missing
class TestGitLFSIODescriptor(ShotgunTestBase):
"""
Testing the Git LFS validation performed by IODescriptorGit.
"""

@classmethod
def setUpClass(cls):
"""
Builds, once for the whole test class, a small local git repo with a
single file tracked via Git LFS. This repo is used as a read-only
clone source by every test - no need to check in a repo fixture,
it's trivial and fast to (re)build on demand.
"""
super().setUpClass()

cls.git_lfs_repo_uri = None
if _is_git_missing() or _is_git_lfs_missing():
# tests are skipped in this case, no need to build the repo
return

cls.git_lfs_repo_uri = tempfile.mkdtemp(prefix="tk_test_lfs_repo_")
env = dict(
os.environ,
GIT_AUTHOR_NAME="tk-core tests",
GIT_AUTHOR_EMAIL="tk-core-tests@example.com",
GIT_COMMITTER_NAME="tk-core tests",
GIT_COMMITTER_EMAIL="tk-core-tests@example.com",
)

def _run(*args):
subprocess.check_call(args, cwd=cls.git_lfs_repo_uri, env=env)

Check notice on line 63 in tests/descriptor_tests/test_git_lfs.py

View check run for this annotation

ShotGrid Chorus / security/bandit

B603: subprocess_without_shell_equals_true

subprocess call - check for execution of untrusted input. secure coding id: SEC-PY-INV-003.

_run("git", "init", "-q", "-b", "master")
Comment thread
julien-lang marked this conversation as resolved.
_run("git", "lfs", "install", "--local")
Comment thread
julien-lang marked this conversation as resolved.
with open(os.path.join(cls.git_lfs_repo_uri, LFS_FILE_NAME), "w") as fh:
fh.write(LFS_FILE_CONTENT)
_run("git", "lfs", "track", LFS_FILE_NAME)
_run("git", "add", ".gitattributes", LFS_FILE_NAME)
_run("git", "commit", "-q", "-m", "initial commit with an lfs file")

@classmethod
def tearDownClass(cls):
if cls.git_lfs_repo_uri:
shutil.rmtree(cls.git_lfs_repo_uri, ignore_errors=True)
super().tearDownClass()

def setUp(self):
"""
Sets up the next test's environment.
"""
ShotgunTestBase.setUp(self)

# each test gets its own bundle cache so a download in one test can't
# be mistaken for an already-resolved download in another
self.bundle_cache = os.path.join(
self.project_root, "bundle_cache_%s" % self._testMethodName
)

def _create_desc(
self,
location,
resolve_latest=False,
desc_type=sgtk.descriptor.Descriptor.CONFIG,
):
"""
Helper method around create_descriptor
"""
return sgtk.descriptor.create_descriptor(
self.mockgun,
desc_type,
location,
bundle_cache_root_override=self.bundle_cache,
resolve_latest=resolve_latest,
)

def test_lfs_content_resolved(self):
"""
A repo whose Git LFS content resolves normally should check out fine.
"""
location_dict = {
"type": "git_branch",
"path": self.git_lfs_repo_uri,
"branch": "master",
}

desc = self._create_desc(location_dict, True)
desc.ensure_local()

lfs_file_path = os.path.join(desc.get_path(), LFS_FILE_NAME)
with open(lfs_file_path, "r") as fh:
self.assertEqual(fh.read(), LFS_FILE_CONTENT)

def test_lfs_content_unresolved(self):
"""
If Git LFS content is checked out as unresolved pointer text (e.g.
git-lfs wasn't registered on the machine that did the clone), Toolkit
should raise rather than silently use the pointer file as-is.
"""
location_dict = {
"type": "git_branch",
"path": self.git_lfs_repo_uri,
"branch": "master",
}

desc = self._create_desc(location_dict, True)

with unittest.mock.patch.dict(os.environ, {"GIT_LFS_SKIP_SMUDGE": "1"}):
with self.assertRaises(sgtk.descriptor.errors.TankDescriptorError):
desc.ensure_local()
24 changes: 24 additions & 0 deletions tests/python/tank_test/tank_test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,30 @@
return unittest.skipIf(_is_git_missing(), "git is missing from PATH")(func)


def _is_git_lfs_missing():
"""
Tests is git-lfs is available in PATH
:returns: True is git-lfs is available, False otherwise.
"""
git_lfs_missing = True
try:
sgtk.util.process.subprocess_check_output(["git", "lfs", "version"])
git_lfs_missing = False
except Exception:
# no git-lfs!
pass

Check notice on line 133 in tests/python/tank_test/tank_test_base.py

View check run for this annotation

ShotGrid Chorus / security/bandit

B110: try_except_pass

Try, Except, Pass detected. secure coding id: SEC-PY-ERR-001.
return git_lfs_missing


def skip_if_git_lfs_missing(func):
"""
Decorator that allows to skip a test if git-lfs is missing.
:param func: Function to be decorated.
:returns: The decorated function.
"""
return unittest.skipIf(_is_git_lfs_missing(), "git-lfs is missing from PATH")(func)


def _is_pyside_missing():
"""
Tests is PySide is available.
Expand Down