#122 Update Eclipse Packaging Guidelines
Closed: Fixed None Opened 7 years ago by swagiaal.


So, there are a few things that will need to be adjusted here:

#1) You really can't use a magic macro in the way that you're proposing for the reconciler. It doesn't account for situations where scriptlets are already in use. You'll need to change it to work like this:

%_eclipse_pkg_post (evaluates to touch /var/run/eclipse/run-reconciler )
%_eclipse_pkg_postun (evaluates to:
touch /var/run/eclipse/run-reconciler

if [ $1 == 0 ]; then
/full/path/to/eclipse-reconciler.sh
fi
)
%_eclipse_pkg_posttrans (evaluates to /full/path/to/eclipse-reconciler.sh)

In addition, we need to see a copy of eclipse-reconciler.sh, so we can sanity check it and ensure that the scriptlets don't need anything special, like ignoring output or ignoring failure cases. We also want to know why the script needs to be run in %postun and %posttrans.

Last, as a minor issue, we think you should just lose all the "Since Fedora 9" language in the document. Fedora 9 is long dead at this point. :)

Please show us a new set of changes and we'll reconsider this draft.

(Vote: (+1:6, 0:0, -1:0) )

1) You really can't use a magic macro in the way that you're proposing for the reconciler. It doesn't account for situations where scriptlets are already in use.

I have split up the macro's as requested. I also left the eclipse_pkg macro there for use when scriptlets are not already in use. I anticipate that this will be the common case and so I would like to simplify and reduce the work which needs to be done by Eclipse packagers. I hope that is ok. I will attached a copy of the macros file for your consideration.

In addition, we need to see a copy of eclipse-reconciler.sh

I will attach one.

Last, as a minor issue, we think you should just lose all the "Since Fedora 9"

Done: https://fedoraproject.org/w/index.php?title=User:Swagiaal/NewEclipsePackagingGuidelines&diff=263371&oldid=263347

Please remove it. It's really not a direction that we want macros to take.

Agreed. Please edit the draft and the macros to remove the %{eclipse_pkg} macro, and instead, provide an example of proper use of the split out macros. I think with that change, it should pass without issue.

Couple questions about the reconciler.sh script:

  • What are the conditionals for -Dosgi.checkConfiguration=true for? Is this script also run by the eclipse package when it updates? Or is it run by end users?
  • I noticed that the backups of the config files are being stored under /var/run/. I don't think this is a good idea -- A tmp dir created using mktemp -d is better.

Replying to [comment:5 spot]:

Agreed. Please edit the draft and the macros to remove the %{eclipse_pkg} macro, and instead, provide an example of proper use of the split out macros. I think with that change, it should pass without issue.

Okay I have removed the _eclipse_pkg macro and update the draft. I will be attaching an update version of the macros file

  • What are the conditionals for -Dosgi.checkConfiguration=true for? Is this script also run by the eclipse package when it updates? Or is it run by end users?

This flag is used when the platform is updated, because the files we ship in the platform rpm will overwrite all the configuration work which the reconciler has done. This flag says ignore what you think is installed and reinstall everything.

It is run by the platform and plugin packages. I hope that there will never be a need for end users to run it but just in case it put some comments and safeguards in there.

  • I noticed that the backups of the config files are being stored under /var/run/. I don't think this is a good idea -- A tmp dir created using mktemp -d is better.

That directory was created following the instructions for tmpfiles found here: http://fedoraproject.org/wiki/Packaging:Tmpfiles.d, but I will change it to use mktemp which makes things much simpler

First one sounds good.

About tmpfiles, thanks. I'll clarify that page too. Tmpfiles.d is really only for daemons.

About tmpfiles, thanks. I'll clarify that page too. Tmpfiles.d is really only for daemons.

I have attached the modified eclipse-reconciler.sh. It now uses mktemp -d.
I realized that we still need /usr/run/eclipse because that is where we create the file /var/run/eclipse/run-reconciler. So I'll keep that directory as is (that is created using tmpfiles) unless you have another recommendation.

Thanks.

%{_localstatedir}/lib/rpm-state/eclipse

The eclipse package (if that's what's providing macros.eclipse) should own both
%dir %{_localstatedir}/lib/rpm-state
%dir %{_localstatedir}/lib/rpm-state/eclipse

Unless we have filesystem or rpm own %{_localstatedir}/lib/rpm-state for us.

It appears we're going to lift the %_localstatedir/lib/rpm-state bit out to a common guideline and make sure it gets owned properly.

A couple of minor points:

We've generally been collecting EPEL-specific information at http://fedoraproject.org/wiki/Packaging:EPEL, so when the guidelines are written up, that EPEL5-specific section will be moved.

The OSGi section is odd. It is in general better to either not mention it, or write it up as if everything works as expected and add a notice box indicating that there are problems and referring to the relevant bugzilla ticket open on the issue. This makes it easy for us to just drop the notice when the problem is fixed instead of having to go through another guideline revision cycle.

