cedaro / satispress

Expose installed WordPress plugins and themes as Composer packages.
509 stars 51 forks source link

Composer: Invalid version string #13

Closed alicam closed 6 years ago

alicam commented 7 years ago

The WPSSO plugin author uses a dash in his version strings. I think that's why I'm getting this:

[RuntimeException] Could not load package satispress/wpsso in https://packages.auswebhost.com/satispress: [UnexpectedValueException] Invalid version string "3.36.2-1"

I tried using wpackagist for WPSSO but it also was having issues, but perhaps for other reasons. I got the message that the WPSSO package didn't exist in the wpackagist repo!

Thoughts? Work-arounds?

A

GaryJones commented 7 years ago

http://semver.org/#spec-item-9 does allow for a version of 3.36.2-1, so this may need looking into on the SatisPress side of things.

GaryJones commented 7 years ago

If you look in https://packages.auswebhost.com/satispress/packages.json, you'll see that the version has been normalized (maybe by SatisPress - I've not checked yet) as 3.36.2.1.

GaryJones commented 7 years ago

There's some heavy regexes going in https://github.com/blazersix/satispress/blob/develop/includes/class-satispress-version-parser.php that might be the cause of this.

bradyvercher commented 7 years ago

I think I made a few minor changes to that file, but otherwise it came directly from Composer. It looks like they've done some refactoring since then, so it may take awhile to dig into this.

bradyvercher commented 7 years ago

After a bit of digging, it looks like that version format isn't supported by Composer itself, which is probably why it's not working anywhere.

GaryJones commented 7 years ago

Good find @bradyvercher.

In which case @alicam, you'll have to ask the plugin author to use something safer, or otherwise try using the 3.36.2.1 format in your consuming composer.json hope that Composer doesn't choke as it does now.

Regardless, it doesn't appear to be a SatisPress issue, so I'm closing this for now.

alicam commented 7 years ago

The plugin author - a very accomplished coder - has been notified, and I've asked him to look at this thread, and perhaps weigh in. I respect his views that his approach is acceptable. It just leaves me in a quandry :(

A

jsmoriss commented 7 years ago

There's plenty of evidence that dashes are acceptable for "stability" stages in both these files:

There does seem to be a lapse in understanding PHP version numbering though -- Note the production release "#" number between the rc (release candidate) and pl (patch level) stages in PHP's version compare function (see http://php.net/manual/en/function.version-compare.php for more info):

dev < alpha = a < beta = b < RC = rc < # < pl = p

A "stability" stage progression might look something like this:

It appears that the "production stage" is not included in the Composer / Satispress stability regex, which may explain this comment in the Composer class:

Numerical-only pre-release identifiers are not supported, see tests.

I'm not sure why Composer wouldn't use PHP's version compare function (or re-use it's code), but perhaps this is legacy code from Composer's original port from openSUSE's libzypp satsolver.

js.

jsmoriss commented 7 years ago

BTW, that Composer class includes a "stable" stability stage (which is not supported by PHP), so perhaps a possible work-around would be to run an applyfilters() on the version number to make it compatible with Composer (example, modifying '/([-])(\d+)$/' to '$1stable$2').

js.

GaryJones commented 7 years ago

Pinging @Seldaek in case he can enlighten us on this issue.

GaryJones commented 7 years ago

@jsmoriss Thank you for the insight and possible workaround. I do feel this should probably be something that is addressed in Composer, but if there are reasons why this can't happen, the workaround you've suggested is definitely worth looking into.

Seldaek commented 7 years ago

We support a bunch of formats, but not 1.2.3-1 because IMO it doesn't mean anything. Why not just add .1 instead? Why a dash there? We have some normalized formats and try not to add everything under the sun because it just adds confusion IMO. Forcing people into somewhat normalized schemes means better communication between maintainers and users. Ideally we'd all use semver.org but anyway..

alicam commented 7 years ago

So it's a Mexican standoff?! After a month, no progress. What's the resolution here that saves me (and I assume others) from having to manually remove the dash in version numbers for a group of plugins such as these, every time they're updated?! That's just stupid.

@jsmoriss suggests a REGEX filter. But I don't know where in the plugin I'd implement/apply it.

Hurting here.

Help?

A

GaryJones commented 7 years ago

While I think the use of -1 as a suffix is technically allowed within the Semver spec, I agree with @Seldaek that it doesn't mean much. If Composer (the CLI) app isn't going to recognise it, there's probably little point in deviating from it within the SatisPress code.

I understand your frustration.

Even with SatisPress normalizing it to 3.36.2.1, did you try to see if your consuming site would pull the package in question down, with the .1 reference (if you need to be that specific), or in fact going more general with the version requirement (such as '^3.36.2', '^3.36' or just '^3')?

alicam commented 7 years ago

@GaryJones I'm manually swapping out the dash for a dot. SO I'm converting 3.36.2-1 to 3.36.2.1 and it's working fine. But it's a pain. I'm waiting for a Composer update that automagically solves it! Maybe that's wishful thinking?!