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

Hotfix for issue #1401 (missing dependencies in Ubuntu PPA package) #1402

Merged
merged 4 commits into from
Jan 31, 2023
Merged

Hotfix for issue #1401 (missing dependencies in Ubuntu PPA package) #1402

merged 4 commits into from
Jan 31, 2023

Conversation

aryoda
Copy link
Contributor

@aryoda aryoda commented Jan 30, 2023

Proposed hotfix for #1401.

Needs to be merged into

  • main
  • dev (must be done in the cmd line I guess since the Github GUI does support only one target branch for a PR):
    git switch dev + git merge main

@aryoda
Copy link
Contributor Author

aryoda commented Jan 30, 2023

@Germar If you update our ppa:testing after this hotfix made its way into the main branch I can test the package installation of my Ubuntu systems ...

@buhtz
Copy link
Member

buhtz commented Jan 30, 2023

Please don't merge this yet because this needs more discussion, also about some basic ("grundsätzliche") topics. :)

Conceptual the PPA is not "upstream". It is a distro-specific and community-maintained source. That one of the upstream maintainers (Germar) do manage that PPA is a coincidence but doesn't mean that it is the same.

IMHO the GitHub repo and the PPA need to be treated different and don't follow the same rules.

On upstream I vote to merge the dependency thing only to dev. There is no need for a hotfix on upstream for this.

On PPA of course there is also no need for a "hotfix" but for a fix of the package. The version keeps 1.3.3 but the package version need to increment. "1.3.3-2" would be the name. The files in /debian don't belong to upstream because they are distro specific (Ubuntu-PPA only. Debian does ignore our /debian.). Changing them won't change upstream code.
That this folder is there just has historical and organizational reasons. For example all the PPA related stuff could be in its own repo on Launchpad. But this can be discussed later on bit-dev.

About dependencies themself. I assume that the kdewallet and gnome wallet recommendations won't make it into an official Debian package. "recommends" are selected and installed by default. In that case the user would trigger the installation of a lot of KDE and Gnome packages depending on those booth. I'm also not sure about the consequences of having this in the PPA package.

After some quick n dirty research I'm also not sure that this two packages even exist in Debian and maybe Ubuntu. Where do you get this names from?

I suspect there had been made some tests with it but I will sleep better if I will test 1.3.3 (not the PPA) with the latest/unstable Debian and Ubuntu again.

@aryoda
Copy link
Contributor Author

aryoda commented Jan 30, 2023

Please don't merge this yet because this needs more discussion

Yes, I see your points, thanks for raising the red flag :-)

Conceptual the PPA is not "upstream". It is a distro-specific and community-maintained source.
[...]
The files in /debian don't belong to upstream because they are distro specific (Ubuntu-PPA only. Debian does ignore our /debian.). Changing them won't change upstream code.
That this folder is there just has historical and organizational reasons. For example all the PPA related stuff could be in its own repo on Launchpad.

Correct, this must be clearly separated from our BiT source code in the future.

On PPA of course there is also no need for a "hotfix" but for a fix of the package.

Please ignore my "dramatic" wording "hotfix" here, I just wanted to make sure

  • we fix this soon
  • the change must be merged into dev and main (that is how a hotfix works)

I personally consider this as an "hotfix" for the PPA package, not BiT itself (as you also do).

On upstream I vote to merge the dependency thing only to dev. There is no need for a hotfix on upstream for this.

