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

Uptodate Source pointing to GitHub in RPM spec #2949

Merged
merged 5 commits into from Mar 3, 2023

Conversation

pcahyna
Copy link
Member

@pcahyna pcahyna commented Feb 28, 2023

Pull Request Details:
  • Type: Bug Fix

  • Impact: Low

  • Reference to related issue (URL): closes Fix ReaR source download links in packaging / drop SourceForge? #2945

  • How was this pull request tested?
    spectool -g packaging/rpm/rear.spec downloads the correct tarball

  • Brief description of the changes in this pull request:
    Change the URL in the Source tag in the RPM spec file to point to the tarball on GitHub instead of SourceForge (where the tarball is not updated anymore).
    AFAIK GitHub creates the tarballs automatically from tags.

@pcahyna
Copy link
Member Author

pcahyna commented Feb 28, 2023

ToDo: the Makefile also needs updating, otherwise it will rewrite the spec when executingmake dist.

Copy link
Member

@schlomo schlomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can simply remove the corresponding sed line from the Makefile and I would appreciate for you to do that as well within this PR.

About Gentoo: According to https://devmanual.gentoo.org/ebuild-writing/variables/index.html#renaming-sources I'd expect the URL line to look like this:

SRC_URI="https://github.com/rear/rear/archive/${PV}.tar.gz -> ${P}.tar.gz"

But indeed, a test by somebody using Gentoo would be nice, although I'd rather make this change blindly as not to change it - the old URL is 100% broken, the new one might just work.

DEB and Arch don't carry the dist archive URL in the description so that we don't need to change it there.

packaging/rpm/rear.spec Show resolved Hide resolved
Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I blindly approve it because I trust you and
I know almost nothing about ReaR's make and build code.

For SUSE and openSUSE I use my own RPM spec files
to be in compliance with what SUSE requires
(certain SUSE specific rules).

@pcahyna
Copy link
Member Author

pcahyna commented Mar 1, 2023

I think you can simply remove the corresponding sed line from the Makefile and I would appreciate for you to do that as well within this PR.

About Gentoo: According to https://devmanual.gentoo.org/ebuild-writing/variables/index.html#renaming-sources I'd expect the URL line to look like this:

SRC_URI="https://github.com/rear/rear/archive/${PV}.tar.gz -> ${P}.tar.gz"

But indeed, a test by somebody using Gentoo would be nice, although I'd rather make this change blindly as not to change it - the old URL is 100% broken, the new one might just work.

ok, will do

@jsmeix jsmeix added bug The code does not do what it is meant to do cleanup labels Mar 1, 2023
@pcahyna pcahyna changed the title Uptodate Source pointing to GitHub in RPM spec Draft: Uptodate Source pointing to GitHub in RPM spec Mar 2, 2023
@schlomo
Copy link
Member

schlomo commented Mar 2, 2023

Yes, agree with you that a more explicitly named tag would be useful.

@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

I think you can simply remove the corresponding sed line from the Makefile and I would appreciate for you to do that as well within this PR.

This won't work properly, because make dist constructs a tarball named e.g. rear-2.7-git.4965.531d9074.githubsourceinspecfile.changed.tar.gz under dist, so the Source: line needs to be updated to point to this tarball, otherwise rpmbuild invoked from make srpm would not find it (just tried that). But I can update the sed line to use a working URL (i.e. GitHub) for Source.

@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

ToDo: the Makefile also needs updating, otherwise it will rewrite the spec when executingmake dist.

it won't, because Makefile does not touch packaging/rpm/rear.spec, it constructs a temporary spec under dist (the message Rewriting packaging/rpm/rear.spec, packaging/debian/rear.dsc and usr/sbin/rear is misleading).

@@ -18,8 +18,7 @@ License: GPL-3.0
Group: Applications/File
URL: http://relax-and-recover.org/

# as GitHub stopped with download section we need to go back to Sourceforge for downloads
Source: https://sourceforge.net/projects/rear/files/rear/%{version}/rear-%{version}.tar.gz
Source: https://github.com/rear/rear/archive/%{version}.tar.gz#/rear-%{version}.tar.gz

