-
Notifications
You must be signed in to change notification settings - Fork 3
chore(ci): import libs #1541
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
base: main
Are you sure you want to change the base?
chore(ci): import libs #1541
Conversation
Reviewer's GuideThis PR reorganizes CI image definitions by refactoring import paths, standardizing shell install workflows, cleaning up package dependency lists, and enhancing build steps with stripping and updated component versions. Class diagram for CI image dependency and import structure changesclassDiagram
class CIImage {
+altPackages: list
+packages: list
+binaries: list
+import: list
+shell: steps
}
class VirtLauncher {
+altLibs: list
+binaries: list
+packages: list
+import: list
+shell: steps
}
class Libvirt {
+altPackages: list
+altLibraries: list
+packages: list
+import: list
+shell: steps
}
class Glibc {
+altPackages: list
+packages: list
+patches: list
+import: list
+shell: steps
}
CIImage <|-- VirtLauncher
CIImage <|-- Libvirt
CIImage <|-- Glibc
VirtLauncher --> Libvirt : imports
VirtLauncher --> CIImage : imports other packages
Libvirt --> Glibc : imports
CIImage --> Glibc : imports
Glibc --> CIImage : used by other images
Class diagram for standardized shell install workflowclassDiagram
class ShellWorkflow {
+beforeInstall: steps
+install: steps
+stripBinaries: bool
+treeOutput: bool
}
class CIImage {
+shell: ShellWorkflow
}
CIImage --> ShellWorkflow
ShellWorkflow : now includes stripping and tree output
ShellWorkflow : install steps standardized across images
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
7e06620
to
4ff0ee4
Compare
69987ed
to
d5bcf2b
Compare
images/libvirt/werf.inc.yaml
Outdated
rm -rf /BINS/var | ||
rm -rf /BINS/etc/libvirt/qemu.conf | ||
rm -rf /BINS/etc/libvirt/virtqemud.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add cleaning of other unnecessary files like (it was in script install-libvirt.sh
):
# $SRC_BASE/src/conf/schemas/storagepoolcaps.rng to /usr/share/libvirt/schemas
# $SRC_BASE/src/conf/schemas/storagepool.rng to /usr/share/libvirt/schemas
# $SRC_BASE/src/conf/schemas/storagevol.rng to /usr/share/libvirt/schemas
f80a130
to
453f76d
Compare
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
Signed-off-by: Maksim Fedotov <[email protected]>
8d997db
to
ec65f1c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `images/packages/glibc/werf.inc.yaml:49-53` </location>
<code_context>
- mkdir -p ~/.ssh && echo "StrictHostKeyChecking accept-new" > ~/.ssh/config
+ git clone --depth=1 $(cat /run/secrets/SOURCE_REPO)/{{ $gitRepoUrl }} --revision {{ $version }} /src
+ cd /src
+ for p in /patches/*.patch ; do
+ echo -n "Apply ${p} ... "
+ git apply --ignore-space-change --ignore-whitespace ${p} && echo OK || (echo FAIL ; exit 1)
+ done
</code_context>
<issue_to_address>
**suggestion:** Patch application does not check for empty patch directory.
If /patches contains no .patch files, the loop will run with a non-existent file, causing errors. Please add a check to ensure patch files exist before attempting to apply them.
```suggestion
cd /src
if ls /patches/*.patch 1> /dev/null 2>&1; then
for p in /patches/*.patch ; do
echo -n "Apply ${p} ... "
git apply --ignore-space-change --ignore-whitespace ${p} && echo OK || (echo FAIL ; exit 1)
done
else
echo "No patch files found in /patches, skipping patch application."
fi
```
</issue_to_address>
### Comment 2
<location> `images/packages/libfuse3/werf.inc.yaml:64-68` </location>
<code_context>
install:
- |
+ # Install packages
+ PKGS="{{ $builderDependencies.packages | join " " }}"
+ for pkg in $PKGS; do
+ cp -a /$pkg/. /
+ rm -rf /$pkg
+ done
+
</code_context>
<issue_to_address>
**suggestion:** Package copy loop assumes all package directories exist.
Add a check to skip packages if their directories do not exist to prevent errors during copy and removal.
```suggestion
PKGS="{{ $builderDependencies.packages | join " " }}"
for pkg in $PKGS; do
if [ -d "/$pkg" ]; then
cp -a /$pkg/. /
rm -rf /$pkg
fi
done
```
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
cd /src | ||
for p in /patches/*.patch ; do | ||
echo -n "Apply ${p} ... " | ||
git apply --ignore-space-change --ignore-whitespace ${p} && echo OK || (echo FAIL ; exit 1) | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Patch application does not check for empty patch directory.
If /patches contains no .patch files, the loop will run with a non-existent file, causing errors. Please add a check to ensure patch files exist before attempting to apply them.
cd /src | |
for p in /patches/*.patch ; do | |
echo -n "Apply ${p} ... " | |
git apply --ignore-space-change --ignore-whitespace ${p} && echo OK || (echo FAIL ; exit 1) | |
done | |
cd /src | |
if ls /patches/*.patch 1> /dev/null 2>&1; then | |
for p in /patches/*.patch ; do | |
echo -n "Apply ${p} ... " | |
git apply --ignore-space-change --ignore-whitespace ${p} && echo OK || (echo FAIL ; exit 1) | |
done | |
else | |
echo "No patch files found in /patches, skipping patch application." | |
fi |
PKGS="{{ $builderDependencies.packages | join " " }}" | ||
for pkg in $PKGS; do | ||
cp -a /$pkg/. / | ||
rm -rf /$pkg | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Package copy loop assumes all package directories exist.
Add a check to skip packages if their directories do not exist to prevent errors during copy and removal.
PKGS="{{ $builderDependencies.packages | join " " }}" | |
for pkg in $PKGS; do | |
cp -a /$pkg/. / | |
rm -rf /$pkg | |
done | |
PKGS="{{ $builderDependencies.packages | join " " }}" | |
for pkg in $PKGS; do | |
if [ -d "/$pkg" ]; then | |
cp -a /$pkg/. / | |
rm -rf /$pkg | |
fi | |
done |
Description
import libs
Why do we need it, and what problem does it solve?
What is the expected result?
Checklist
Changelog entries