obs-service-format_spec_file is moving comments around

classic Classic list List threaded Threaded
21 messages Options
12
Reply | Threaded
Open this post in threaded view
|

obs-service-format_spec_file is moving comments around

Michal Srb
Hi,

I have problem with format_spec_file, it moves comments to different lines for no obvious reason.

For example in SUSE:SLE-12-SP1:Update/tigervnc:

Snipped from original spec file:
  Url:            http://tigervnc.org/
  BuildRoot:      %{_tmppath}/%{name}-%{version}-build
  Summary:        A high-performance, platform-neutral implementation of VNC
  License:        GPL-2.0 and MIT
  Group:          System/X11/Servers/XF86_4
  # would need fixing to use fPIC in common/CMakeLists.txt
  ExcludeArch:    s390
  Source1:        <a href="https://github.com/TigerVNC/tigervnc/archive/v%version">https://github.com/TigerVNC/tigervnc/archive/v%version}.tar.gz
  Source3:        vnc.xinetd

After format_spec_file:
  Url:            http://tigervnc.org/
  BuildRoot:      %{_tmppath}/%{name}-%{version}-build
  Summary:        A high-performance, platform-neutral implementation of VNC
  # would need fixing to use fPIC in common/CMakeLists.txt
  License:        GPL-2.0 and MIT
  Group:          System/X11/Servers/XF86_4
  ExcludeArch:    s390
  Source1:        <a href="https://github.com/TigerVNC/tigervnc/archive/v%">https://github.com/TigerVNC/tigervnc/archive/v%{version}.tar.gz
  Source3:        vnc.xinetd

The comment moved to a different line where it doesn't make sense.
Is it a bug in obs-service-format_spec_file?

Michal Srb
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Luigi Baldoni
Michal Srb wrote
I have problem with format_spec_file, it moves comments to different lines for no obvious reason.
I don't think it's smart enough to keep comments where they belong.

Someone told me to do this to leave them in place:

# SECTION mycomment
foo
# /SECTION

Regards
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Rüdiger Meier
In reply to this post by Michal Srb


On 07/18/2017 11:14 AM, Michal Srb wrote:

> Hi,
>
> I have problem with format_spec_file, it moves comments to different lines for no obvious reason.
>
> For example in SUSE:SLE-12-SP1:Update/tigervnc:
>
> Snipped from original spec file:
>    Url:            http://tigervnc.org/
>    BuildRoot:      %{_tmppath}/%{name}-%{version}-build
>    Summary:        A high-performance, platform-neutral implementation of VNC
>    License:        GPL-2.0 and MIT
>    Group:          System/X11/Servers/XF86_4
>    # would need fixing to use fPIC in common/CMakeLists.txt
>    ExcludeArch:    s390
>    Source1:        <a href="https://github.com/TigerVNC/tigervnc/archive/v%version">https://github.com/TigerVNC/tigervnc/archive/v%version}.tar.gz
>    Source3:        vnc.xinetd
>
> After format_spec_file:
>    Url:            http://tigervnc.org/
>    BuildRoot:      %{_tmppath}/%{name}-%{version}-build
>    Summary:        A high-performance, platform-neutral implementation of VNC
>    # would need fixing to use fPIC in common/CMakeLists.txt
>    License:        GPL-2.0 and MIT
>    Group:          System/X11/Servers/XF86_4
>    ExcludeArch:    s390
>    Source1:        <a href="https://github.com/TigerVNC/tigervnc/archive/v%">https://github.com/TigerVNC/tigervnc/archive/v%{version}.tar.gz
>    Source3:        vnc.xinetd
>
> The comment moved to a different line where it doesn't make sense.
> Is it a bug in obs-service-format_spec_file?

This detail is probably a bug but actually it's broken by design. There should
not be any "plugin" for versioning-control-systems which edits your files silently
just *before* you save (commit) it. It's the opposite of the one major thing what
any VCS should do: DO NOT LOSE/RANDOMIZE ANY WORK DONE BY THE USER.

The best way is to commit always using --noservice. You can also disable the
services completely on your systems somehow, don't remember right now how I did it.

cu,
Rudi
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Peter Simons
Rüdiger Meier writes:

 > The best way is to commit always using --noservice.

That is my experience as well. The obs-service-format_spec_file plugin has
messed up my spec files on several occasions, and none of my bug reports
on Github or elsewhere has had any observable effect. It seems like that
code is essentially unmaintained, which is a rather disconcerting notion
considering at what a central position in the development process that
plugin lives.

Best regards,
Peter
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Richard Biener
On Tue, 18 Jul 2017, Peter Simons wrote:

> Rüdiger Meier writes:
>
>  > The best way is to commit always using --noservice.
>
> That is my experience as well. The obs-service-format_spec_file plugin has
> messed up my spec files on several occasions, and none of my bug reports
> on Github or elsewhere has had any observable effect. It seems like that
> code is essentially unmaintained, which is a rather disconcerting notion
> considering at what a central position in the development process that
> plugin lives.
Just kill it then.

Richard.

> Best regards,
> Peter
> --
> To unsubscribe, e-mail: [hidden email]
> To contact the owner, e-mail: [hidden email]
>

--
Richard Biener <[hidden email]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Johannes Meixner
In reply to this post by Rüdiger Meier

Hello,

On Jul 18 19:41 Rüdiger Meier wrote (excerpt):
> ... actually it's broken by design.
> There  should not be any "plugin" ...
> which edits your files silently

Persoanlly I cannot agree more - but that won't help
because apparently it is "right by intention" to silently
edit package source files (in particular the spec file)
because since the beginning of time S.u.S.E had
silently working scripts that "improve" spec files.


> The best way is to commit always using --noservice.

That does not help for submitrequests from other
openSUSE contributors who didn't know that and
who therefore submit their intended (good) changes
plus the silently autogenerated mess.

Because it works this way since the beginning of time
I meanwhile simply ignore that automated mess-up.
For some time I tried to correct the autogenerated mess
but in the end it was only a waste of my time because
one gets same mess again and again with each submitrequest
until one gives up.

As long as it build succeeds I do no longer care.


Kind Regards
Johannes Meixner
--
SUSE LINUX GmbH - GF: Felix Imendoerffer, Jane Smithard,
Graham Norton - HRB 21284 (AG Nuernberg)
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Tomas Chvatal
In reply to this post by Richard Biener
Richard Biener píše v St 19. 07. 2017 v 09:44 +0200:

> On Tue, 18 Jul 2017, Peter Simons wrote:
>
> > Rüdiger Meier writes:
> >
> >  > The best way is to commit always using --noservice.
> >
> > That is my experience as well. The obs-service-format_spec_file
> > plugin has
> > messed up my spec files on several occasions, and none of my bug
> > reports
> > on Github or elsewhere has had any observable effect. It seems like
> > that
> > code is essentially unmaintained, which is a rather disconcerting
> > notion
> > considering at what a central position in the development process
> > that
> > plugin lives.
>
> Just kill it then.
>
Well we have replacement in spec-cleaner.

Even if you disagree with some changes it does it is unit-tested to
always do the same thing, not suddenly move comments/etc.

Problem is just one tiny issue [1] where I have no clue how to make
this true for all the platforms we have. And it seems none knows how to
tweak it for already existing products (yes it is gone for the 42.3,
but what about 42.2 and all the SLE variants we have/etc...).

So anyone willing to figure it out?

Cheers

Tom

[1] https://github.com/openSUSE/spec-cleaner/issues/172

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Jan Engelhardt-4

On Wednesday 2017-07-19 11:24, Tomas Chvatal wrote:

>>>
>>>> The best way is to commit always using --noservice.
>>>
>>> That is my experience as well. The obs-service-format_spec_file
>>> plugin has messed up my spec files on several occasions, and none
>>> of my bug reports on Github or elsewhere has had any observable
>>> effect.
>>
>> Just kill it then.
>>
>Well we have replacement in spec-cleaner.

That does not make it better - after all, it's a "pick your poison"
kind of choice. s-c may not move comments, but in turn
it transmogrifies other things.
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Simon Lees-3


On 19/07/17 20:27, Jan Engelhardt wrote:

>
> On Wednesday 2017-07-19 11:24, Tomas Chvatal wrote:
>>>>
>>>>> The best way is to commit always using --noservice.
>>>>
>>>> That is my experience as well. The obs-service-format_spec_file
>>>> plugin has messed up my spec files on several occasions, and none
>>>> of my bug reports on Github or elsewhere has had any observable
>>>> effect.
>>>
>>> Just kill it then.
>>>
>> Well we have replacement in spec-cleaner.
>
> That does not make it better - after all, it's a "pick your poison"
> kind of choice. s-c may not move comments, but in turn
> it transmogrifies other things.
>
But at least when it does you can open a bugreport with a description of
what has happened why it shouldn't have happened and then in most cases
someone will reply to the bug report and get the issue fixed at some point.

--

Simon Lees (Simotek)                            http://simotek.net

Emergency Update Team                           keybase.io/simotek
SUSE Linux                           Adelaide Australia, UTC+10:30
GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B


signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Michal Kubecek
On Wednesday, 19 July 2017 13:32 Simon Lees wrote:

> On 19/07/17 20:27, Jan Engelhardt wrote:
> > On Wednesday 2017-07-19 11:24, Tomas Chvatal wrote:
> >> Well we have replacement in spec-cleaner.
> >
> > That does not make it better - after all, it's a "pick your poison"
> > kind of choice. s-c may not move comments, but in turn
> > it transmogrifies other things.
>
> But at least when it does you can open a bugreport with a description
> of what has happened why it shouldn't have happened and then in most
> cases someone will reply to the bug report and get the issue fixed at
> some point.

That doesn't help with the basic issue of the tool: it (or rather its
author) assumes the is One and Only Right Way to write specfiles. Just
for fun, I ran it on one of my specfiles (with -m) and diff-ed the
original specfile and the result.

  1. Sometimes I want to add an empty line between sections to visually
     separate them. The tool eats them.
  2. For specfiles with more patches (say, from ten up), I find it much
     easier to read and work with if the "Patch*" lines are separated
     from the rest of the header. The tool moves them right in the
     middle of it.
  3. I don't want to write "Url" because it's completely wrong but the
     tool will "fix" my correct "URL" each time.
  4. I keep (if-ed) BuildRoot in some of my specfiles because I need
     them to build on SLE11. The tool started to eat it. Actually, it
     eats the "BuildRoot" line and leaves an empty %if-%endif section
     in place.

Out of these, only 3 and 4 make sense to report as bugs; in fact, I
already tried with "3" earlier but the response was that can't be done
with the way spec-cleaner handles tag canonicalization.

But the real problem is that 1 and 2 are typical "matter of taste"
cases. And there are more, e.g. someone prefers to always write curly
braces around macro and variable names, someone writes them whenever
they are not surrounded by spaces and someone only if necessary.

I fully understand that some maintainers would prefer different
formatting of a specfile than me and I certainly don't want to force my
preferences on them. Unfortunately, spec-cleaner is based on the idea
that there is only One Correct Way and there is no place for personal
preferences. That's why I find spec-cleaner useful but it would make me
very unhappy if someone decided to enforce it to be run on every
submission (or on every submission of people who didn't explicitely
disable it).

                                                         Michal Kubeček

--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Tomas Chvatal
Michal Kubecek píše v Po 24. 07. 2017 v 08:19 +0200:

> On Wednesday, 19 July 2017 13:32 Simon Lees wrote:
> > On 19/07/17 20:27, Jan Engelhardt wrote:
> > > On Wednesday 2017-07-19 11:24, Tomas Chvatal wrote:
> > > > Well we have replacement in spec-cleaner.
> > >
> > > That does not make it better - after all, it's a "pick your
> > > poison"
> > > kind of choice. s-c may not move comments, but in turn
> > > it transmogrifies other things.
> >
> > But at least when it does you can open a bugreport with a
> > description
> > of what has happened why it shouldn't have happened and then in
> > most
> > cases someone will reply to the bug report and get the issue fixed
> > at
> > some point.
>
> That doesn't help with the basic issue of the tool: it (or rather
> its
> author) assumes the is One and Only Right Way to write specfiles.
> Just
> for fun, I ran it on one of my specfiles (with -m) and diff-ed the
> original specfile and the result.
>
>   1. Sometimes I want to add an empty line between sections to
> visually
>      separate them. The tool eats them.
Michal, you were on the OSC and I think you were in the audience of the
spec-cleaner talk. I think i mentioned that I wrote switch that allows
you to keep your vertical alignment...

>   2. For specfiles with more patches (say, from ten up), I find it
> much
>      easier to read and work with if the "Patch*" lines are separated
>      from the rest of the header. The tool moves them right in the
>      middle of it.

That is why we created feature of codeblocks and you can do whatever
you fancy with them, even explaned that on the talk...

>   3. I don't want to write "Url" because it's completely wrong but
> the
>      tool will "fix" my correct "URL" each time.
Create a poll, I don't care what comes out of it. Simply put preferred
solution should be one key used for it everywhere.

>   4. I keep (if-ed) BuildRoot in some of my specfiles because I need
>      them to build on SLE11. The tool started to eat it. Actually, it
>      eats the "BuildRoot" line and leaves an empty %if-%endif section
>      in place.
You do realize you can inject the buildroot on projectconfig level and
not pollute packages for factory right? Also all these are in code in a
way you can introduce new switch like --sle11-compat and keep them
enabled, I simply didn't care about sle11 enough anymore.

Also conditionalizing the buildrequires is totally pointless, if you
define it to default value it has no effect being there all the time.

That empty condition is bug tho, maybe i should put check if there is
some content in nested condition and prune it if it gets empty, feel
free to report a bug.

Tom

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Michal Kubecek
On Monday, 24 July 2017 10:24 Tomas Chvatal wrote:
> Michal Kubecek píše v Po 24. 07. 2017 v 08:19 +0200:
> >
> >   1. Sometimes I want to add an empty line between sections to
> > visually
> >      separate them. The tool eats them.
>
> Michal, you were on the OSC and I think you were in the audience of
> the spec-cleaner talk. I think i mentioned that I wrote switch that
> allows you to keep your vertical alignment...

The only OSC I attended was in Prague back in 2012 (or even 2011),
I doubt you mean that one. I was at your talk about spec-cleaner some
time ago (in SUSE office) but I don't think you mentioned that there
either.

Anyway, there is -k option (--keep-space) but according to --help text,
it only preserves empty lines "in preamble" and, indeed, it only
preserves empty lines up to a certain point in specfile. Even if it did
preserve them in the whole file, there would still be two important
questions:

  - why are they not preserved by "-m" which, according to
    documentation, "does not do anything intrusive (ie. just sets the
    Copyright)"
  - if, one day, spec-cleaner is run automatically on every commit
    (instead of current service) which is the plan, IIUC, is this
    going to be the default?

> >   2. For specfiles with more patches (say, from ten up), I find it
> >      much easier to read and work with if the "Patch*" lines are
> >      separated from the rest of the header. The tool moves them
> >      right in the middle of it.
>
> That is why we created feature of codeblocks and you can do whatever
> you fancy with them, even explaned that on the talk...

OK, I'll look for the documentation on that.

> >   3. I don't want to write "Url" because it's completely wrong but
> >      the tool will "fix" my correct "URL" each time.
>
> Create a poll, I don't care what comes out of it. Simply put preferred
> solution should be one key used for it everywhere.

...which is exactly the problem I'm talking about. Moreover, in this
particular case, it would be a bit like creating a poll about what the
result of "2 + 2" is. "URL" is an acronym meaning "Uniform Resource
Locator" and as such, the correct way to write it is "URL", there is no
voting about that.

> >   4. I keep (if-ed) BuildRoot in some of my specfiles because I need
> >      them to build on SLE11. The tool started to eat it. Actually,
> >      it eats the "BuildRoot" line and leaves an empty %if-%endif
> >      section in place.
>
> You do realize you can inject the buildroot on projectconfig level and
> not pollute packages for factory right?

I didn't until now. That may be a solution - except I don't always have
the prjconf under my control.

> Also all these are in code in
> a way you can introduce new switch like --sle11-compat and keep them
> enabled, I simply didn't care about sle11 enough anymore.

Let me say I find this very disturbing, considering SLE11 SP4 is still
under regular support and is going stay so until March 2019.

> Also conditionalizing the buildrequires is totally pointless, if you
> define it to default value it has no effect being there all the time.

Even if I define them to what is their default value right now, I can
never know if the value is not going to change at some point. If there
was a way, I would prefer only setting BuildRoot if it's not predefined.
I'm not aware of any.

But I'm afraid this is all missing the point I was trying to make. What
I wanted to say was that current spec-cleaner, even with -m option, is
way to intrusive to be seriously considered a replacement of the OBS
service which is used now. For the record, I'm talking about the version
I tried few hours ago (which is the one from fully updated openSUSE
42.2).

                                                          Michal Kubeček

--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Simon Lees-3
In reply to this post by Michal Kubecek


On 24/07/17 15:49, Michal Kubecek wrote:

> On Wednesday, 19 July 2017 13:32 Simon Lees wrote:
>> On 19/07/17 20:27, Jan Engelhardt wrote:
>>> On Wednesday 2017-07-19 11:24, Tomas Chvatal wrote:
>>>> Well we have replacement in spec-cleaner.
>>>
>>> That does not make it better - after all, it's a "pick your poison"
>>> kind of choice. s-c may not move comments, but in turn
>>> it transmogrifies other things.
>>
>> But at least when it does you can open a bugreport with a description
>> of what has happened why it shouldn't have happened and then in most
>> cases someone will reply to the bug report and get the issue fixed at
>> some point.
>
> That doesn't help with the basic issue of the tool: it (or rather its
> author) assumes the is One and Only Right Way to write specfiles. Just
> for fun, I ran it on one of my specfiles (with -m) and diff-ed the
> original specfile and the result.
>
>   1. Sometimes I want to add an empty line between sections to visually
>      separate them. The tool eats them.
>   2. For specfiles with more patches (say, from ten up), I find it much
>      easier to read and work with if the "Patch*" lines are separated
>      from the rest of the header. The tool moves them right in the
>      middle of it.
>   3. I don't want to write "Url" because it's completely wrong but the
>      tool will "fix" my correct "URL" each time.
>   4. I keep (if-ed) BuildRoot in some of my specfiles because I need
>      them to build on SLE11. The tool started to eat it. Actually, it
>      eats the "BuildRoot" line and leaves an empty %if-%endif section
>      in place.
>
> Out of these, only 3 and 4 make sense to report as bugs; in fact, I
> already tried with "3" earlier but the response was that can't be done
> with the way spec-cleaner handles tag canonicalization.
>
> But the real problem is that 1 and 2 are typical "matter of taste"
> cases. And there are more, e.g. someone prefers to always write curly
> braces around macro and variable names, someone writes them whenever
> they are not surrounded by spaces and someone only if necessary.
>
I agree with some of the issues around whitespace etc, at the same time
as someone who reads a large number of specfiles I appreciate the
uniformity that spec cleaner brings. Almost every project I contribute
to has a coding standard and some of them even do crazy things like
mandate 3 spaces but you accept it and live with it so i've kinda just
treated spec cleaner in the same way.

--

Simon Lees (Simotek)                            http://simotek.net

Emergency Update Team                           keybase.io/simotek
SUSE Linux                           Adelaide Australia, UTC+10:30
GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B


signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Johannes Meixner

Hello,

On Jul 25 10:40 Simon Lees wrote (excerpt):
> I agree with some of the issues around whitespace etc, at the
> same time as someone who reads a large number of specfiles
> I appreciate the uniformity that spec cleaner brings.
> Almost every project I contribute to has a coding standard...

from my point of view the discussion has left what
the actual (severe) root issue is and moved towards
a minor "how to enhance a particular tool" expert talk.

From my point of view the root issue is:

An automated tool modifies sources silently and anonymously.

From my point of view it is never ever allowed
(even not legally allowed) that an automated tool
silently and anonymously modifies sources.

I think only those who are listed as the authors
are allowed to modify their sources.

I think silent and anonymous modifications could
even violate licenses (but I am not a lawyer).

At least I think silent and anonymous modifications are
against the ideas behind how things are meant to work
(e.g. modifications without an entry in the changelog).

I think if there are automated modifications
then the automated tool must make its modifications
as separated commits and the automated tool must make
an appropriate entry in the changelog so that it is clear
that the automated tool acts as a separated author.

From my point of view the right way for automated coding
style enforcement is that an automated check during commit
rejects sources that are not in compliance with the coding
style rules so that the author is forced to adapt his
sources to meet the coding style requirements.

Of course such an automated check during commit may only
warn about minor coding style violations and error out
only in case of major coding style violations.

Summary:
From my point of view nothing is wrong with automated checks
(I do appreciate helpful automated checks) but basically
all is wrong with silent and anonymous modifications.


Kind Regards
Johannes Meixner
--
SUSE LINUX GmbH - GF: Felix Imendoerffer, Jane Smithard,
Graham Norton - HRB 21284 (AG Nuernberg)

--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Peter Simons
Johannes Meixner writes:

 > From my point of view it is never ever allowed (even not legally
 > allowed) that an automated tool silently and anonymously modifies
 > sources.

I don't know what you mean by "anonymously" in this context, but I agree
with your sentiment about silently re-writing source code before
committing it. The fact that the state I commit to the server is not the
same state that I see on my hard drive before running "osc commit" is
quite unacceptable.

osc should verify that running spec-cleaner on the current state is a
no-op before allowing me to commit -- i.e. it should guarantee that *I*
run spec-cleaner before committing --, but osc should not run spec-cleaner
for me.

Having said that, I am hugely in favor of enforcing a rule that all spec
files must pass through spec-cleaner though, for the following reasons:

 - All spec files will look basically the same. Attributes are specified
   in the same order and in the same spelling, which makes it easier to
   read them. It's also easier to "grep" for things if spelling is
   uniform. Having a reliable structure simplifies the task of scripting
   things in general, because stronger assumptions can be made.

 - When spec files are normalized before committing (like, ensuring that
   all BuildDepends are ordered alphabetically), then the resulting
   diffs will be minimal. There won't be any pointless white-space
   changes obscuring the purpose of the modification. Nor will there be
   changes that turn the spelling of "Url" into "URL" for no apparent
   reason. That makes life easier for people who review SRs etc.

 - If every spec file is known to pass through spec-cleaner, then we can
   reliably re-run the tool on every spec file to perform mass style
   updates. A new version of spec-cleaner might, for example, re-write
   the copyright disclaimer, or it might replace an obsolete attribute
   with the newer version, or whatever. Once you know that all spec
   files can be passed through the tool without breaking them, you can
   use the tool for maintenance tasks that wouldn't be feasible
   otherwise.

Best regards,
Peter
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Jan Engelhardt-4
In reply to this post by Simon Lees-3

 On Wednesday, 19 July 2017 13:32 Simon Lees wrote:
> On 19/07/17 20:27, Jan Engelhardt wrote:
>> On Wednesday 2017-07-19 11:24, Tomas Chvatal wrote:
>>> Well we have replacement in spec-cleaner.
>>
>> That does not make it better - after all, it's a "pick your poison"
>> kind of choice. s-c may not move comments, but in turn
>> it transmogrifies other things.
>
> But at least when it does you can open a bugreport

As soon as I did that, they called that bug a feature. So s-c is not
an option either.
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Johannes Meixner
In reply to this post by Peter Simons

Hello,

On Jul 25 12:37 Peter Simons wrote (excerpt):
> Johannes Meixner writes:
>
> > From my point of view it is never ever allowed (even not legally
> > allowed) that an automated tool silently and anonymously modifies
> > sources.
>
> I don't know what you mean by "anonymously" in this context

With "anonymously" I meant what I had also written:
---------------------------------------------------------------
I think if there are automated modifications
then the automated tool must make its modifications
as separated commits and the automated tool must make
an appropriate entry in the changelog so that it is clear
that the automated tool acts as a separated author.
---------------------------------------------------------------

Changes by an automated tool are no changes made by me
so that the automated tool must make itself visible
as another separated author "who" changed it.

I think it could be even legally problematic when changes
that are not made by me happen to be public visible as
my commit with my commit message and my changelog entry.

Strictly speaking:
An automated tool that anonymously changes sources
lies to others about who changed what.


> like, ensuring that all BuildDepends are ordered alphabetically

In general alphabetical ordering is meaningless
( except exceptions like a telephone book ;-)
so that usually an enforced alphabetical ordering
removes information.

For example build requirements should be ordered according
to the reason behind why each build requirements are there
e.g. something like
--------------------------------------------------------------
# For 'configure --enable-fancystuff-all':
BuildRequires: fancystuff-devel
BuildRequires: fancystuff-extensions
BuildRequires: fancystuff-tools
# For making HTML and PDF documentation:
BuildRequires: docbook
BuildRequires: xml-tools
BuildRequires: html-tools
BuildRequires: ghostscript
BuildRequires: pdf-tools
--------------------------------------------------------------


In general it seems to be good when openSUSE RPM spec files
are in compliance with a reasonable openSUSE standard.

But on the other hand enforcing it could be a hindrance
for openSUSE contributors to use ready-made RPM spec files
from whatever upstream projects also for openSUSE RPMs
with only some minor openSUSE-specific adaptions
to get software easily built also for openSUSE.

What is more important for openSUSE:
Be open for others (and accept diversity)
or
be uniform (to make maintenance easier)?

Perhaps only for openSUSE core packages (whatever that
exactly means - perhaps packages that are also in SLE ?)
the RPM spec files and the RPM changelogs must be in
compliance with a standard to make maintenance easier?


Kind Regards
Johannes Meixner
--
SUSE LINUX GmbH - GF: Felix Imendoerffer, Jane Smithard,
Graham Norton - HRB 21284 (AG Nuernberg)

--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Dave Plater lst


On 25/07/2017 15:48, Johannes Meixner wrote:
> Changes by an automated tool are no changes made by me
> so that the automated tool must make itself visible
> as another separated author "who" changed it.
>
> I think it could be even legally problematic when changes
> that are not made by me happen to be public visible as
> my commit with my commit message and my changelog entry.

I agree, the automated tool must at least present the changes it wishes
to make to the committer and present the option to override them.
I get around this problem using kwrite for spec files and if the file
gets changed on check in it warns me and I can see a diff of the changes.
I've never seen anything apart from a slight reordering of build
requirements and an update of the copyright year however when I had
spec-cleaner-format_spec_file instead of obs-service-format_spec_file
I quickly uninstalled it because it moved around conditionals and other
destructive things.
It's possible that this thread relates to spec-cleaner-format_spec_file
and not obs-service-format_spec_file.
Best regards
Dave P
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Sebastian-2
In reply to this post by Peter Simons
Hi,


On 07/25/2017 12:37 PM, Peter Simons wrote:
> osc should verify that running spec-cleaner on the current state is a
> no-op before allowing me to commit -- i.e. it should guarantee that *I*
> run spec-cleaner before committing --, but osc should not run spec-cleaner
> for me.
Or: It shows a diff with the proposed changes and the user can either
accept them or abort the commit. Would be much more user-friendly and
easier :)



--
python programming - mail server - photo - video - https://sebix.at
cryptographic key at https://sebix.at/DC9B463B.asc and on public keyservers


signature.asc (871 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: obs-service-format_spec_file is moving comments around

Mikhail Terekhov-3
In reply to this post by Peter Simons
On Tue, Jul 25, 2017 at 6:37 AM, Peter Simons <[hidden email]> wrote:
> Johannes Meixner writes:
>
...
> Having said that, I am hugely in favor of enforcing a rule that all spec
> files must pass through spec-cleaner though, for the following reasons:
>
>  - All spec files will look basically the same. Attributes are specified
>    in the same order and in the same spelling, which makes it easier to
>    read them. It's also easier to "grep" for things if spelling is
>    uniform. Having a reliable structure simplifies the task of scripting
>    things in general, because stronger assumptions can be made.

If by spelling you mean case then most programming editors support
case insensitive search.

Reliable scripting should be based on the spec file syntax not formatting.

>
>  - When spec files are normalized before committing (like, ensuring that
>    all BuildDepends are ordered alphabetically), then the resulting
>    diffs will be minimal. There won't be any pointless white-space
>    changes obscuring the purpose of the modification. Nor will there be
>    changes that turn the spelling of "Url" into "URL" for no apparent
>    reason. That makes life easier for people who review SRs etc.

A normalization tool should not be allowed to perform any kind of semantic
changes. Only formatting. And even then - not anonymously.

>
>  - If every spec file is known to pass through spec-cleaner, then we can
>    reliably re-run the tool on every spec file to perform mass style
>    updates. A new version of spec-cleaner might, for example, re-write
>    the copyright disclaimer, or it might replace an obsolete attribute
>    with the newer version, or whatever. Once you know that all spec
>    files can be passed through the tool without breaking them, you can
>    use the tool for maintenance tasks that wouldn't be feasible
>    otherwise.

Any maintenance tool could be reliable only if it is based on the spec
file syntax and if it is stable (as in stable sort) to be easily reviewable.

Best regards,
Mikhail
--
To unsubscribe, e-mail: [hidden email]
To contact the owner, e-mail: [hidden email]

12