Why is %eclipse_base something that needs to be set manually? Why is this not defined, at least to some useful default value, by macros.eclipse?

Replying to [comment:11 toshio]:

%{_localstatedir}/lib/rpm-state/eclipse

Okay I have update the spec file and scripts to use the rpm-state dir.
Attaching new files

Replying to [comment:12 tibbs]:

It appears we're going to lift the %_localstatedir/lib/rpm-state bit out to a common guideline and make sure it gets owned properly.

Will this be the case in f17 ? Should I have eclipse own/create this dir or is there no need ?

A couple of minor points:

We've generally been collecting EPEL-specific information at http://fedoraproject.org/wiki/Packaging:EPEL, so when the guidelines are written up, that EPEL5-specific section will be moved.

Yeah that section can be moved. I have removed it from the document.
https://fedoraproject.org/w/index.php?title=User:Swagiaal/NewEclipsePackagingGuidelines&diff=264219&oldid=263556

The OSGi section is odd. It is in general better to either not mention it, or write it up as if everything works as expected and add a notice box indicating that there are problems and referring to the relevant bugzilla ticket open on the issue. This makes it easy for us to just drop the notice when the problem is fixed instead of having to go through another guideline revision cycle.

The information there is out of date and does not belong here so I removed it.
https://fedoraproject.org/w/index.php?title=User:Swagiaal/NewEclipsePackagingGuidelines&diff=264220&oldid=264219

Why is %eclipse_base something that needs to be set manually? Why is this not defined, at least to some useful default value, by macros.eclipse?

That is a good call I have added that macro and some others to macros.eclipse and updated the guidelines:
https://fedoraproject.org/w/index.php?title=User:Swagiaal/NewEclipsePackagingGuidelines&diff=264223&oldid=264220

This was approved (vote info will be posted soon). Two points:

Please have eclipse own the rpm-state directory. We will try to get this into rpm soon but multiple ownership doesn't actually hurt anything.

Is there any chance you'd consider taking a look at the glossary section at the top? The plugin1/plugin2 thing is kind of odd.

Draft approved, minor cleanups may be requested in the ticket and directory ownership notes amended (+1:6, 0:0, -1:0)

Please have eclipse own the rpm-state directory. We will try to get this into rpm soon but multiple ownership doesn't actually hurt anything.

Will do.

Is there any chance you'd consider taking a look at the glossary section at the top? The plugin1/plugin2 thing is kind of odd.

Hmm... Is this better: https://fedoraproject.org/w/index.php?title=User:Swagiaal/NewEclipsePackagingGuidelines&diff=265508&oldid=264223 ?

Replying to [comment:16 spot]:

Draft approved, minor cleanups may be requested in the ticket and directory ownership notes amended (+1:6, 0:0, -1:0)

Thanks for all the guidance guys. What is the next step now that this is approved ?

That glossary looks far better to me. Not sure if anyone will have a problem with the external link there, though. I guess any committee members who do should speak up.

As for what happens next, the official guideline page will be updated and an announcement sent, generally when someone has time between now and our next meeting. There's nothing else you need to do unless you notice some problem with the approved guideline that you want us to look at before it gets written up.

Filed this bug to get rpm-state directory into the rpm package:

https://bugzilla.redhat.com/show_bug.cgi?id=771713

Regarding the glossary change: looks good to me.

There's nothing else you need to do unless you notice some problem with the approved guideline that you want us to look at before it gets written up.

Great! I fixed a minor typo here: https://fedoraproject.org/w/index.php?title=User:Swagiaal/NewEclipsePackagingGuidelines&diff=265519&oldid=265508

Announce text:

The Eclipse Plugin Packaging Guidelines were updated. The most major change is the addition of a section discussing how to run the reconciler. For the full updated guidelines see:
https://fedoraproject.org/wiki/Packaging:EclipsePlugins

Unfortunately we will no longer be moving forward with the reconciler solution because it exposes conflicts with with plugins installed by the user to their home directories. As a result I would like to request that the packaging guidelines be update to remove references to running the reconciler.

Here is the change:
https://fedoraproject.org/w/index.php?title=User:Swagiaal/EclipsePackagingGuidelinesNoReconciler&diff=271431&oldid=271429

And this should be the new guideline page:
https://fedoraproject.org/wiki/User:Swagiaal/EclipsePackagingGuidelinesNoReconciler

Hmm, interesting. Any reason not to just do this?

If not, +1 from me; if there are problems with the thing then of course it needs to go. Plus it does simplify things a bit.

Yeah, I'm just going to do this. If it's wrong, it's wrong, shouldn't need a vote to drop it.

Metadata Update from @tibbs:
- Issue assigned to spot

2 years ago

Login to comment on this ticket.

Metadata