-
Notifications
You must be signed in to change notification settings - Fork 190
CFE-4341: Adjusted package module inventory to include quotes around fields when needed #5462
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
Conversation
@cf-bottom jenkins with exotics |
Alright, I triggered a build: (with exotics) Jenkins: https://ci.cfengine.com/job/pr-pipeline/10516/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10516/ |
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.
👍
@cf-bottom jenkins with exotics please, thanks. |
Sure, I triggered a build: (with exotics) Jenkins: https://ci.cfengine.com/job/pr-pipeline/10518/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10518/ |
Looks like the architecture is coming out as In the test failure:
and in the test policy:
|
The |
Any update on this or a workflow for me to apply the change manually while waiting on this PR to get approved |
a01ff9b
to
838585b
Compare
Hi @gettoknowmii, @craigcomstock is taking some personal time off, thus I made an attempt to fix it while he is gone. @cf-bottom can you run this through Jenkins? |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/10537/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10537/ |
I was able to reproduce on master:
Tested with this branch:
and it seems to work 🎉 |
Reproduced on Ubuntu 22 by adding these three lines sys.stdout.write("Name=bogus\n")
sys.stdout.write("Version=1.4.8,1\n")
sys.stdout.write("Architecture=amd64\n") to end of |
@craigcomstock can you squash the commits on Monday? I'm not allowed to :( |
e7fcb7f
to
97f27bb
Compare
…n needed On FreeBSD where package versions can have commas such as joe-4.6,1 this was causing errors with the use of packagesmatching() function. ``` error: Line from package inventory 'joe,4.6,1,amd64,pkg' did not yield correct number of elements. ``` Used CsvWriterField to quote the field in case it includes " , \r \n. See CsvWriterField() at https://github.com/NorthernTechHQ/libntech/blob/master/libutils/csv_writer.c#L73 Had to add a workaround to convert CRLF written by csv_writer to expected LF in resulting inventory_list. Ticket: CFE-4341 Changelog: title Co-authored-by: Lars Erik Wik <[email protected]>
97f27bb
to
27fdfb5
Compare
Tested this on freebsd 13.2 and looks good.
gives
Which includes quotes where commas are present in versions. |
@gettoknowmii are you able to clone, build and install? I have merged this PR so the fix is available in master branch. Instructions are here: https://github.com/cfengine/core/blob/master/docs/BSD.md |
@craigcomstock thank you. I will go ahead and build and install and test this. |
This works and solves my issue. Thank you so much @craigcomstock. I will be opening a PR with https://bugs.freebsd.org/ so they can patch it upstream. Thanks to everyone who worked on this. |
@gettoknowmii, thanks for testing 🚀 |
On FreeBSD where package versions can have commas such as joe-4.6,1 this was causing errors with the use of packagesmatching() function.
Ticket: CFE-4341
Changelog: title