I am not quite sure about our workflow to create a new package on our Ubuntu PPA,
but do we really want to update our PPA:stable package from a dev branch (not from main)`?

About dependencies themself. I assume that the kdewallet and gnome wallet recommendations won't make it into an official Debian package. "recommends" are selected and installed by default. In that case the user would trigger the installation of a lot of KDE and Gnome packages depending on those booth.

After reconsidering this I think I should indeed remove these both recommended packages again because the installation may probably cause that keyring is choosing another default keyring so that credentials that are already stored in one keyring implementation may not be found anymore in the new default keyring (see #990 (comment)).

Our BiT CLI should indeed not depend on any UI stuff (KDE or Gnome).

After some quick n dirty research I'm also not sure that this two packages even exist in Debian and maybe Ubuntu. Where do you get this names from?

These are Ubuntu packages but Ubuntu uses DEB files requiring the dependencies in the control file.
So I think our folder name Debian is somehow misleading if the content is only used to update our Ubuntu PPA...

So: On what next steps can we agree?

  • I need confirmation that the changed control file is indeed only used to feed our Ubuntu PPA
  • I would like to remove at least the "Recommend" changes from this PR
  • Edit: DONE: Rename this PR from "Debian" to "Ubuntu PPA"
  • ... (any further proposals?)...

@aryoda aryoda changed the title Hotfix for issue #1401 (missing Debian dependencies) Hotfix for issue #1401 (missing dependencies in Ubuntu PPA package) Jan 30, 2023
@buhtz
Copy link
Member

buhtz commented Jan 30, 2023

Moin again,

I understand the term "hotfix" in the context of our branching model. In that case a hotfix implicates a new version number. So if you would "hotfix" it means you will create a new release "1.3.4". That is why it is hot 🌶️ . 😄

I am not quite sure about our workflow to create a new package on our Ubuntu PPA,
but do we really want to update our PPA:stable package from a dev branch (not from main)`?

This is up to Germar to decide. I assume it isn't a big problem to create a deb file based on the dev branch.
On the other hand I see no big problem to integrate that modification also into main. This would IMHO be an acceptable exception to our branching model (where usually main commits are releases only). But just with a simple commit, not with a release tag.

These are Ubuntu packages but Ubuntu uses DEB files requiring the dependencies in the control file.
So I think our folder name Debian is somehow misleading if the content is only used to update our Ubuntu PPA...

deb-files (for Ubuntu) are created with Debian tools. They expect a /debian folder. I'm not sure of course but assume that all debian-based Distributions not named "Debian" do use such folders.

I need confirmation that the changed control file is indeed only used to feed our Ubuntu PPA

I can't confirm exactly that but confirm that Debian defenitly doesn't use that folder.
See the debian branch of the backintime package on the "Salsa" (Debians own GitLab) instance. In the same repo there is the upstream source (a 1:1 copy of our GitHub repo) in the upstream branch. The debian folder in the latter is our upstream-debian-folder that shouldn't belong there as we discussed. Comparing our upstream control file and Debian's own control file you see there is much difference.

Beside this Salsa instance you can find on the Debians package tracker a link called browse source code where you can look into the source of the selected package.

@buhtz
Copy link
Member

buhtz commented Jan 30, 2023

  • I would like to remove at least the "Recommend" changes from this PR

You could move the two packages from "Recommend" into "Suggested". See https://www.debian.org/doc/debian-policy/ch-relationships.html#syntax-of-relationship-fields

@Germar
Copy link
Member

Germar commented Jan 30, 2023

