Skip to content

Build: use same hack for VCS and build environments #9055

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 31, 2022
Merged

Build: use same hack for VCS and build environments #9055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 24 additions & 23 deletions readthedocs/doc_builder/director.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,30 @@ def setup_vcs(self):
),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

These two conditionals will have moved to executing in the context manager. It doesn't seem important that either are executed outside the environment in the first place. But in case anyone has any more information about why this is, these seem to be the only logic order changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

This code was being already executed inside the context manager. The difference now is that instead of the context manager being executed inside the BuildDirector is executed in the Celery task, so we can call an extra method there (sync_versions), which needs to be executed inside the context manager (Docker environment)


environment = self.data.environment_class(
before_vcs.send(
sender=self.data.version,
environment=self.vcs_environment,
)

# Create the VCS repository where all the commands are going to be
# executed for a particular VCS type
self.vcs_repository = self.data.project.vcs_repo(
version=self.data.version.slug,
environment=self.vcs_environment,
verbose_name=self.data.version.verbose_name,
version_type=self.data.version.type,
)

self.pre_checkout()
self.checkout()
self.post_checkout()

commit = self.data.build_commit or self.vcs_repository.commit
if commit:
self.data.build["commit"] = commit

def create_vcs_environment(self):
self.vcs_environment = self.data.environment_class(
project=self.data.project,
version=self.data.version,
build=self.data.build,
Expand All @@ -74,28 +97,6 @@ def setup_vcs(self):
# ca-certificate package which is compatible with Lets Encrypt
container_image=settings.RTD_DOCKER_BUILD_SETTINGS["os"]["ubuntu-20.04"],
)
with environment:
before_vcs.send(
sender=self.data.version,
environment=environment,
)

# Create the VCS repository where all the commands are going to be
# executed for a particular VCS type
self.vcs_repository = self.data.project.vcs_repo(
version=self.data.version.slug,
environment=environment,
verbose_name=self.data.version.verbose_name,
version_type=self.data.version.type,
)

self.pre_checkout()
self.checkout()
self.post_checkout()

commit = self.data.build_commit or self.vcs_repository.commit
if commit:
self.data.build["commit"] = commit

def create_build_environment(self):
self.build_environment = self.data.environment_class(
Expand Down
17 changes: 13 additions & 4 deletions readthedocs/projects/tasks/builds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,11 +543,20 @@ def execute(self):

# Clonning
self.update_build(state=BUILD_STATE_CLONING)
self.data.build_director.setup_vcs()

# Sync tags/branches from VCS repository into Read the Docs' `Version`
# objects in the database
self.sync_versions(self.data.build_director.vcs_repository)
# TODO: remove the ``create_vcs_environment`` hack. Ideally, this should be
# handled inside the ``BuildDirector`` but we can't use ``with
# self.vcs_environment`` twice because it kills the container on
# ``__exit__``
self.data.build_director.create_vcs_environment()
with self.data.build_director.vcs_environment:
self.data.build_director.setup_vcs()

# Sync tags/branches from VCS repository into Read the Docs'
# `Version` objects in the database. This method runs commands
# (e.g. "hg tags") inside the VCS environment, so it requires to be
# inside the `with` statement
self.sync_versions(self.data.build_director.vcs_repository)

# TODO: remove the ``create_build_environment`` hack. Ideally, this should be
# handled inside the ``BuildDirector`` but we can't use ``with
Expand Down