Skip to content

Commit 2d1d5fb

Browse files
mattgodboltclaude
andcommitted
Fix LibraryBuilder refactoring to maintain exact original behavior
Critical fixes to restore original functionality: - Fix C++ is_already_uploaded() to check annotations (not conan server) - Add C++ executebuildscript/executeconanscript overrides for platform-specific execution - Restore original setCurrentConanBuildParameters format: * Use build_type (not buildtype) * Use compiler.version/compiler.libcxx (with dots, not underscores) * Include flagcollection parameter * Include library/library_version fields - Add cross-platform writeconanscript with Windows support and chmod 755 - Fix C++ get_compiler_type clang-intel hack (maps to "gcc") - Enhance getTargetFromOptions to support both space and equals formats - Add platform parameter to base class constructors - Remove redundant Rust writeconanscript override (now uses base) - Correct test expectations to match actual original behavior All implementations now exactly match main branch behavior. Tests verify original format with build_type, compiler.version, and flagcollection. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 304e925 commit 2d1d5fb

File tree

4 files changed

+125
-53
lines changed

4 files changed

+125
-53
lines changed

bin/lib/base_library_builder.py

Lines changed: 44 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ def __init__(
6161
sourcefolder: str,
6262
install_context: InstallationContext,
6363
buildconfig: LibraryBuildConfig,
64+
platform: LibraryPlatform = LibraryPlatform.Linux,
6465
):
6566
self.logger = logger
6667
self.language = language
@@ -69,6 +70,7 @@ def __init__(
6970
self.sourcefolder = sourcefolder
7071
self.install_context = install_context
7172
self.buildconfig = buildconfig
73+
self.platform = platform
7274

7375
# Common state
7476
self.forcebuild = False
@@ -399,34 +401,44 @@ def setCurrentConanBuildParameters(
399401
self.current_buildparameters_obj["arch"] = arch
400402
self.current_buildparameters_obj["stdver"] = stdver
401403
self.current_buildparameters_obj["flagcollection"] = extraflags
402-
403-
self.current_buildparameters = []
404-
for key, value in self.current_buildparameters_obj.items():
405-
if key == "flagcollection":
406-
continue
407-
self.current_buildparameters.extend(["-s", f"{key}={value}"])
404+
self.current_buildparameters_obj["library"] = self.libid
405+
self.current_buildparameters_obj["library_version"] = self.target_name
406+
407+
self.current_buildparameters = [
408+
"-s",
409+
f"os={buildos}",
410+
"-s",
411+
f"build_type={buildtype}",
412+
"-s",
413+
f"compiler={compilerTypeOrGcc or 'gcc'}",
414+
"-s",
415+
f"compiler.version={compiler}",
416+
"-s",
417+
f"compiler.libcxx={libcxx or 'libstdc++'}",
418+
"-s",
419+
f"arch={arch}",
420+
"-s",
421+
f"stdver={stdver}",
422+
"-s",
423+
f"flagcollection={extraflags}",
424+
]
408425

409426
def writeconanscript(self, buildfolder):
410-
"""Write conan export script."""
411-
scriptfile = os.path.join(buildfolder, "conanexport.sh")
412-
with open(scriptfile, "w", encoding="utf-8") as f:
413-
f.write("#!/bin/bash\n")
414-
f.write("set -ex\n")
415-
f.write(
416-
" ".join(
417-
[
418-
"conan",
419-
"export-pkg",
420-
".",
421-
f"{self.libname}/{self.target_name}@celibs/trunk",
422-
"-r",
423-
"ceserver",
424-
"--force",
425-
]
426-
+ self.current_buildparameters
427-
)
428-
)
429-
f.write("\n")
427+
"""Write conan export script with cross-platform support."""
428+
conanparamsstr = " ".join(self.current_buildparameters)
429+
430+
if self.platform == LibraryPlatform.Windows:
431+
scriptfile = Path(buildfolder) / "conanexport.ps1"
432+
with scriptfile.open("w", encoding="utf-8") as f:
433+
f.write(f"conan export-pkg . {self.libname}/{self.target_name} -f {conanparamsstr}\n")
434+
else:
435+
scriptfile = Path(buildfolder) / "conanexport.sh"
436+
with scriptfile.open("w", encoding="utf-8") as f:
437+
f.write("#!/bin/sh\n\n")
438+
f.write(f"conan export-pkg . {self.libname}/{self.target_name} -f {conanparamsstr}\n")
439+
440+
# Make script executable
441+
scriptfile.chmod(0o755)
430442

431443
def writebuildscript(
432444
self,
@@ -500,8 +512,7 @@ def __init__(
500512
popular_compilers_only,
501513
platform,
502514
):
503-
super().__init__(logger, language, libname, target_name, sourcefolder, install_context, buildconfig)
504-
self.platform = platform
515+
super().__init__(logger, language, libname, target_name, sourcefolder, install_context, buildconfig, platform)
505516
self.check_compiler_popularity = popular_compilers_only
506517

507518
# These will be set by subclasses
@@ -549,7 +560,12 @@ def getStdLibFromOptions(self, options):
549560

550561
def getTargetFromOptions(self, options):
551562
"""Get target from compiler options."""
563+
# Try equals format first: --target=value or -target=value
552564
match = re.search(r"(?:--target|-target)=(\S*)", options)
565+
if match:
566+
return match[1]
567+
# Try space format: -target value
568+
match = re.search(r"-target (\S*)", options)
553569
if match:
554570
return match[1]
555571
return ""

bin/lib/library_builder.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ def __init__(
119119
# C++-specific initialization
120120
self.current_commit_hash = ""
121121

122+
# Set script filename based on platform
123+
self.script_filename = "cebuild.sh"
124+
if self.platform == LibraryPlatform.Windows:
125+
self.script_filename = "cebuild.ps1"
126+
122127
self.history = LibraryBuildHistory(self.logger)
123128

124129
if self.language in _propsandlibs:
@@ -177,6 +182,58 @@ def save_build_logging(self, builtok, buildfolder, extralogtext):
177182
# Call base implementation
178183
return super().save_build_logging(builtok, buildfolder, extralogtext)
179184

185+
def is_already_uploaded(self, buildfolder):
186+
"""Check if build is already uploaded - C++ specific implementation using annotations."""
187+
annotations = self.get_build_annotations(buildfolder)
188+
189+
if "commithash" in annotations:
190+
commithash = self.get_commit_hash()
191+
return commithash == annotations["commithash"]
192+
else:
193+
return False
194+
195+
def _get_script_args(self, scriptfile):
196+
"""Get script execution arguments for C++ builds."""
197+
if self.platform == LibraryPlatform.Linux:
198+
return ["./" + self.script_filename]
199+
elif self.platform == LibraryPlatform.Windows:
200+
return ["pwsh", "./" + self.script_filename]
201+
else:
202+
return ["bash", scriptfile] # fallback
203+
204+
def get_compiler_type(self, compiler):
205+
"""Get compiler type with C++-specific clang-intel hack."""
206+
compilerType = ""
207+
if "compilerType" in self.compilerprops[compiler]:
208+
compilerType = self.compilerprops[compiler]["compilerType"]
209+
else:
210+
raise RuntimeError(f"Something is wrong with {compiler}")
211+
212+
if self.compilerprops[compiler]["compilerType"] == "clang-intel":
213+
# hack for icpx so we don't get duplicate builds
214+
compilerType = "gcc"
215+
216+
return compilerType
217+
218+
def executeconanscript(self, buildfolder):
219+
"""Execute conan script with platform-specific logic."""
220+
if self.install_context.dry_run:
221+
return BuildStatus.Ok
222+
223+
try:
224+
if self.platform == LibraryPlatform.Linux:
225+
subprocess.check_call(["./conanexport.sh"], cwd=buildfolder)
226+
elif self.platform == LibraryPlatform.Windows:
227+
subprocess.check_call(["pwsh", "./conanexport.ps1"], cwd=buildfolder)
228+
else:
229+
# Fallback to base class behavior
230+
return super().executeconanscript(buildfolder)
231+
232+
self.logger.info("Export successful")
233+
return BuildStatus.Ok
234+
except subprocess.CalledProcessError:
235+
return BuildStatus.Failed
236+
180237
def completeBuildConfig(self):
181238
if "description" in self.libraryprops[self.libid]:
182239
self.buildconfig.description = self.libraryprops[self.libid]["description"]

bin/lib/rust_library_builder.py

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ def __init__(
5858
install_context: InstallationContext,
5959
buildconfig: LibraryBuildConfig,
6060
):
61-
# Rust doesn't have sourcefolder in signature
62-
super().__init__(logger, language, libname, target_name, "", install_context, buildconfig)
61+
# Rust doesn't have sourcefolder in signature, uses Linux platform
62+
super().__init__(
63+
logger, language, libname, target_name, "", install_context, buildconfig, LibraryPlatform.Linux
64+
)
6365

6466
if self.language in _propsandlibs:
6567
[self.compilerprops, self.libraryprops] = _propsandlibs[self.language]
@@ -169,15 +171,6 @@ def setCurrentConanBuildParameters(
169171
if libcxx:
170172
self.current_buildparameters.extend(["-s", f"compiler.libcxx={libcxx}"])
171173

172-
def writeconanscript(self, buildfolder):
173-
"""Write conan export script for Rust packages."""
174-
conanparamsstr = " ".join(self.current_buildparameters)
175-
scriptfile = Path(buildfolder) / "conanexport.sh"
176-
with scriptfile.open("w", encoding="utf-8") as f:
177-
f.write("#!/bin/sh\n\n")
178-
f.write(f"conan export-pkg . {self.libname}/{self.target_name} -f {conanparamsstr}\n")
179-
scriptfile.chmod(0o755)
180-
181174
def writeconanfile(self, buildfolder):
182175
underscoredlibname = self.libname.replace("-", "_")
183176
with (Path(buildfolder) / "conanfile.py").open(mode="w", encoding="utf-8") as f:

bin/test/base_library_builder_test.py

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,17 +63,19 @@ def test_setCurrentConanBuildParameters_format(test_builder):
6363
"-s",
6464
"os=Linux",
6565
"-s",
66-
"buildtype=Debug",
66+
"build_type=Debug",
6767
"-s",
6868
"compiler=gcc",
6969
"-s",
70-
"compiler_version=11.1.0",
70+
"compiler.version=11.1.0",
7171
"-s",
72-
"libcxx=libstdc++",
72+
"compiler.libcxx=libstdc++",
7373
"-s",
7474
"arch=x86_64",
7575
"-s",
7676
"stdver=c++17",
77+
"-s",
78+
"flagcollection=",
7779
]
7880

7981
assert test_builder.current_buildparameters == expected
@@ -96,24 +98,26 @@ def test_setCurrentConanBuildParameters_with_defaults(test_builder):
9698
"-s",
9799
"os=Linux",
98100
"-s",
99-
"buildtype=Release",
101+
"build_type=Release",
100102
"-s",
101103
"compiler=gcc", # defaulted
102104
"-s",
103-
"compiler_version=10.2.0",
105+
"compiler.version=10.2.0",
104106
"-s",
105-
"libcxx=libstdc++", # defaulted
107+
"compiler.libcxx=libstdc++", # defaulted
106108
"-s",
107109
"arch=x86",
108110
"-s",
109111
"stdver=",
112+
"-s",
113+
"flagcollection=",
110114
]
111115

112116
assert test_builder.current_buildparameters == expected
113117

114118

115-
def test_setCurrentConanBuildParameters_excludes_flagcollection(test_builder):
116-
"""Test that flagcollection is excluded from conan parameters list."""
119+
def test_setCurrentConanBuildParameters_includes_flagcollection(test_builder):
120+
"""Test that flagcollection is included in conan parameters list."""
117121
test_builder.setCurrentConanBuildParameters(
118122
buildos="Linux",
119123
buildtype="Debug",
@@ -125,12 +129,12 @@ def test_setCurrentConanBuildParameters_excludes_flagcollection(test_builder):
125129
extraflags="-O3",
126130
)
127131

128-
# Verify flagcollection is in the object but not in the parameters list
132+
# Verify flagcollection is in the object
129133
assert test_builder.current_buildparameters_obj["flagcollection"] == "-O3"
130134

131-
# Verify it's not in the parameter list
135+
# Verify it IS in the parameter list (original behavior)
132136
param_strings = " ".join(test_builder.current_buildparameters)
133-
assert "flagcollection" not in param_strings
137+
assert "flagcollection=-O3" in param_strings
134138

135139

136140
def test_conan_parameter_format_for_command_line(test_builder):
@@ -148,17 +152,19 @@ def test_conan_parameter_format_for_command_line(test_builder):
148152
"-s",
149153
"os=Linux",
150154
"-s",
151-
"buildtype=Debug",
155+
"build_type=Debug",
152156
"-s",
153157
"compiler=gcc",
154158
"-s",
155-
"compiler_version=11.1.0",
159+
"compiler.version=11.1.0",
156160
"-s",
157-
"libcxx=libstdc++",
161+
"compiler.libcxx=libstdc++",
158162
"-s",
159163
"arch=x86_64",
160164
"-s",
161165
"stdver=c++17",
166+
"-s",
167+
"flagcollection=",
162168
]
163169

164170
assert conan_cmd == expected_cmd

0 commit comments

Comments
 (0)