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

Simplify git cloning #38

Merged
merged 5 commits into from
Jul 26, 2018
Merged
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
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
download_url=url,
scripts=scripts,
include_package_data=True,
install_requires=["gitpython","pyyaml"],
install_requires=["gitpython"],
classifiers=[
'Intended Audience :: Science/Research',
'Operating System :: OS Independent',
Expand Down
3 changes: 1 addition & 2 deletions ska_conda/pkg_defs/Chandra.Maneuver/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,4 @@ test:


about:
home: https://github.com/sot/Chandra.Maneuver

home: git@github.com:sot/Chandra.Maneuver
2 changes: 1 addition & 1 deletion ska_conda/pkg_defs/annie/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,5 @@ test:


about:
home: https://github.com/sot/annie
home: git@github.com:sot/annie

2 changes: 1 addition & 1 deletion ska_conda/pkg_defs/maude/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,4 @@ test:


about:
home: https://github.com/sot/maude
home: git@github.com:sot/maude
3 changes: 1 addition & 2 deletions ska_conda/pkg_defs/parse_cm/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ test:
imports:
- parse_cm


about:
home: https://github.com/sot/parse_cm
home: git@github.com:sot/parse_cm

2 changes: 0 additions & 2 deletions ska_conda/pkg_defs/ska3_builder/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ requirements:
build:
- python
- pip
- pyyaml
- gitpython
- git
run:
- python
- ipython
- pyyaml
- gitpython
- git
- conda-build
Expand Down
50 changes: 24 additions & 26 deletions ska_conda/ska_builder.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os
import subprocess
import git
import yaml
import re

ska_conda_path = os.path.abspath(os.path.dirname(__file__))
pkg_defs_path = os.path.join(ska_conda_path, "pkg_defs")
Expand All @@ -10,43 +10,28 @@

class SkaBuilder(object):

def __init__(self, build_root='.', user='sot', git_repo_path='git@github.com:{user}/{name}.git'):
self.user = user
self.git_repo_path = git_repo_path
def __init__(self, build_root='.'):
self.build_dir = os.path.abspath(os.path.join(build_root, "builds"))
self.src_dir = os.path.abspath(os.path.join(build_root, "src"))
os.environ["SKA_TOP_SRC_DIR"] = self.src_dir

def _clone_repo(self, name, tag=None):
if name in no_source_pkgs:
return
print("Cloning source %s." % name)
print("Cloning or updating source source %s." % name)
clone_path = os.path.join(self.src_dir, name)
if not os.path.exists(clone_path):
# Try ssh first to avoid needing passwords for the private repos
Copy link
Member

Choose a reason for hiding this comment

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

Awesome!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding continuing, my thought was that there could be other reasons for failure that you might not care about if all you really wanted to do was update a couple of packages at the end of the list. Or if you were a non-privileged user who'd just been handed this code base (JWST?) and this logic lets you really just see how everything works without stopping at parse_cm. You could get in to dependency hell in a fresh build with no access to the currently build packages in ska3-conda, but the only fallout of this change in that case would just be that it would take a lot longer to finish failing out with the rest of the packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, "forgetting ssh key password" was a creditable failure in my testing.

Copy link
Member

Choose a reason for hiding this comment

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

OK, well then put in a text input to ask the user to continue ala install.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK.

# We could add these ssh strings to the meta.yaml for convenience
# The default ssh is not going to work if the package name doesn't match the
# top-level git repo name (cmd_states, eng_archive)
try:
repo = git.Repo.clone_from(self.git_repo_path.format(user=self.user, name=name), clone_path)
assert not repo.bare
print("Cloned via default ssh git")
except:
yml = os.path.join(pkg_defs_path, name, "meta.yaml")
with open(yml) as f:
requires = False
while not requires:
line = f.readline().strip()
if line.startswith("path:"):
requires = True
data = yaml.load(f)
url = data['about']['home']
repo = git.Repo.clone_from(url, clone_path)
print("Cloned from url {}".format(url))
metayml = os.path.join(pkg_defs_path, name, "meta.yaml")
# It isn't clean yaml at this point, so just extract the string we want after "home:"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the previous requires/path loop was dealing with the fact that the meta.yaml isn't compliant YAML at this point at the path key. I just went with a regex here to grab the string after home: instead of trying to use the YAML parser. I think that is fair for our application.

Copy link
Member

Choose a reason for hiding this comment

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

I suspect it could be made into compliant YAML by enclosing the {{ .. }}/.. stuff in double quotes, but I agree with your point to just get the job done here.

meta = open(metayml).read()
url = re.search("home:\s*(\S+)", meta).group(1)
repo = git.Repo.clone_from(url, clone_path)
print("Cloned from url {}".format(url))
else:
repo = git.Repo(clone_path)
repo.remotes.origin.fetch()
repo.remotes.origin.fetch("--tags")
print("Updated repo in {}".format(clone_path))
assert not repo.is_dirty()
# I think we want the commit/tag with the most recent date, though
# if we actually want the most recently created tag, that would probably be
Expand Down Expand Up @@ -85,11 +70,24 @@ def build_one_package(self, name, tag=None):
self._build_package(name)

def build_list_packages(self):
failures = []
with open(build_list, "r") as f:
for line in f.readlines():
pkg_name = line.strip()
if not pkg_name.startswith("#"):
self.build_one_package(pkg_name)
try:
self.build_one_package(pkg_name)
# If there's a failure, confirm before continuing
except:
print(f'{pkg_name} failed, continue anyway (y/n)?')
if input().lower().strip().startswith('y'):
failures.append(pkg_name)
continue
else:
raise ValueError(f"{pkg_name} failed")
if len(failures):
raise ValueError("Packages {} failed".format(",".join(failures)))


def build_all_packages(self):
self.build_list_packages()