-
Notifications
You must be signed in to change notification settings - Fork 64
Add Deb822 sources support (issue #149) #150
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: master
Are you sure you want to change the base?
Conversation
gtk/rgrepositorywindow.cc
Outdated
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.
Any reason why were the rgrepositorywin.* files renamed to rgrepositorywindow?
This probably won't work properly with the rest of the build process and also the translatable strings present in these files won't get translated.
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'll revert this, renaming was unintentional
|
I tried this real quick (did not really look into the code) with my "apt modernize-source" updates sources but I only get an empty window when I open the repository dialog. My debian sources: $ cat /etc/apt/sources.list.d/debian.sources
# Modernized from /etc/apt/sources.list
Types: deb deb-src
URIs: http://ftp.de.debian.org/debian/
Suites: trixie
Components: main non-free-firmware
Signed-By: /usr/share/keyrings/debian-archive-keyring.gpg
# Modernized from /etc/apt/sources.list
Types: deb deb-src
URIs: http://security.debian.org/debian-security/
Suites: trixie-security
Components: main non-free-firmware
Signed-By: /usr/share/keyrings/debian-archive-keyring.gpg
# Modernized from /etc/apt/sources.list
Types: deb deb-src
URIs: http://ftp.de.debian.org/debian/
Suites: trixie-updates
Components: main non-free-firmware
Signed-By: /usr/share/keyrings/debian-archive-keyring.gpg |
I've made several improvements, Could you please test again with your file? You should see all three sources listed in the window. |
common/rpackagelister.cc
Outdated
| #endif | ||
| } | ||
|
|
||
| bool RPackageLister::handleFailedInstallation(const string &pkgName) |
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 change looks unrelated to deb822? It looks interesting and maybe worth its own PR but I would prefer if we could keep this PR strictly focused on deb822 support :)
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.
got 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.
I still get compiler errors for this function and when I remove it the build fails now with:
$ make
...
/usr/bin/ld: ../common/libsynaptic.a(rsources.o): in function `SourcesList::ReadDeb822SourcePart(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)':
/home/mvogt/devel/synaptic/trunk.git/common/rsources.cc:592:(.text+0x386e): undefined reference to `RDeb822Source::ParseDeb822File(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<RDeb822Source::Deb822Entry, std::allocator<RDeb822Source::Deb822Entry> >&)'
/usr/bin/ld: /home/mvogt/devel/synaptic/trunk.git/common/rsources.cc:600:(.text+0x39c3): undefined reference to `RDeb822Source::ConvertToSourceRecord(RDeb822Source::Deb822Entry const&, SourcesList::SourceRecord&)'
/usr/bin/ld: ../common/libsynaptic.a(rsources.o): in function `SourcesList::WriteDeb822Source(SourcesList::SourceRecord*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)':
/home/mvogt/devel/synaptic/trunk.git/common/rsources.cc:651:(.text+0x5c24): undefined reference to `RDeb822Source::ConvertFromSourceRecord(SourcesList::SourceRecord const&, RDeb822Source::Deb822Entry&)'
/usr/bin/ld: /home/mvogt/devel/synaptic/trunk.git/common/rsources.cc:656:(.text+0x5d5e): undefined reference to `RDeb822Source::WriteDeb822File(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<RDeb822Source::Deb822Entry, std::allocator<RDeb822Source::Deb822Entry> > const&)'
/usr/bin/ld: ../common/libsynaptic.a(rsources.o): in function `SourcesList::UpdateSources()':
/home/mvogt/devel/synaptic/trunk.git/common/rsources.cc:340:(.text+0x6a80): undefined reference to `RDeb822Source::ConvertFromSourceRecord(SourcesList::SourceRecord const&, RDeb822Source::Deb822Entry&)'
/usr/bin/ld: /home/mvogt/devel/synaptic/trunk.git/common/rsources.cc:345:(.text+0x6ad5): undefined reference to `RDeb822Source::WriteDeb822File(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<RDeb822Source::Deb822Entry, std::allocator<RDeb822Source::Deb822Entry> > const&)'this is a standard debian/unstable system I'm testing with. Any hints how to resolve this?
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.
The linker errors persist because we need to regenerate the build system after adding rsource_deb822.cc and rsource_deb822.h to Makefile.am. try running:
./autogen.sh
./configure
make clean
make
This will ensure the new files are properly integrated into the build system.
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.
Thanks, I tried this but no luck - when I pulled now I get a different error and it looks like common/rsource_deb822.cc is now urf-16 instead of the expected ascii or utf-8. this makes g++ unhappy:
$ file common/rsource_deb822.cc
common/rsource_deb822.cc: C source, Unicode text, UTF-16, little-endian text, with CRLF line terminators
...
$ file common/rswig.h
common/rswig.h: C++ source, ASCII textcan you please update again? (in GH the file is also shown as binary data now).
It currently fails to build in the new "handleFailedInstallation" - if I remove this new function it builds and runs but still no luck, still an empty repositories window for me. |
mvo5
left a comment
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.
Thanks a lot for working on this! Some quick feedback inline, still have not read the code line-by-line but went real quick over it)
common/rpackagemanager.h
Outdated
|
|
||
| }; | ||
|
|
||
| /** |
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.
Lets put this into either "rsources.{cc,h}" or a new "rsource_deb822.{cc,h}" - the rpackagemanager file is not quite the right place for this (and/or rsources_deb822 and rsources_manager).
common/rsource_deb822.cc
Outdated
| @@ -0,0 +1,252 @@ | |||
| /* rsource_deb822.cc - Deb822 format sources support | |||
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 new file should be added to POTFILES.in.
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 may be wrong, but shouldn't this new file be also added to common/Makefile.am?
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.
yes you are right
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.
Could you please add it? Thanks! :-)
|
Thank you, I still get compiler errors when I run this locally, if you could rebase the branch to current master that would be great, this way we will get working CI again (the github action was on ubuntu-20.04 and 15b6a17 which is EOL now moved it to ubuntu-latest) |
0e07c9b to
15b6a17
Compare
|
Why was this closed and all commits removed? |
…. Fix UTF-8 encoding issues.
… UTF-8 support using wide strings and codecvt_utf8 for file I/O operations.
While rebasing, The PR closed due to a force-push that temporarily removed my changes. |
I've rebased the branch to the current master, which includes the CI workflow update from ubuntu-20.04 to ubuntu-latest. This should resolve the CI issues. Regarding the compiler errors you're seeing locally, could you please share the specific error messages? |
|
@aybanda Not sure about Michael's errors, but this is how it looks like on my (Debian Testing) system: |
|
hey @mvo5 I tried few things, fixed few issues |
|
Sadly even more errors now on my system. |
|
Sorry for the late test. I have just tried the latest build, it sadly still has issues: When having the sources.list file in a legacy format (instead of deb822 files in sources.list.d), clicking the "Enabled" checkbox to enable a previously disabled repository will still convert the sources.list file to a deb822 format. This is incorrect - Synaptic should support both legacy and deb822 formats and apply them correctly based on source file types (and/or file location). Also, the converted file is invalid even as deb822: Notice that it is using |
|
Just to be clear, just about all the keys are plural. Examples: and a self-intstalled one: Mine shows |
|
Thanks a lot! It indeed works now without a conversion. I have found a few small issues though, but these may be unrelated to your changes: Enabling a previously disabled cdrom repository also do the following three changes.
Adding a new repo entry seems to work without a problem (except the issues mentioned above - they also apply here). |
|
Here is a diff as an example: |
|
I have also tested the modernized (deb822) source file and it sadly got broken. Synaptic returned a file format error after attempting to disable one of the entries. Here is a diff of the changes Synaptic has made: As you can see, the (By the way, also notice the empty line added at the end of the file - probably the same problem as with classic sources.list.) |
|
Thanks @AsciiWolf for the quick and detailed review! I’ll go through the points and fix them once I get a bit of time. Appreciate the thorough testing! |
|
Thanks again Ajay for working on this! I have sent you another 40 USD from the bounty via PayPal. :-) |
…ng, and Signed-By preservation
|
Thanks for the update! Quick test: Legacy
Deb822
|
…e, preserve comments and fields, and show deb-src entries
|
Hi @AsciiWolf |
|
Here are the source files I used for testing.
|
…o prevent corruption of .sources files
… (portable, non-GTK)
…rces, add debug output for config path
…c to trace stanza and field parsing
…ser and source management
…n; preserve Deb822 flag on edit
|
I have just repeated the test with latest code: Legacy: Deb822: Not sure why this happens, but it certainly does not look right and indeed apt does not like it: |
|
@aybanda Any update? :) |
|
I have found out that some of the small formatting issues that I pointed out before, for example the trailing space added at the end of the line, also happen in older Synaptic builds with legacy sources.list. We can probably ignore these small issues if they cause no real problems. But we must make sure that there is no invalid deb822 or legacy output generated by Synaptic. Adding, modifying, removing, enabling/disabling etc. the entries must work flawlessly. edit: This issue also happens with older Synaptic and looks harmless. |





This PR implements support for the Deb822 source format in Synaptic, as requested in issue #149.
What’s new
RDeb822SourceandRSourceManagerclasses for parsing, validating, and serializing Deb822 sources.Testing
tests/test_deb822_integration.cc) to verify parsing, file I/O, and validation.Sample Python test results
Created test directory: /tmp/tmpabcd1234
Initialized source manager
Source validation: PASS
Source string representation:
deb http://example.com stable main
Successfully wrote source to file: /tmp/tmpabcd1234/test.sources
Read source from file:
Type: deb
URI: http://example.com
Suite: stable
Components: main
Disabled source test: PASS
Multiple URIs test: PASS
Multiple suites test: PASS