# BuildRoot: is required for SLES 11 and RHEL/CentOS 5 builds on openSUSE Build Service (#2135)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsmeix please remind me - is SLES 11 going to be deprecated in the next release? If so, we could remove this line.

Copy link
Member

@jsmeix jsmeix Mar 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SLES11 is no longer officially supported by ReaR upstream
since a long time and since ReaR 2.7 is released even I
do no longer care about SLE11 at ReaR upstream
so feel free to remove such old and obsolete stuff.

Explanation:
During ReaR 2.7 development I only liked to
not knowingly break ReaR 2.7 for SLE11 because
I might need ReaR 2.7 for SLE11 for special SUSE customers
on very specific request and therefore I liked to avoid
that I would have to fix various things at various places
to make ReaR 2.7 usable for a specific use case on SLE11.

Furthermore:
SLES11 Long Term Service Pack Support (LTSS)
had ended on 31 March 2022 so in general
SLES11 is no longer supported by SUSE, see
https://en.wikipedia.org/wiki/SUSE_Linux_Enterprise#End-of-support_schedule

@schlomo
Copy link
Member

schlomo commented Mar 2, 2023

Good catch. I was also fighting our Makefile yesterday and would love to find a way how ReaR can run from source without modifying the source area at all.

For the Source: entry in the spec file, is any valid URL OK or must it actually match the content and the dist archive file name? I find it very important that the dist archive file name can continue to carry the full git branch and version info.

@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

For the Source: entry in the spec file, is any valid URL OK or must it actually match the content and the dist archive file name? I find it very important that the dist archive file name can continue to carry the full git branch and version info.

It does not need to be a URL at all. I would actually prefer the Makefile to keep only the file name in Source. URLs of type https://sourceforge.net/projects/rear/files/rear/2.7/rear-2.7-git.4965.531d9074.githubsourceinspecfile.changed.tar.gz, which the current Makefile produces, have never been correct even when SourceForge download was working and won't became more correct by changing them to github.com. I would prefer just Source: rear-2.7-git.4965.531d9074.githubsourceinspecfile.changed.tar.gz which expresses clearly that the tarball has been built locally and can't be found anywhere on the Internet.

@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

Good catch. I was also fighting our Makefile yesterday and would love to find a way how ReaR can run from source without modifying the source area at all.

have a look at the dist target - it produces modified sources under build without touching the original sources (except man).

@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

by the way, I believe ReaR can already run from source checkout without modifications - https://github.com/rear/rear#quick-start-guide

@schlomo
Copy link
Member

schlomo commented Mar 2, 2023 via email

@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

For the Source: entry in the spec file, is any valid URL OK or must it actually match the content and the dist archive file name?

if by dist archive file name you mean the archive file name that rpmbuild will actually use to build the source RPM, the last component of the Source URL must match it. This is because the last component of the URL is THE way to determine the source tarball name that rpmbuild uses. There is no independent way to provide it. OTOH, the URL does not need to be valid and indeed, as I wrote above, it does not need to be an URL at all, a local file name is enough.

@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

The problem is that ReaR running from source add os.conf and the log files into the source tree. And uses it to build the rescue image.

So the problem is that the rescue image will contain os.conf and log files? But IIRC this will be the case also for installed ReaR.

Remove the sourceforge.net URL from the Makefile recipe that generates
the Source tag in the RPM spec in the `dist` target. We don't use
sourceforge.net anymore and the Source points to a locally-generated
tarball, so the URL has not been valid anyway.

If $(distversion) == $(version) (when OFFICIAL is set), we could keep
the Source tag unchanged - this is true of most of the sed
transformations in this recipe.
The message looked like the recipe rewrites files in the source tree.
In fact, it only rewrites their copies in the build directory and leaves
the originals untouched.

XXX the message is now a bit long, it exceeds 80 characters.
Replace SourceForge by GitHub
@pcahyna pcahyna changed the title Draft: Uptodate Source pointing to GitHub in RPM spec Uptodate Source pointing to GitHub in RPM spec Mar 2, 2023
@pcahyna
Copy link
Member Author

