packit icon indicating copy to clipboard operation
packit copied to clipboard

packit-cli source-git update-dist-git avoid unnecessary changes

Open pbrezina opened this issue 2 years ago • 3 comments

What happened? What is the problem?

This is the resulting diff (minus patch comments, I already removed them, see https://github.com/packit/packit/issues/2023)

 Name:           authselect
 Version:        1.2.6
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Configures authentication and identity sources from supported profiles
 URL:            https://github.com/authselect/authselect

 License:        GPLv3+
 Source0:        %{url}/archive/%{version}/%{name}-%{version}.tar.gz

-%global makedir %{_builddir}/%{name}-%{version}
+Patch0001:      0001-po-update-translations.patch
+Patch0002:      0002-profiles-do-not-try-to-change-password-via-sssd-for-.patch
+Patch0003:      0003-po-update-translations.patch

-Patch0001: 0001-po-update-translations.patch
+# # Downstream only
+Patch0901:      0901-rhel8-remove-mention-of-Fedora-Change-page-in-compat.patch
+Patch0902:      0902-rhel8-remove-ecryptfs-support.patch
+Patch0903:      0903-rhel8-Revert-profiles-add-support-for-resolved.patch
+Patch0904:      0904-rhel8-Revert-yescrypt.patch

-# Downstream only
-Patch0901: 0901-rhel8-remove-mention-of-Fedora-Change-page-in-compat.patch
-Patch0902: 0902-rhel8-remove-ecryptfs-support.patch
-Patch0903: 0903-rhel8-Revert-profiles-add-support-for-resolved.patch
-Patch0904: 0904-rhel8-Revert-yescrypt.patch
+%global makedir %{_builddir}/%{name}-%{version}

Notice that it screwed up "Downstream only" comment, rearranged and re-indented lines. I can accept lines rearrangement since patches right after sources is a better place, but still the change is not needed.

What did you expect to happen?

It should be:

@@ -3,7 +3,7 @@

 Name:           authselect
 Version:        1.2.6
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Configures authentication and identity sources from supported profiles
 URL:            https://github.com/authselect/authselect

@@ -13,6 +13,8 @@ Source0:        %{url}/archive/%{version}/%{name}-%{version}.tar.gz
 %global makedir %{_builddir}/%{name}-%{version}

 Patch0001: 0001-po-update-translations.patch
+Patch0002: 0002-profiles-do-not-try-to-change-password-via-sssd-for-.patch
+Patch0003: 0003-po-update-translations.patch

 # Downstream only
 Patch0901: 0901-rhel8-remove-mention-of-Fedora-Change-page-in-compat.patch
@@ -300,6 +302,11 @@ exit 0

This way, I got from 400+ changes (because translation patch commit message it very long) to 9. This should be the default behavior to keep the dist git readable and manageable.

Work definition

See https://github.com/packit/packit/issues/2024#issuecomment-1667665431 See https://github.com/packit/packit/issues/2024#issuecomment-1667672664

pbrezina avatar Aug 03 '23 09:08 pbrezina

Hi, thanks for opening this issue!

It's true the mechanism to inject patches inside the spec file can be naive.

I understand your proposal as:

  1. If the patches are already defined, find the last patch definition.
  2. Append new patches below the last definition
  3. If no patches are defined, do what we do right now

this makes sense to me and shouldn't be hard to implement

please help us adjust priority of this issue: is this blocking your source-git adoption?

TomasTomecek avatar Aug 07 '23 11:08 TomasTomecek

Hi, thanks for opening this issue!

It's true the mechanism to inject patches inside the spec file can be naive.

I understand your proposal as:

1. If the patches are already defined, find the last patch definition.

2. Append new patches below the last definition

3. If no patches are defined, do what we do right now

this makes sense to me and shouldn't be hard to implement

Yes and also keep the indentation.

please help us adjust priority of this issue: is this blocking your source-git adoption?

Yes, I want to move to source-git to safe time. The current behavior requires more time to fix it than to do it without packit.

pbrezina avatar Aug 07 '23 11:08 pbrezina

Thank you for opening these 3 issues. We've just discussed them and agree they are a great improvement in the source-git workflow. We are no longer actively prioritizing source-git work and since our PO is away this week, we'll go through them again next week.

TomasTomecek avatar Aug 10 '23 08:08 TomasTomecek

Hello all the source-git lovers,

as part of a backlog cleanup, we are closing source-git-related issues because it is not our priority anymore and we don't want to provide false promises to anyone. And if we were to change the priorities, the whole source-git project would need to be revisited nevertheless.

A more detailed description of why we have come up to this decision can be found at https://packit.dev/source-git/status (most of this is still relevant), TLDR is that we can provide more benefits in other areas.

What now:

  • The service part is currently being decommissioned for no real use. (It even was offline for a couple of weeks because of a certificate issue and no one realised.)
  • The CLI is still there, but the Packit team won't work on these tasks. We are happy to help and accept any external contribution.
  • If you find something really, really important and can't do it yourself, let us know. We might still find a way but we would need to be really sure that our time spent on this is worth the benefit.

Our current priorities are visible on our board (revisited quarterly) and are based on user benefit and demand. (=> You can influence that..;)

Thank you for your valuable feedback and for believing in source-git -- hopefully, someone will continue this way (happy to provide all the findings and code!). But we don't want to lie to you about the current state of source-git in the team.

František

lachmanfrantisek avatar Aug 15 '24 12:08 lachmanfrantisek