Skip to content

Commit e491f9d

Browse files
authored
Merge pull request #6 from compiler-explorer/enhance-version-validation
Enhance version validation with GitHub API integration and improved e…
2 parents c067fda + ff9ea2f commit e491f9d

8 files changed

Lines changed: 218 additions & 54 deletions

File tree

cli/main.py

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ def process_cpp_library(
8888
return
8989
click.echo("✓ Path check passed")
9090

91-
# TODO: Update main repo files for C++
92-
# This will need implementation once we know the exact file structure
9391

9492
# Show diffs if verify flag is set
9593
if verify:
@@ -820,13 +818,7 @@ def main(
820818
config.package_install = True
821819
click.echo("✓ Using CMake package installation for headers")
822820

823-
# Normalize versions by checking git tags
824-
click.echo("Checking git tags for version format...")
825-
config.normalize_versions_with_git_lookup()
826-
if config.target_prefix:
827-
click.echo(
828-
f"✓ Detected version format requires target_prefix: {config.target_prefix}"
829-
)
821+
config.validate_versions_and_exit_on_missing()
830822
else:
831823
# Interactive mode
832824
config = ask_library_questions()

cli/questions.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,6 @@ def ask_library_questions() -> LibraryConfig:
221221
# Create the config and normalize versions with git lookup
222222
config = LibraryConfig(**config_data)
223223

224-
# Normalize versions by checking git tags (only for non-Rust)
225-
if language != Language.RUST:
226-
print("\nChecking git tags for version format...")
227-
config.normalize_versions_with_git_lookup()
228-
if config.target_prefix:
229-
print(f"✓ Detected version format requires target_prefix: {config.target_prefix}")
224+
config.validate_versions_and_exit_on_missing()
230225

231226
return config

core/git_operations.py

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,28 +133,63 @@ def clone_repositories(self) -> tuple[Path, Path]:
133133
return self.main_repo_path, self.infra_repo_path
134134

135135
def create_branch(self, repo_path: Path, branch_name: str):
136-
"""Create a new branch in the given repository, syncing with upstream first"""
137-
# If we have a GitHub token (meaning we're using forks), sync with upstream first
136+
"""Create a new branch in the given repository, syncing with upstream and origin first"""
137+
# If we have a GitHub token (meaning we're using forks), sync everything first
138138
if self.github_token:
139139
try:
140-
# Fetch from upstream to get latest changes
140+
print(f"🔄 Syncing {repo_path.name} with upstream and origin...")
141+
142+
# Fetch from all remotes to get latest changes
141143
self._run_git_command(["git", "fetch", "upstream"], cwd=str(repo_path))
144+
self._run_git_command(["git", "fetch", "origin"], cwd=str(repo_path))
142145

143146
# Make sure we're on main branch
144147
self._run_git_command(["git", "checkout", "main"], cwd=str(repo_path))
145148

146-
# Merge upstream/main into our local main
149+
# Merge upstream/main into our local main (get latest from original repo)
147150
self._run_git_command(["git", "merge", "upstream/main"], cwd=str(repo_path))
148151

149-
# Push updated main to our fork
152+
# Also merge origin/main in case our fork has changes (this handles the fork sync)
153+
try:
154+
self._run_git_command(["git", "merge", "origin/main"], cwd=str(repo_path))
155+
except Exception:
156+
# This might fail if origin/main is behind upstream/main, which is fine
157+
logger.info(f"Origin/main merge not needed for {repo_path.name}")
158+
159+
# Push updated main to our fork to keep it in sync
150160
self._run_git_command(["git", "push", "origin", "main"], cwd=str(repo_path))
151161

162+
print(f"✓ Successfully synced {repo_path.name}")
163+
152164
except Exception as e:
153165
# If syncing fails, log warning but continue
154166
logger.warning(f"Failed to sync with upstream for {repo_path.name}: {e!s}")
167+
print(f"⚠️ Warning: Could not fully sync {repo_path.name}: {e}")
168+
169+
# Check if the branch already exists locally, delete it if so
170+
try:
171+
self._run_git_command(["git", "branch", "-D", branch_name], cwd=str(repo_path))
172+
print(f"🗑️ Deleted existing local branch: {branch_name}")
173+
except Exception:
174+
# Branch doesn't exist locally, that's fine
175+
pass
155176

156-
# Create the new branch from the (hopefully updated) main
157-
self._run_git_command(["git", "checkout", "-b", branch_name], cwd=str(repo_path))
177+
# Check if the branch exists on origin and handle it
178+
try:
179+
self._run_git_command(
180+
["git", "rev-parse", "--verify", f"origin/{branch_name}"], cwd=str(repo_path)
181+
)
182+
# Branch exists on origin, check it out and merge latest main
183+
print(f"📥 Branch {branch_name} exists on origin, checking out and updating...")
184+
self._run_git_command(
185+
["git", "checkout", "-b", branch_name, f"origin/{branch_name}"], cwd=str(repo_path)
186+
)
187+
# Merge the latest main into this branch to ensure it's up to date
188+
self._run_git_command(["git", "merge", "main"], cwd=str(repo_path))
189+
print(f"✓ Updated existing branch {branch_name} with latest main")
190+
except Exception:
191+
# Branch doesn't exist on origin, create it from main
192+
self._run_git_command(["git", "checkout", "-b", branch_name], cwd=str(repo_path))
158193

159194
def get_diff(self, repo_path: Path) -> str:
160195
"""Get the git diff for uncommitted changes"""

core/library_utils.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
import tempfile
55
from pathlib import Path
66

7-
from .subprocess_utils import run_command, run_make_command
7+
from .models import LibraryType
8+
from .subprocess_utils import run_ce_install_command, run_command, run_make_command
89

910
logger = logging.getLogger(__name__)
1011

@@ -265,8 +266,6 @@ def detect_library_type_from_analysis(
265266
Returns:
266267
Tuple of (is_valid, library_type_value)
267268
"""
268-
from .models import LibraryType
269-
270269
# First check existing configuration if available
271270
if existing_config:
272271
# Check if it's explicitly marked as header-only via build_type
@@ -339,8 +338,6 @@ def check_ce_install_link_support(infra_path: Path) -> dict[str, bool]:
339338
Returns:
340339
Dict with support status for different link parameters
341340
"""
342-
from .subprocess_utils import run_ce_install_command
343-
344341
try:
345342
help_result = run_ce_install_command(
346343
["cpp-library", "add", "--help"], cwd=infra_path, debug=False
@@ -403,6 +400,11 @@ def build_ce_install_command(
403400
else:
404401
logger.warning("No library type specified, using default behavior")
405402

403+
# Add target prefix if specified
404+
if hasattr(config, "target_prefix") and config.target_prefix:
405+
subcommand.extend(["--target-prefix", config.target_prefix])
406+
logger.info(f"Adding target prefix: {config.target_prefix}")
407+
406408
# Add package install flag if specified
407409
if hasattr(config, "package_install") and config.package_install:
408410
subcommand.append("--package-install")

core/models.py

Lines changed: 118 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
from __future__ import annotations
22

33
import subprocess
4+
import tempfile
5+
import urllib.request
46
from enum import Enum
57
from pathlib import Path
68

79
import yaml
810
from pydantic import BaseModel, HttpUrl, field_validator
911

12+
from .subprocess_utils import run_command
13+
1014

1115
class Language(str, Enum):
1216
C = "C"
@@ -33,8 +37,70 @@ class LibraryType(str, Enum):
3337
CSHARED = "cshared"
3438

3539

40+
def extract_github_repo_info(github_url: str) -> tuple[str, str] | None:
41+
"""Extract owner and repo name from GitHub URL"""
42+
if not github_url:
43+
return None
44+
45+
# Handle various GitHub URL formats
46+
url = github_url.rstrip("/")
47+
if url.startswith("https://github.com/"):
48+
path = url[len("https://github.com/") :]
49+
elif url.startswith("http://github.com/"):
50+
path = url[len("http://github.com/") :]
51+
elif url.startswith("github.com/"):
52+
path = url[len("github.com/") :]
53+
else:
54+
return None
55+
56+
parts = path.split("/")
57+
if len(parts) >= 2:
58+
return parts[0], parts[1]
59+
return None
60+
61+
62+
def check_github_release_exists(github_url: str, version: str) -> bool:
63+
"""Check if a GitHub release/tag exists using GitHub API"""
64+
repo_info = extract_github_repo_info(github_url)
65+
if not repo_info:
66+
return False
67+
68+
owner, repo = repo_info
69+
70+
# Try both with and without 'v' prefix
71+
versions_to_check = [version]
72+
if version.startswith("v"):
73+
versions_to_check.append(version[1:])
74+
else:
75+
versions_to_check.append(f"v{version}")
76+
77+
for test_version in versions_to_check:
78+
url = f"https://api.github.com/repos/{owner}/{repo}/releases/tags/{test_version}"
79+
try:
80+
with urllib.request.urlopen(url, timeout=10) as response:
81+
if response.status == 200:
82+
return True
83+
except (urllib.error.HTTPError, urllib.error.URLError, TimeoutError):
84+
# Try checking tags endpoint if releases endpoint fails
85+
tag_url = f"https://api.github.com/repos/{owner}/{repo}/git/refs/tags/{test_version}"
86+
try:
87+
with urllib.request.urlopen(tag_url, timeout=10) as tag_response:
88+
if tag_response.status == 200:
89+
return True
90+
except (urllib.error.HTTPError, urllib.error.URLError, TimeoutError):
91+
continue
92+
93+
return False
94+
95+
3696
def check_git_tag_exists(repo_url: str, tag: str) -> bool:
3797
"""Check if a git tag exists in the remote repository"""
98+
# First try GitHub API if it's a GitHub URL
99+
if "github.com" in repo_url:
100+
if check_github_release_exists(repo_url, tag):
101+
return True
102+
103+
# Fallback to git ls-remote for non-GitHub repos or if API fails
38104
try:
39105
# Check if tag exists remotely without cloning
40106
result = subprocess.run(
@@ -108,10 +174,6 @@ def check_existing_library_config_remote(repo_url: str, library_id: str) -> dict
108174
Check if a library already exists by temporarily cloning the infra repository.
109175
This is used during interactive detection when we don't have the repo yet.
110176
"""
111-
import tempfile
112-
113-
from .subprocess_utils import run_command
114-
115177
try:
116178
with tempfile.TemporaryDirectory() as tmpdir:
117179
infra_path = Path(tmpdir) / "infra"
@@ -138,16 +200,16 @@ def check_existing_library_config_remote(repo_url: str, library_id: str) -> dict
138200
return None
139201

140202

141-
def determine_version_format(repo_url: str, version: str) -> tuple[str, str | None]:
203+
def determine_version_format(repo_url: str, version: str) -> tuple[str, str | None, bool]:
142204
"""
143205
Determine the actual version format by checking git tags.
144-
Returns (normalized_version, target_prefix)
206+
Returns (normalized_version, target_prefix, version_exists)
145207
"""
146208
if not repo_url:
147209
# No repo URL, can't check tags - just normalize by removing 'v' prefix
148210
if version.startswith("v"):
149-
return version[1:], "v"
150-
return version, None
211+
return version[1:], "v", False
212+
return version, None, False
151213

152214
# Check if version starts with 'v'
153215
if version.startswith("v"):
@@ -157,27 +219,27 @@ def determine_version_format(repo_url: str, version: str) -> tuple[str, str | No
157219

158220
if check_git_tag_exists(repo_url, version_with_v):
159221
# v1.2.3 exists - use it and set target_prefix
160-
return version_without_v, "v"
222+
return version_without_v, "v", True
161223
elif check_git_tag_exists(repo_url, version_without_v):
162224
# Only 1.2.3 exists - user made a mistake, use 1.2.3
163-
return version_without_v, None
225+
return version_without_v, None, True
164226
else:
165-
# Neither exists - assume user intended v1.2.3 format
166-
return version_without_v, "v"
227+
# Neither exists - version doesn't exist
228+
return version_without_v, "v", False
167229
else:
168230
# User entered 1.2.3 - check if both 1.2.3 and v1.2.3 exist
169231
version_without_v = version
170232
version_with_v = f"v{version}"
171233

172-
if check_git_tag_exists(repo_url, version_without_v):
173-
# 1.2.3 exists - use it without prefix
174-
return version_without_v, None
175-
elif check_git_tag_exists(repo_url, version_with_v):
176-
# Only v1.2.3 exists - should use target_prefix
177-
return version_without_v, "v"
234+
if check_git_tag_exists(repo_url, version_with_v):
235+
# v1.2.3 exists - should use target_prefix
236+
return version_without_v, "v", True
237+
elif check_git_tag_exists(repo_url, version_without_v):
238+
# Only 1.2.3 exists - use it without prefix
239+
return version_without_v, None, True
178240
else:
179-
# Neither exists - assume user intended 1.2.3 format
180-
return version_without_v, None
241+
# Neither exists - version doesn't exist
242+
return version_without_v, None, False
181243

182244

183245
class LibraryConfig(BaseModel):
@@ -217,22 +279,30 @@ def validate_version(cls, v):
217279
else:
218280
raise ValueError("Version must be a string or list of strings")
219281

220-
def normalize_versions_with_git_lookup(self):
282+
def normalize_versions_with_git_lookup(self) -> list[str]:
221283
"""
222284
Normalize versions by checking git tags and set target_prefix if needed.
223285
This should be called after the model is fully populated with github_url.
286+
Returns list of any versions that don't exist in the repository.
224287
"""
225288
if not self.github_url:
226-
return
289+
return []
227290

228291
versions = self.get_versions()
229292
normalized_versions = []
230293
target_prefix = None
294+
missing_versions = []
231295

232296
for version in versions:
233-
normalized_version, prefix = determine_version_format(str(self.github_url), version)
297+
normalized_version, prefix, exists = determine_version_format(
298+
str(self.github_url), version
299+
)
234300
normalized_versions.append(normalized_version)
235301

302+
# Track missing versions
303+
if not exists:
304+
missing_versions.append(version)
305+
236306
# Set target_prefix if any version needs it
237307
if prefix:
238308
target_prefix = prefix
@@ -246,6 +316,31 @@ def normalize_versions_with_git_lookup(self):
246316
if target_prefix:
247317
self.target_prefix = target_prefix
248318

319+
return missing_versions
320+
321+
def validate_versions_and_exit_on_missing(self) -> None:
322+
"""
323+
Validate versions for non-Rust libraries and exit with error if any are missing.
324+
This function handles the common pattern of checking versions and failing fast.
325+
"""
326+
if self.language == Language.RUST:
327+
return # Rust versions don't need git tag validation
328+
329+
print("\nChecking git tags for version format...")
330+
missing_versions = self.normalize_versions_with_git_lookup()
331+
332+
if missing_versions:
333+
print("❌ Error: The following versions were not found in the repository:")
334+
for version in missing_versions:
335+
print(f" - {version}")
336+
print("Please check the version numbers and try again.")
337+
exit(1)
338+
else:
339+
print("✓ All versions found in repository")
340+
341+
if self.target_prefix:
342+
print(f"✓ Detected version format requires target_prefix: {self.target_prefix}")
343+
249344
def get_versions(self) -> list[str]:
250345
"""Get list of versions, handling both single and multiple version cases"""
251346
if isinstance(self.version, str):

docs/architecture.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,12 @@ For C/C++ libraries, the tool asks additional questions:
261261
- `cshared`: C shared libraries
262262

263263
#### Version Format Detection
264-
- Performs git tag lookup on remote repository
264+
- Performs git tag lookup on remote repository using GitHub API for enhanced reliability
265265
- Determines if library uses version prefix (e.g., 'v' in 'v1.2.3')
266-
- Preserves original version format in display and properties
266+
- Automatically normalizes user input (removes 'v' prefix) and sets target_prefix appropriately
267+
- Validates all versions exist in the repository before proceeding
268+
- Fails fast with clear error messages for non-existent versions
269+
- Falls back to git ls-remote for non-GitHub repositories
267270

268271
## Performance Considerations
269272

0 commit comments

Comments
 (0)