pcahyna commented Mar 2, 2023

@schlomo I think I addressed all the ToDos now.

Copy link
Member

@schlomo schlomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, not sure about the context where the full URL in the spec file will be used though.

For sure I see no harm so if it helps anybody then this is great.

Makefile Show resolved Hide resolved
@@ -18,8 +18,7 @@ License: GPL-3.0
Group: Applications/File
URL: http://relax-and-recover.org/

# as GitHub stopped with download section we need to go back to Sourceforge for downloads
Source: https://sourceforge.net/projects/rear/files/rear/%{version}/rear-%{version}.tar.gz
Source: https://github.com/rear/rear/archive/%{version}.tar.gz#/rear-%{version}.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which circumstance or context does this line actually take effect if make dist always changes that to just the dist archive file name?

Copy link
Member Author

@pcahyna pcahyna Mar 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only when you want to build the RPM manually and need to know where to obtain the sources from. In the future, we might also avoid spec file rewriting in make dist if OFFICIAL is set and thus the RPM is going to be built from unmodified sources.

@pcahyna
Copy link
Member Author

pcahyna commented Mar 3, 2023

Looks good, not sure about the context where the full URL in the spec file will be used though.

For sure I see no harm so if it helps anybody then this is great.

The usual RPM build automation (rpmbuild and koji) do not use the full URL, only the last component (at least in Fedora, maybe the tooling for other distros is actually using it). The full URL is there mostly to document where the source can be obtained. Note that https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_troublesome_urls mentions cases where a URL can't be used, so only the file name is used in Source and the full URL is in a comment.

There is a tool though called spectool that automates the download of the source file : so instead of downloading the tarball manually, you can execute spectool -g packaging/rpm/rear.spec. For this, it is handy to have a correct URL. There may be other tooling that needs valid URL. (Packit builds executed from PRs do not need it though, because Packit rebuilds the tarball including the changes and changes Source to point to the tarball that it has built, in analogy with make dist.)

All in all, this PR is less important than it may seem. The incorrect URL was not causing any failure.

@schlomo
Copy link
Member

schlomo commented Mar 3, 2023

I'm also not super happy with out Makefile, but this PR is a good improvement and we learned something. Thanks a lot!

@schlomo schlomo merged commit e9a1eeb into rear:master Mar 3, 2023
@jsmeix
Copy link
Member

jsmeix commented Mar 3, 2023

In the openSUSE Build Service projects
Archiving:Backup:Rear and Archiving:Backup:Rear:Snapshot
I removed the build repository "SLE_11_SP4"
so ReaR gets no longer built there for SLE11, cf.
#2949 (comment)
and old stuff for SLE11 in packaging/rpm/rear.spec
can be safely removed.

@jsmeix jsmeix added this to the ReaR v2.8 milestone Mar 3, 2023
@pcahyna
Copy link
Member Author

pcahyna commented Mar 8, 2023

@pcahyna I did the ReaR 2.7 release (my first release ever) according to https://github.com/rear/rear/wiki/Release-process as far as I could understand the instructions there and partially in some kind of "bona fide script kiddie" mode ;-) so feel free to enhance https://github.com/rear/rear/wiki/Release-process as needed.

@jsmeix thank you for showing us how this Makefile target is actually being used in the release process. If we decide to change the functionality of the target, we should definitely consider how it fits into the release process.

@pcahyna
Copy link
Member Author

pcahyna commented Mar 8, 2023

@schlomo

Yes, agree with you that a more explicitly named tag would be useful.

(this was in reaction to my

there is both a branch named rear-2.7 and a tag named rear-2.7, and similarly for the older releases (2.6, 2.5). So I have to go back to the original idea of using bare %{version}, in the archive name and keep the trick, like 2.7.tar.gz.

In the future it would probably be a good idea to use a release branch name that does not shadow the release tag name, like e.g. release-2.8.

)

I think that the tag name is actually OK and one should use a different release branch name to avoid ambiguity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The code does not do what it is meant to do cleanup fixed / solved / done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix ReaR source download links in packaging / drop SourceForge?
3 participants