Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
14 changes: 1 addition & 13 deletions docs/design/bundler.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ nokogiri-1.16.6.gem
tzinfo-2.0.6.gem
```

Notice that all the `.gem` dependencies are kept in their original format, and Git dependencies are just plain clones

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Commit message:

Use bare git clones (git clone --bare) instead of full clones for
git dependencies. Bare clones contain only git objects without a
working tree, which is all that bundler needs to resolve the source.

This is all true, undisputed, but I'd like to see more context in the message for bookkeeping reasons, i.e. this commit deletes the BUNDLE_LOCAL_* variables and converts to bare git clones but doesn't discuss why that is okay to do, especially since newer bundler would likely complain. Therefore, I'd like to see context expansion saying something along the lines of the following:

  • the design was originally based on newer bundler than we tested for which differs in the git gem cache format
  • adjust the design to favour the older bundler approach (bare git clones) as tested, the next patch will expand this with proper support for newer bundler and provide version agnostic support

Notice that all the `.gem` dependencies are kept in their original format, and Git dependencies are bare git clones
Comment thread
eskultety marked this conversation as resolved.
of the repository placed in a folder. For Git dependencies, the folder name must match this specific
[format](https://github.com/rubygems/rubygems/blob/3da9b1dda0824d1d770780352bb1d3f287cb2df5/bundler/lib/bundler/source/git.rb#L130):

Expand Down Expand Up @@ -389,8 +389,6 @@ BUNDLE_DEPLOYMENT=true
BUNDLE_NO_PRUNE=true
BUNDLE_ALLOW_OFFLINE_INSTALL=true
BUNDLE_DISABLE_VERSION_CHECK=true
BUNDLE_DISABLE_LOCAL_BRANCH_CHECK=true
Comment thread
eskultety marked this conversation as resolved.
BUNDLE_DISABLE_LOCAL_REVISION_CHECK=true
```

- **BUNDLE_CACHE_PATH**: The directory that Bundler will place cached gems in when running bundle package, and that
Expand All @@ -415,16 +413,6 @@ package cache.
- **BUNDLE_DISABLE_VERSION_CHECK**: Stop Bundler from checking if a newer Bundler version is
available on rubygems.org.

- **BUNDLE_DISABLE_LOCAL_REVISION_CHECK**: Allow Bundler to use a local git override without a
branch specified in the Gemfile

- **BUNDLE_DISABLE_LOCAL_BRANCH_CHECK**: Allow Bundler to use a local git override without checking
if the revision present in the lockfile is present in the repository.

- **BUNDLE_LOCAL__<GEM_NAME>**: Instead of checking out the remote git repository for GEM_NAME,
the local git directory override will be used. See below for more information on Bundler's git
dependency handling.

For more information, see Bundler's [documentation](https://bundler.io/v2.5/man/bundle-config.1.html).

#### Offline installation pitfalls
Expand Down
10 changes: 2 additions & 8 deletions hermeto/core/package_managers/bundler/gem_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,10 @@ class GitDependency(_GemMetadata):
Attributes:
url: The URL of the git repository.
branch: The branch to checkout.
ref: Commit hash.
"""

url: AcceptedUrl
branch: str | None = None
ref: AcceptedGitRef

@cached_property
Expand Down Expand Up @@ -134,17 +132,13 @@ def download_to(self, deps_dir: RootedPath) -> None:
git_repo_path.path.mkdir(parents=True)

log.info("Cloning git repository %s", self.url)
repo = GitRepo.clone_from(
GitRepo.clone_from(
url=str(self.url),
to_path=git_repo_path.path,
bare=True,
env={"GIT_TERMINAL_PROMPT": "0"},
)
Comment thread
eskultety marked this conversation as resolved.

if self.branch is not None:
repo.git.checkout(self.branch)

repo.git.reset("--hard", self.ref)

Comment thread
eskultety marked this conversation as resolved.

class PathDependency(_GemMetadata):
"""
Expand Down
32 changes: 2 additions & 30 deletions hermeto/core/package_managers/bundler/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ def fetch_bundler_source(request: Request) -> RequestOutput:
)
components.extend(_comps)
git_paths.extend(_git_paths)
project_files.append(
_prepare_for_hermetic_build(request.source_dir, request.output_dir, git_paths)
)
project_files.append(_prepare_for_hermetic_build(request.source_dir, request.output_dir))

annotations = []
if backend_annotation := create_backend_annotation(components, "bundler"):
Expand Down Expand Up @@ -203,9 +201,7 @@ def _prepare_environment_variables_for_hermetic_build() -> list[EnvironmentVaria
]


