mojohaus / rpm-maven-plugin

http://www.mojohaus.org/rpm-maven-plugin/
Other
56 stars 48 forks source link

Parameter obsoletes implies provides #24

Closed snigel closed 8 years ago

snigel commented 8 years ago

When adding the obsoletes parameter an equal provides parameter will be added.

This makes it impossible to create a package like this: provides myCurrentPackage ${version} provides myObsoletedPackage ${version} obsoletes myObsoletedPacakage < x.y.z

Because the result will be: provides myCurrentPackage ${version} provides myObsoletedPackage ${version} provides myObsoletedPacakage < x.y.z obsoletes myObsoletedPacakage < x.y.z

In other words, my package will now provide the same thing that it is obsoleting, making it impossible to use the older versions of myObsoletedPackage.

The code for this can be found in AbstractRPMMojo.java

// if this package obsoletes any packages, make sure those packages are added to the provides list
        if ( obsoletes != null )
        {
            if ( provides == null )
            {
                provides = obsoletes;
            }
            else
            {
                provides.addAll( obsoletes );
            }
        }

My guess is that the code above should be removed and that 'provides' should be added manually.

Here is a minimal pom to reproduce the issue:

<?xml version="1.0" encoding="UTF-8"?>
<project>
  <modelVersion>4.0.0</modelVersion>
  <groupId>collection.trash</groupId>
  <artifactId>landfill</artifactId>
  <version>1.0-SNAPSHOT</version>
  <packaging>jar</packaging>

<properties>
  <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<build>
    <plugins>
      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>rpm-maven-plugin</artifactId>
        <version>2.1.4</version>
        <executions>
          <execution>
            <id>generate-rpm</id>
            <goals>
              <goal>rpm</goal>
            </goals>
          </execution>
        </executions>
        <configuration>
          <group>Application/Collectors</group>
          <obsoletes>
            <obsolete>test &lt; 3.2.1 </obsolete>
          </obsoletes>
        </configuration>
      </plugin>
    </plugins>
  </build>
</project>
bokken commented 8 years ago

The change for this should be to provide an option to disable the current behavior of implying provide. Simply changing to no longer imply provide is non-passive to existing users expecting that behavior.

goeranu commented 8 years ago

Maybe I should elaborate a bit on this.

This functionality seems to try to automate a common pattern. But it is not quite how the pattern is meant to be used. If a package which replaces another package, and if it is compatible enough, then it should both do obsoletes and provides. But it should do so specifying explicit versions. It should do obsoletes on all versions of the package that is older or equal to the last version it replaces. It should do provides on exactly a newer version.

On the other hand, it should only do obsoletes but not provides if it replaces, but in a less compatible way. What is "compatible enough" is a judgement the packager needs to handle in each case.

See for example openSUSE Upgrade Dependencies and Fedora Packaging Guidelines for a more detailed discussion on the subject.

The current functionality that copies the obsoletes to provides means the package claims to actually provide all old versions of the package. If the obsoletes is on the name only, the new package claims to provide any and all version of the obsoleted package. If the obsoletes includes "< x.y.z", it claims to also provide all those old versions. Neither is correct!

I would advice against making this a option; it would never be correct to use it.

dantran commented 8 years ago

@bokken your thought on @goeranu's detail explaination. I am not a RPM heavy user to make this call

bokken commented 8 years ago

If you would like to change the behavior to handle versions correctly (I agree the current behavior if a version is present is completely wrong) that would be great.

This project has a rather long history of maintaining passive behavior, even if that behavior is less than optimal.

My suggestion is that a flag be added which disables the implying of provision from obsoletion, which gives the behavior you desire while remaining passive to current users. A more complete handling of versions would be great, but that could be added in the future if you are not wanting to do that work now.

goeranu commented 8 years ago

I still find it strange to have a flag for this. Sure, it is good to “maintain passive behavior” as you put it. But to the extent of not fixing bugs? I can not see any case where the current behavior would be correct; it is a bug. So everyone who in the future will use the obsolete feature with the rpm plugin will have to rediscover it. And have to find out they have to add a flag to get the normal behavior. Is that really a good idea?

dantran commented 8 years ago

I kind agree with @goeranu

we can disable the unintended code by default, and allow activate via system properties. if no complaint after a few release we will remove it permanently.