On 14/05/12 17:07, Joop Boonen wrote:

I wonder if this should be fixed, in either spec-cleaner (use%makeinstall in stead of %make_install) or in the buildserver (add macro %make_install for SLE)?

Spec cleaner was written to support only the latest distributions. -- Best Regards / S pozdravom, Pavol Rusnak PGP 0xB9A02A3D prusnak[at]opensuse.org -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-buildservice+owner@opensuse.org

-----Original Message----- From: Joop Boonen [mailto:joop.boonen@boonen.org] Sent: Monday, May 14, 2012 5:53 PM To: Pavol Rusnak Cc: opensuse-buildservice@opensuse.org; opensuse-packaging@opensuse.org Subject: [opensuse-buildservice] Re: [opensuse-packaging] To %make_install or to %makeinstall (spec-cleaner)

On Mon, May 14, 2012 5:28 pm, Pavol Rusnak wrote:

...

On 14/05/12 17:07, Joop Boonen wrote:

...

I wonder if this should be fixed, in either spec-cleaner (use%makeinstall in stead of %make_install) or in the buildserver (add macro %make_install for SLE)?

Spec cleaner was written to support only the latest distributions.

I like spec cleaner a lot and I think it really helps cleaning up a spec file.

Might it be option to translate into %makeinstall instead of %make_install?

Using %makeinstall in a spec file is a bad idea. While it makes it work on SLE, it breaks it on Fedora where %makeinstall is still the old broken variant. Defining %make_install on SLE might be the better option. I currently just replace %make_install directly with its first-level expansion, that seems to be only thing working across all OBS-supported Distros (but obviously might cause problems in the future). Gruesse, Frank -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-buildservice+owner@opensuse.org

On 14/05/12 18:22, Joop Boonen wrote:

Workaround:

%if 0%{!?make_install:1} %define make_install make install 'DESTDIR=%{buildroot}' %endif

If so then would it be an option to add this define to spec-cleaner?

Sorry. Spec-cleaner aims to remove cruft from spec files, not to add new one. -- Best Regards / S pozdravom, Pavol Rusnak PGP 0xB9A02A3D prusnak[at]opensuse.org -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-buildservice+owner@opensuse.org

On 2012-05-15 01:42:36 (+0200), Ruediger Meier wrote: [...]

...

...

...

Spec cleaner was written to support only the latest distributions.

The I can only recommend and ask that nobody uses spec cleaner. There are packagers who care about also building for older versions of openSUSE/Evergreen, for SLE, and even older RHEL. It's sad to break package builds just because of spec file syntax (if the deps are too old or not available, etc..., fine, but not because of a spec syntax gimmick, cmon.) [...]

...

...

Might it be option to translate into %makeinstall instead of %make_install?

...

Using %makeinstall in a spec file is a bad idea. While it makes it work on SLE, it breaks it on Fedora where %makeinstall is still the old broken variant. Defining %make_install on SLE might be the better option.

...

I currently just replace %make_install directly with its first-level expansion, that seems to be only thing working across all OBS-supported Distros (but obviously might cause problems in the future).

I'm using

%{?make_install} %{!?make_install:make install DESTDIR=%{buildroot}}

for all my packages. This works for all repos and spec-cleaner does not touch it.

... and entirely defeats the point of having a macro as a shortcut :) Why not just do %__make install DESTDIR='%{buildroot}' then? It's shorter, and works ;) I mean, the expansion shouldn't depend on the distro. It depends on the Makefile: either the Makefile has DESTDIR support, or it doesn't. We need one macro that _always_ does DESTDIR, on every distro, and possibly one macro that always does it "the old way" (though it's probably better to patch DESTDIR support into the Makefile instead). %define __makeinstall %__make install DESTDIR='%{buildroot}' cheers -- -o) Pascal Bleser /\\ http://opensuse.org -- we haz green _\_v http://fosdem.org -- we haz conf

On Tuesday 15 May 2012, Pascal Bleser wrote:

On 2012-05-15 01:42:36 (+0200), Ruediger Meier wrote: [...]

...

...

...

...

Spec cleaner was written to support only the latest distributions.

The I can only recommend and ask that nobody uses spec cleaner. There are packagers who care about also building for older versions of openSUSE/Evergreen, for SLE, and even older RHEL.

It's sad to break package builds just because of spec file syntax (if the deps are too old or not available, etc..., fine, but not because of a spec syntax gimmick, cmon.)

Yep, I have often seen repo maintainers running spec-cleaner or prepare_spec automatically on all packages of their repos. Mostly this just results in some debatable white space cleanups but also often it breaks things. For example I hate it seeing such stupid things directly committed into Factory (usually with non sense commit messages) and then having conflicts and broken things within the devel repos ... Even worse is that osc client seems to use some kind pre-commit-cleanup-hook by default. It changes your _uncommited_ edits after you have reviewed the diff! So you commit something which you have never seen nor tested before. And your original edits are lost. This kind of auto style cleanup should be done by the editor but never by a VCS client.

...

[...]

I'm using

%{?make_install} %{!?make_install:make install DESTDIR=%{buildroot}}

for all my packages. This works for all repos and spec-cleaner does not touch it.

... and entirely defeats the point of having a macro as a shortcut :)

Hm, I just use the shortcut only if exists. What wrong with this?

Why not just do %__make install DESTDIR='%{buildroot}' then? It's shorter, and works ;)

Maybe you are right but %make_install might contain even more distro specific things - who knows. That's why we also use %configure instead of ./configure. And if I would see any distro where %configure is not defined then I would use a similar ifdef solution rather than always using the fallback.

I mean, the expansion shouldn't depend on the distro. It depends on the Makefile: either the Makefile has DESTDIR support, or it doesn't.

We need one macro that _always_ does DESTDIR, on every distro, and possibly one macro that always does it "the old way" (though it's probably better to patch DESTDIR support into the Makefile instead).

%define __makeinstall %__make install DESTDIR='%{buildroot}'

cheers -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-buildservice+owner@opensuse.org

On 15 May 2012 12:50, Joop Boonen wrote:

On Tue, May 15, 2012 7:02 am, Pascal Bleser wrote:

...

On 2012-05-15 01:42:36 (+0200), Ruediger Meier wrote: [...]

...

...

...

...

Spec cleaner was written to support only the latest distributions.

The I can only recommend and ask that nobody uses spec cleaner. There are packagers who care about also building for older versions of openSUSE/Evergreen, for SLE, and even older RHEL.

It's sad to break package builds just because of spec file syntax (if the deps are too old or not available, etc..., fine, but not because of a spec syntax gimmick, cmon.)

I agree, so why not a spec cleaner that does all the nice things of the current spec cleaner. Puts the BuildRequires and Requires each in one line and in an alphabetical order. The replaces /usr/lib(64) by %{_libdir} etc

And it replaces %makeinstall %make_install etc by a universal %__make install DESTDIR='%{buildroot}' ?

Because we may want to redefine %make_install in the future. All supported version of openSUSE (and the latest two unsupported ones...) support %make_install. And if needed we can make the OBS builds support %make_install in the distributions that originally didn't support it. There is really no point in touching spec-cleaner. -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-buildservice+owner@opensuse.org