def _prepare_for_hermetic_build(
source_dir: RootedPath, output_dir: RootedPath, git_paths: list | None = None
) -> ProjectFile:
def _prepare_for_hermetic_build(source_dir: RootedPath, output_dir: RootedPath) -> ProjectFile:
"""Prepare a package for hermetic build by injecting necessary config."""
potential_bundle_config = source_dir.join_within_root(".bundle/config").path
hermetic_config = dedent(
Expand All @@ -218,30 +214,6 @@ def _prepare_for_hermetic_build(
BUNDLE_VERSION: "system"
"""
)
# Note: if a package depends on a git revision then the following variables
# are necessary for a hermetic build:
# BUNDLE_DISABLE_LOCAL_BRANCH_CHECK
# BUNDLE_DISABLE_LOCAL_REVISION_CHECK
# because otherwise some (potentially all, depending on exact set of
# ecosystem components versions, environment variables and celestial
# alignment) Bundler versions will try to fetch the latest changes of the
# remotes which may be present even when instructed not to with --local
# flag.
# See https://bundler.io/guides/git.html#local-git-repos for details.
# (or https://github.com/rubygems/bundler-site/blob/
# 9ff3b76e9866524ecefe165633ffb547f0004a99/source/guides/git.html.md
# if the link above ceases to exist).
if git_paths is not None:
hermetic_config += 'BUNDLE_DISABLE_LOCAL_BRANCH_CHECK: "true"\n'
hermetic_config += 'BUNDLE_DISABLE_LOCAL_REVISION_CHECK: "true"\n'
for packname, dirname in git_paths:
# "-" in variable names is deprecated in Bundler and now generates
# a warning and a suggestion to replace all dashes with triple
# underscores. Package names sometimes contain dashes:
varname = "BUNDLE_LOCAL." + packname.upper().replace("-", "___")
location = "${output_dir}/deps/bundler/" + dirname
config_entry = varname + f': "{location}"'
hermetic_config += f"{config_entry}\n"
if potential_bundle_config.is_file():
config_data = potential_bundle_config.read_text()
config_data += hermetic_config
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
version: spec.version,
type: 'git',
url: spec.source.uri,
branch: spec.source.branch,
ref: spec.source.revision
}
parsed_specs << parsed_spec
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_bundler.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def test_bundler_packages(
),
[], # No additional commands are run to verify the build
[],
id="bundler_e2e",
id="bundler_e2e_ruby33",
),
pytest.param(
utils.TestParameters(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,3 @@ project_files:
BUNDLE_ALLOW_OFFLINE_INSTALL: "true"
BUNDLE_DISABLE_VERSION_CHECK: "true"
BUNDLE_VERSION: "system"
BUNDLE_DISABLE_LOCAL_BRANCH_CHECK: "true"
BUNDLE_DISABLE_LOCAL_REVISION_CHECK: "true"
BUNDLE_LOCAL.JSON___SCHEMA: "${output_dir}/deps/bundler/json-schema-26487618a684"
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,3 @@ project_files:
BUNDLE_ALLOW_OFFLINE_INSTALL: "true"
BUNDLE_DISABLE_VERSION_CHECK: "true"
BUNDLE_VERSION: "system"
BUNDLE_DISABLE_LOCAL_BRANCH_CHECK: "true"
BUNDLE_DISABLE_LOCAL_REVISION_CHECK: "true"
BUNDLE_LOCAL.JSON___SCHEMA: "${output_dir}/deps/bundler/json-schema-26487618a684"
4 changes: 2 additions & 2 deletions tests/unit/package_managers/bundler/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ def test_parse_gemlock(
{
"type": "git",
"url": "https://github.com/3scale/json-schema.git",
"branch": "devel",
"ref": GIT_REF,
**base_dep,
},
Expand All @@ -164,7 +163,6 @@ def test_parse_gemlock(
name="example",
version="0.1.0",
url="https://github.com/3scale/json-schema.git",
branch="devel",
ref=GIT_REF,
),
PathDependency(
Expand Down Expand Up @@ -215,6 +213,7 @@ def test_download_git_dependency_works(
mock_git_clone.assert_called_once_with(
url=str(dep.url),
to_path=dep_path,
bare=True,
env={"GIT_TERMINAL_PROMPT": "0"},
)
assert dep_path.exists()
Expand All @@ -241,6 +240,7 @@ def test_download_duplicate_git_dependency_is_skipped(
mock_git_clone.assert_called_once_with(
url=str(dep.url),
to_path=dep_path,
bare=True,
env={"GIT_TERMINAL_PROMPT": "0"},
)
assert dep_path.exists()
Expand Down