I am not quite sure about our workflow to create a new package on our Ubuntu PPA,
but do we really want to update our PPA:stable package from a dev branch (not from main)`?

This is up to Germar to decide. I assume it isn't a big problem to create a deb file based on the dev branch. On the other hand I see no big problem to integrate that modification also into main. This would IMHO be an acceptable exception to our branching model (where usually main commits are releases only). But just with a simple commit, not with a release tag.

I create the PPA from a local checkout. So for the PPA it doesn't matter if you update main or just dev. But we provide the possibility to build .deb files from source with makebeb.sh. And because of this I'd recommend to update both.

These are Ubuntu packages but Ubuntu uses DEB files requiring the dependencies in the control file.
So I think our folder name Debian is somehow misleading if the content is only used to update our Ubuntu PPA...

deb-files (for Ubuntu) are created with Debian tools. They expect a /debian folder. I'm not sure of course but assume that all debian-based Distributions not named "Debian" do use such folders.

/debian is crucial for the .deb build. Please do not rename.

  • I would like to remove at least the "Recommend" changes from this PR

You could move the two packages from "Recommend" into "Suggested".

Please don't set those dependencies. Neither as recommend nor suggested. keyring-secretstorrage is the minimum that is required. gnome-keyring or kde-keyring will be installed by the desktop environment.

@aryoda
Copy link
Contributor Author

aryoda commented Jan 30, 2023

OK, I have removed the keyring dependencies again, please review.

I create the PPA from a local checkout. So for the PPA it doesn't matter if you update main or just dev. But we provide the possibility to build .deb files from source with makedeb.sh.

THX, no I understand the build workflow. It uses dpkg-buildpackage internally which chroots into the debian folder (which is why the folder must be named as it is).

I have put this topic on my TODO list (to document how to build the DEB pkg for Ubuntu).

Last missing part (for me) is how to upload this to launchpad.

@Germar
Copy link
Member

Germar commented Jan 31, 2023

I just pushed the new version to the testing PPA.

Last missing part (for me) is how to upload this to launchpad.

For this I set the new version in VERSION and CHANGES and run updateversion.sh. This will change the version in every other file and create a new debian/changelog with my name at the end. This is crucial for the build process later on.

Next I git commit those changes and create a new tar archive with make-tarball.sh which will actually git clone current branch into a new folder ../backintime-$VERSION and than make an archive from that new folder. This way we can be sure there are no left-over files inside the release nor in the PPA.

Inside this new folder I run makedeb-src.sh which will create all the necessary files for the build service and sign it with my personal GPG key. That's why it is important that debian/changelog end with my name. To publish those files I run dput ppa:bit-team/testing *.changes. Without my signature the Launchpad servers will reject those files.

@aryoda
Copy link
Contributor Author

aryoda commented Jan 31, 2023

I just pushed the new version to the testing PPA.

Did you manually merge my PR into your checked-out source code before building and pushing to PPA?

I think the control file of the DEB pkg does still not contain the missing python3-packaging dependency (I did a manual download and opened it via my Archive Manager to decompress it...)

PS: THX for describing the PPA publishing workflow, I have added this missing part to my TODO list for dev-doc too.

@aryoda aryoda merged commit e22c7f2 into bit-team:main Jan 31, 2023
aryoda added a commit that referenced this pull request Jan 31, 2023
…1402) (#1403)

* Hotfix for issue #1401 (missing Debian dependencies)

[skip travis]
@aryoda
Copy link
Contributor Author

aryoda commented Jan 31, 2023

@Germar OK, I have merged this PR with the missing dependency into main and dev now.

Did you manually merge my PR into your checked-out source code before building and pushing to PPA?

I think the control file of the DEB pkg does still not contain the missing python3-packaging dependency (I did a manual download and opened it via my Archive Manager to decompress it...)

Could someone please check this?

@Germar
Copy link
Member

Germar commented Jan 31, 2023

Did you manually merge my PR into your checked-out source code before building and pushing to PPA?

I think the control file of the DEB pkg does still not contain the missing python3-packaging dependency

No, I changed it manually in my local clone. But before launching make-tarball.sh I remembered, that I've to commit changes before. I didn't want to commit it to main, so I reverted all changes, created a new branche and did all the changes again. Looks like I forgot the most important change the 2nd time 🤦‍♂️, sorry.
I'll make a new release tomorrow.

@Germar
Copy link
Member

Germar commented Feb 1, 2023

I pushed it again to both testing and stable PPA

@aryoda
Copy link
Contributor Author

aryoda commented Feb 1, 2023

I pushed it again to both testing and stable PPA

THX a lot, it looks good now (includes the missing dependency):

https://launchpadlibrarian.net/649192535/backintime_1.3.3~kinetic_1.3.3-3~kinetic.diff.gz

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.

3 participants