-
Notifications
You must be signed in to change notification settings - Fork 1
Help automating the update process #173
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?
Conversation
892b108
to
dbeaa2b
Compare
utils/update-elements.sh
Outdated
|
||
local NEW_TAG="${ELEMENT_TAG[$SRC_PROJECT]}" | ||
if [ -z "$NEW_TAG" ]; then | ||
echo "Error: Couldn't get the tag for source junction project `$SRC_PROJECT`." >&2 |
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.
Remove the backticks or it tries to execute $SRC_PROJECT
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.
This element is never used -- lets remove it
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.
What do you mean it's never used? Regardless I'd argue it's a different issue that deserves a different MR.
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.
This element has diverged a lot from FDSDK, I recommend we don't try to automatically merge in changes from upstream as there will always be conflicts.
@@ -1,3 +1,5 @@ | |||
# Derived from gnome-build-meta.bst 48.3 gnomeos/initramfs.bst | |||
|
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.
This element has diverged a lot from GNOME OS, I recommend we don't try to automatically merge in changes from upstream as there will always be conflicts.
@@ -1,3 +1,5 @@ | |||
# Derived from gnome-build-meta.bst 48.3 gnomeos/linux-module-cert.bst | |||
|
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 should remove this completely. It's a dependency of the FDSDK linux.bst element, which we don't use.
The purpose of this element is to provide the public certificate for kernel modules. However in EOS7, we generate that during linux.bst build process.
@@ -1,3 +1,5 @@ | |||
# Derived from gnome-build-meta.bst 48.3 core-deps/libpeas-1.bst | |||
|
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.
There isn't a libpeas-1.bst in gnome-build-meta. GNOME OS only has libpeas v2.
We should ignore everything in the eos/payg
directory as everything there is kludges specific to the PAYG boot components.
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.
While it disappeared in GNOME 49, there is one in 48.3: https://gitlab.gnome.org/GNOME/gnome-build-meta/-/blob/48.3/elements/core-deps/libpeas-1.bst.
elements/eos/repo.bst
Outdated
@@ -1,5 +1,4 @@ | |||
# Adapted from the Freedesktop SDK element `vm/minimal-ostree/repo.bst` | |||
# from Freedesktop SDK 24.08.22. | |||
# Derived from freedesktop-sdk.bst freedesktop-sdk-24.08.22 vm/minimal-ostree/repo.bst |
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.
I suspect this diverged so much from the FDSDK minimal-ostree example that it's not useful to try and integrate changes. Also, the FDSDK minimal-ostree example is largely unmaintained anyway. I think Endless are the only people in the ecosystem using OSTree now.
Left some comments about specific elements -- some of these (esp. removing unused elements) might be better solved separately. When I read a comment like "# Derived from xyz" I assume it's a free-form comment for human use. I wonder if it'd be clearer to put a tag that matches the script name. For example: `# utils/update_elements.sh: Derived from xyz" Then at least it's clear what uses the comment and readers can look up where the comment format is defined. |
fc06720
to
a2e6294
Compare
@ssssam Yeah I considered having a more explicitely robotic header. Let's do this. |
Regarding the elements whose headers should be removed in your opinion: I'd still argue it's better to have these elements be updated so we can cherry-pick the changes, including dismissing them completely, than loosing the link. Also it's easier to remove the headers later than to add them. Ultimately it's up to @starnight to decide. |
This uniformizes the header giving the source of the overriden elements. It will be used to help automating the update process.
This helps updating the overridden elements, as it is otherwise labor-intensive and error-prone.
a2e6294
to
01c7761
Compare
This uniformizes the header giving the source of the overridden elements, and adds utils/update-elements.sh to help updating the overridden elements.
It will help the labor-intensive and error-prone work that is updating the elements to a new gnome-build-meta version.
The script doesn't do everything, as e.g. it only updates .bst elements in
elements/
, and not the files they refer to infiles/
or anywhere else, but it's still a nice help.