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

generate manifest file from the collected_sources #3153

Merged
merged 1 commit into from
Apr 19, 2023
Merged

generate manifest file from the collected_sources #3153

merged 1 commit into from
Apr 19, 2023

Conversation

yash-zededa
Copy link
Collaborator

@yash-zededa yash-zededa commented Apr 14, 2023

The PR will do the below:-

  • Creation of the manifest file using shell script
  • Archiving the manifest file in the collected_source.tar.gz
  • Content of manifest file.
alpine,acl.2.3.1-r0,1ea87358d76d9e006cfc081d6a79dc636ee0ed26,alpine/acl.2.3.1-r0.1ea87358d76d9e006cfc081d6a79dc636ee0ed26
alpine,alpine-baselayout.3.2.0-r16,8a8c96a0ea2fcd824c361aa4438763fa33ee8ca0,alpine/alpine-baselayout.3.2.0-r16.8a8c96a0ea2fcd824c361aa4438763fa33ee8ca0
alpine,alpine-baselayout.3.2.0-r23,348653a9ba0701e8e968b3344e72313a9ef334e4,alpine/alpine-baselayout.3.2.0-r23.348653a9ba0701e8e968b3344e72313a9ef334e4
alpine,apk-tools.2.12.7-r0,03c6b1b0ec20f417cee0f3c1d56795b27871b1c5,alpine/apk-tools.2.12.7-r0.03c6b1b0ec20f417cee0f3c1d56795b27871b1c5
alpine,apk-tools.2.12.9-r3,34d90ac8388e88126893f5d27ea35d304e65e5ab,alpine/apk-tools.2.12.9-r3.34d90ac8388e88126893f5d27ea35d304e65e5ab
alpine,attr.2.5.1-r1,c0b3bcef6de7d3ae5b326994c3282d8b4f524451,alpine/attr.2.5.1-r1.c0b3bcef6de7d3ae5b326994c3282d8b4f524451

@deitch
Copy link
Contributor

deitch commented Apr 14, 2023

A few requests before a formal review:

  • can you squash the commits into a single commit?
  • can you include here (in the comments) a few sample lines of what the output csv will look like?
  • there are many yetus complains

Copy link
Contributor

@deitch deitch left a comment

Choose a reason for hiding this comment

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

Besides the yetus, request to include the manifest inside the tar file.

Makefile Outdated Show resolved Hide resolved
@yash-zededa
Copy link
Collaborator Author

A few requests before a formal review:

  • can you squash the commits into a single commit?
  • can you include here (in the comments) a few sample lines of what the output csv will look like?
  • there are many yetus complains

@deitch

based on reviews we might have changes in the script or in the makefile. I prefer to squash at once when all the changes are reviewed and approved.

will include few lines of output from the manifest file.

will check and fix the yetus stuff if possible.

@shjala
Copy link
Member

shjala commented Apr 18, 2023

@yash-zededa for yetus, copy-pasting the script here might help.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

See comments plus complaints from shellcheck and blanks from the yetus run.

@deitch
Copy link
Contributor

deitch commented Apr 19, 2023

I think you need to rebase.

@yash-zededa
Copy link
Collaborator Author

I think you need to rebase.

Yes, I have synced the fork

@deitch
Copy link
Contributor

deitch commented Apr 19, 2023

Yetus is clean! You do need to sign the DCO though

@eriknordmark
Copy link
Contributor

Your sample manifest in the description is very confusing as it is a single line. Is that what the script outputs? If not, can you fix the description?

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM; nit about the PR description and the script usage line.

Signed-off-by: yash-zededa <yash@zededa.com>

flat directory for collected_source tar file & manifest file for the collected sources

Signed-off-by: yash-zededa <yash@zededa.com>

minor fix in the makefile, used kernel packages for manifest generation

Signed-off-by: yash-zededa <yash@zededa.com>

new line at the end

Signed-off-by: yash-zededa <yash@zededa.com>

added licence info in generate source, moved manifest file to the collected sources tar

Signed-off-by: yash-zededa <yash@zededa.com>

removed verbose logging

Signed-off-by: yash-zededa <yash@zededa.com>

retrigger checks

Signed-off-by: yash-zededa <yash@zededa.com>

minor fixes

Signed-off-by: yash-zededa <yash@zededa.com>

fixed the script line usage

Signed-off-by: yash-zededa <yash@zededa.com>
@yash-zededa
Copy link
Collaborator Author

yash-zededa commented Apr 19, 2023

Your sample manifest in the description is very confusing as it is a single line. Is that what the script outputs? If not, can you fix the description?

alpine,acl.2.3.1-r0,1ea87358d76d9e006cfc081d6a79dc636ee0ed26,alpine/acl.2.3.1-r0.1ea87358d76d9e006cfc081d6a79dc636ee0ed26

The script is generating the manifest data per line. For example the above would be one line.

@eriknordmark eriknordmark merged commit 91b0265 into lf-edge:master Apr 19, 2023
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.

4 participants