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

[pkg_deb] Fix multiline fields in changes file #691

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

Homulvas
Copy link
Contributor

@Homulvas Homulvas commented Mar 29, 2023

Fix broken .changes file reported in #632
As per documentation https://www.debian.org/doc/debian-policy/ch-controlfields.html#debian-changes-files-changes
Files and checksum fields always start with an empty line.
Fix this by marking the fields as is_multiline=True. Also remove the trailing whitespace after a single line includes just the field key. Added a test that checks for an exact match to generated .changes file.

fixes #632
fixes #659

@google-cla
Copy link

google-cla bot commented Mar 29, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Homulvas Homulvas marked this pull request as draft March 30, 2023 07:06
@Homulvas
Copy link
Contributor Author

Since I hardcoded the checksums in the test case the tests now show that on macos the resulting deb has a different checksum and the date is off by one hour in the changes file. I think ideally this rule should produce an identical package across different environments.

@Homulvas Homulvas marked this pull request as ready for review March 30, 2023 08:12
@Homulvas Homulvas changed the title fix multiline fields in changes files Fix multiline fields in changes file Mar 30, 2023
@Homulvas Homulvas changed the title Fix multiline fields in changes file [pkg_deb] Fix multiline fields in changes file Mar 30, 2023
@Homulvas Homulvas force-pushed the fix-debs branch 2 times, most recently from 5a1b8a0 to 4bf01b4 Compare March 30, 2023 12:01
pkg/private/deb/make_deb.py Outdated Show resolved Hide resolved
@Homulvas Homulvas requested a review from aiuto March 30, 2023 18:03
@aiuto
Copy link
Collaborator

aiuto commented Apr 4, 2023

So then the question is if that is correct or not. IRRC, there were another set of bugs about the control file fields being wrong. I don't recall if they were on github or internal.
Do you have pointers to the definitive specs. I'm going to be tied up for a while and won't have time to look deeply into that before end of April.

@Homulvas
Copy link
Contributor Author

Homulvas commented Apr 4, 2023

https://www.debian.org/doc/debian-policy/ch-controlfields.html#description describes how Description is handled differently between control and changes files.
If you grep is always empty in the doc you'll find that it mentions Files, Changes, and Checksums-x these are the fields that were changed to be multiline in this PR (these fields are not part of the control file anyway).

@phlax
Copy link

phlax commented Apr 4, 2023

i would suggest using dscverify to validate the package files

@Homulvas
Copy link
Contributor Author

Homulvas commented Apr 5, 2023

i would suggest using dscverify to validate the package files

before

$ dscverify fizzbuzz_4.5.6_all.changes --no-sig-check
fizzbuzz_4.5.6_all.changes:
dscverify: no file spec lines in fizzbuzz_4.5.6_all.changes
Validation FAILED!!
$ changestool fizzbuzz_4.5.6_all.changes verify
Checking Source packages...
Checking Binary consistency...
ERROR: 'fizzbuzz_4.5.6_all.changes' has binary 'fizzbuzz' in 'Binary:' and 'Description:' header, but no files for it found!
Checking checksums...

after

$ dscverify fizzbuzz_4.5.6_all.changes --no-sig-check
fizzbuzz_4.5.6_all.changes:
   validating fizzbuzz_4.5.6_all.deb
All files validated successfully.
$ changestool fizzbuzz_4.5.6_all.changes verify
Checking Source packages...
Checking Binary consistency...
Checking checksums...

@Homulvas Homulvas requested a review from aiuto April 11, 2023 09:29
pkg/private/deb/make_deb.py Show resolved Hide resolved
@Homulvas
Copy link
Contributor Author

@aiuto can you land this as I don't have access to that?

@aiuto aiuto merged commit 5932519 into bazelbuild:main Jun 5, 2023
@Homulvas Homulvas deleted the fix-debs branch June 14, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants