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

eext: Adding source bundle capability for eext packages to invoke using repoName #88

Merged
merged 8 commits into from
Nov 2, 2023

Conversation

manith-arista
Copy link
Contributor

@manith-arista manith-arista commented Sep 6, 2023

Created new feature to support source bundles for eext packages.
Eext packages can now invoke pre-defined source bundles to define source and signature of srpm, which will allow easier version control of srpms, distro upgrades and less clutter in manifest files.

manifest/manifest.go Outdated Show resolved Hide resolved
manifest/manifest.go Outdated Show resolved Hide resolved
impl/mock_cfg_test.go Outdated Show resolved Hide resolved
impl/create_srpm.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@aajith-arista aajith-arista left a comment

Choose a reason for hiding this comment

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

Hi Manith,
Thanks for doing this cleanly.
I've some suggestions on how the type hierarchies should be.
General approach is good.

Thanks,
Arun

@manith-arista manith-arista force-pushed the manith-renameUrl branch 2 times, most recently from 7dcd606 to 91a09e2 Compare September 8, 2023 11:35
Copy link
Collaborator

@aajith-arista aajith-arista left a comment

Choose a reason for hiding this comment

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

Hi Manith,
After you go through this, let's have a call to make sure we're on the same page or if you want reasoning for some of my suggestions

manifest/manifest.go Outdated Show resolved Hide resolved
manifest/manifest.go Outdated Show resolved Hide resolved
manifest/testData/sampleManifest1.yaml Outdated Show resolved Hide resolved
configfiles/srcconfig.yaml Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
impl/create_srpm.go Outdated Show resolved Hide resolved
impl/create_srpm.go Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
impl/create_srpm.go Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
@manith-arista manith-arista force-pushed the manith-renameUrl branch 2 times, most recently from 3857525 to b447e27 Compare September 13, 2023 11:14
@manith-arista manith-arista self-assigned this Sep 14, 2023
Copy link
Collaborator

@aajith-arista aajith-arista left a comment

Choose a reason for hiding this comment

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

Hi Manith
Thanks for making the changes previously requested.
A few more changes, but it's quite close now

srcconfig/srcconfig_test.go Outdated Show resolved Hide resolved
srcconfig/srcconfig_test.go Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
impl/create_srpm.go Outdated Show resolved Hide resolved
impl/create_srpm.go Outdated Show resolved Hide resolved
impl/create_srpm.go Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
srcconfig/srcconfig.go Outdated Show resolved Hide resolved
aajith-arista
aajith-arista previously approved these changes Sep 19, 2023
Copy link
Collaborator

@aajith-arista aajith-arista left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of all the comments patiently.
Nicely done!

Copy link
Collaborator

@aajith-arista aajith-arista left a comment

Choose a reason for hiding this comment

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

LGTM
Will approve once all the 3rd party muts are also ready

To support srpms to be hosted on artifactory repo, we provide full-url option.
Also added support for source bundles, which consist of a predefined format,
based on release version.
Created template for source bundle with srcconfig, for packages to refer
using repoName.
Added logic to expand url template of source rpms fro pacakge manifest data.
Updated tests to utilize the source bundles instead of srpm urls.
Added viper setup configuration and default values for srcconfig parameters.
Added logic to parse srcconfig template, read package manifest and
retrieve corresponding source bundle for building srpm.
Packages can specify {.Host} parameter in thier manifests instead of
specifying the source repo url.
Verify functionality of source config bundle by updating the test case
to obtain srpm for repo using custom src bundle.
Release macro in eext packages' spec files were created with incorrect
values. Issue arises due to naming the rpmReleaseMacro as 'release'
too, which conflicts with the macro for 'Release'. Instead using
'eext_release' to denote rpmReleaseMacro to resolve conflict.
Copy link
Collaborator

@aajith-arista aajith-arista left a comment

Choose a reason for hiding this comment

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

Thanks for getting this done

@manith-arista manith-arista merged commit 1fda617 into main Nov 2, 2023
2 checks passed
@manith-arista manith-arista deleted the manith-renameUrl branch November 2, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants