[opensuse-packaging] obs-service-format_spec_file is moving comments around
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: 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: 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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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 -- View this message in context: http://opensuse.14.x6.nabble.com/obs-service-format-spec-file-is-moving-comm... Sent from the opensuse-packaging mailing list archive at Nabble.com. -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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: 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: 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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
--
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
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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
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.
From my point of view the right way for automated coding
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. 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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
On 28/07/17 03:04, Dave Plater wrote:
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
Cross posting to opensuse-buildservice, being able to review the contents of these changes before sending the commit would be nice but it probably requires new features in the buildservice which I would welcome, if only for the times when i'm doing a commit to a maintenance update and it triggers major spec file changes or worse a unintended run of one of the source services completely changing the tarball. -- 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
On Freitag, 28. Juli 2017, 01:27:26 CEST wrote Simon Lees:
On 28/07/17 03:04, Dave Plater wrote:
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
Cross posting to opensuse-buildservice, being able to review the contents of these changes before sending the commit would be nice but it probably requires new features in the buildservice which I would welcome, if only for the times when i'm doing a commit to a maintenance update and it triggers major spec file changes or worse a unintended run of one of the source services completely changing the tarball.
a simple osc service run osc diff should show you any changes happen to your sources. However, you should discuss changes with the maintainers of these service packages. The OBS people only jump in on very grave problems here :) The format_spec_file should never touch a tar ball though. There are others which can do that, but these are usually disabled by default in openSUSE main packages. Only an explicit osc service runall will execute them. -- Adrian Schroeter email: adrian@suse.de SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) Maxfeldstraße 5 90409 Nürnberg Germany -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
On 28/07/17 09:05, Adrian Schröter wrote:
On Freitag, 28. Juli 2017, 01:27:26 CEST wrote Simon Lees:
On 28/07/17 03:04, Dave Plater wrote:
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
Cross posting to opensuse-buildservice, being able to review the contents of these changes before sending the commit would be nice but it probably requires new features in the buildservice which I would welcome, if only for the times when i'm doing a commit to a maintenance update and it triggers major spec file changes or worse a unintended run of one of the source services completely changing the tarball.
a simple
osc service run osc diff
should show you any changes happen to your sources. However, you should discuss changes with the maintainers of these service packages. The OBS people only jump in on very grave problems here :)
The format_spec_file should never touch a tar ball though. There are others which can do that, but these are usually disabled by default in openSUSE main packages. Only an explicit
osc service runall
will execute them.
The reason I included this list is that i'm after a extra feature in osc commit, because people are complaining that its service handling is sub par (feature request as follows): Based off a command line parameter to osc commit (feature either always on with a disable flag, or off with an enable flag). Should the flag for the feature be set, before the services run osc commit would store a copy of the diff created by osc diff, then the service would run after that another diff would occur. If there was a difference between the old and new (ie a service had changed something) a warning would be displayed to the user saying "Services have modified your package in the following way... <Show a diff of changes> you can accept these changes or abort the commit and run again with --no-service" Obviously not everyone will want this and in some cases ie a repo building a git project people will want to ignore it so by adding a commandline param people can update there alias to suite what they want. -- 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
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
On Tue, Jul 25, 2017 at 6:37 AM, Peter Simons
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org
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)
participants (14)
-
Adrian Schröter -
Dave Plater -
Jan Engelhardt -
Johannes Meixner -
Luigi Baldoni -
Michal Kubecek -
Michal Srb -
Mikhail Terekhov -
Peter Simons -
Richard Biener -
Rüdiger Meier -
Sebastian -
Simon Lees -
Tomas Chvatal