farmOS-map icon indicating copy to clipboard operation
farmOS-map copied to clipboard

Stop maintaining fork of ol3-google-maps

Open mstenta opened this issue 4 years ago • 5 comments

This is a bit of a thorny issue... opening this to document it for future reference... I may end up working around it in the short-term, at least until a proper upstream fix is available.

Back story:

farmOS-map includes the ol3-google-maps library for Google Map layer support (added via #8). However, when Google Layers are put in a layer group, they do not work. This is documented in this upstream issue: https://github.com/mapgears/ol3-google-maps/issues/285

I dug into it and figured out the cause, described here: https://github.com/mapgears/ol3-google-maps/issues/285#issuecomment-568072168

I managed to fix the issue for farmOS's immediate needs, but it is not a "complete" solution, so it was not ready for a pull request to be opened and merged upstream. I didn't had time to work on it further, so I simply created a fork with a farmOS-map branch containing the fixes we needed and updated farmOS-map's package.json to point to that fork+branch.

See this commit:https://github.com/farmOS/farmOS-map/commit/9cb831146def46cdc913a5093ecaf6914e3b9e09

And this comment: https://github.com/farmOS/farmOS-map/issues/8#issuecomment-570804872

Fast forward to today:

Now, we are trying to pull farmOS-map into farmOS 2.x via Composer (https://www.drupal.org/project/farm/issues/3186641). We decided that we need to host farmOS-map on NPM and pull it in via asset-packagist.org (the reasoning for this is described in that issue).

See related issue: #64

However, after publishing v1.3.0 to NPM, attempting to pull it in via Composer results in the following error:

$ composer require npm-asset/farmos.org--farmos-map:^1.3
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - npm-asset/farmos.org--farmos-map 1.x-dev requires npm-asset/github:mstenta/ol3-google-maps dev-farmos-map -> no matching package found.
    - npm-asset/farmos.org--farmos-map 1.3.0 requires npm-asset/github:mstenta/ol3-google-maps dev-farmos-map -> no matching package found.
    - Installation request for npm-asset/farmos.org--farmos-map ^1.3 -> satisfiable by npm-asset/farmos.org--farmos-map[1.3.0, 1.x-dev].

Potential causes:
 - A typo in the package name
 - The package is not available in a stable-enough version according to your minimum-stability setting
   see <https://getcomposer.org/doc/04-schema.md#minimum-stability> for more details.
 - It's a private package and you forgot to add a custom repository to find it

Read <https://getcomposer.org/doc/articles/troubleshooting.md> for further common problems.

Installation failed, reverting ./composer.json to its original content.

It seems that asset packagist does not support github fork+branch dependencies.

I found this issue, which sounds like it is the same, but no solutions: https://github.com/hiqdev/asset-packagist/issues/133

Proper long-term solution

The correct solution to this is to fix https://github.com/mapgears/ol3-google-maps/issues/285 upstream, so that we can simply pull in olgm via NPM normally.

Potential short-term solution

A potential short-term solution would be to publish my fork+branch on NPM so that we can reference it directly. I hate maintaining a fork like that, but we don't have the resources to dig into the proper solution right now. Ideally, we can just delete the NPM package in the future when https://github.com/mapgears/ol3-google-maps/issues/285 is fixed, and collapse/simplify this whole thing (and close this issue). But for now, I think that's the approach I'm going to take...

mstenta avatar Feb 09 '21 13:02 mstenta

publish my fork+branch on NPM so that we can reference it directly

https://www.npmjs.com/package/@farmos.org/farmos-olgm

mstenta avatar Feb 09 '21 14:02 mstenta

I implemented the described workaround. Renaming this issue to "Stop maintaining fork of ol3-google-maps"... to be continued...

mstenta avatar Feb 09 '21 19:02 mstenta

@symbioquine: Curious how we should proceed with this issue now that the dependency on @farmos.org/farmos-olgm has been moved to https://github.com/symbioquine/farm_map_google

farmOS-map is no longer depending on it, but technically the @farmos.org organization is still "maintaining" the fork.

mstenta avatar Sep 06 '21 12:09 mstenta

I think it's still mainly a matter of getting your issue https://github.com/mapgears/ol3-google-maps/issues/285 resolved. I can try following up on that. Maybe I can find a strategy that they'll accept a PR for...

symbioquine avatar Sep 06 '21 16:09 symbioquine

Looks like we'll be able to stop using/supporting the farm_map_google module once we update to OpenLayers 9 - and maybe add some core behavior to support it...

https://openlayers.org/en/latest/examples/google.html https://github.com/openlayers/openlayers/commit/7245d80e118b462cd613c8f55645f6cb05e1d84e https://github.com/openlayers/openlayers/commit/5755b13bdf7cd396b1cc01c37bf90334cce5fe5e

symbioquine avatar May 07 '24 14:05 symbioquine