Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update MapLibre GL JS submodule to v1.15.2 #309

Closed
wipfli opened this issue Jun 7, 2022 · 21 comments
Closed

Update MapLibre GL JS submodule to v1.15.2 #309

wipfli opened this issue Jun 7, 2022 · 21 comments
Assignees

Comments

@wipfli
Copy link
Contributor

wipfli commented Jun 7, 2022

I would like to update the MapLibre GL JS submodule to the latest 1.x version. This will probably affect the shaders and the style spec.

@wipfli
Copy link
Contributor Author

wipfli commented Jun 7, 2022

Relevant files could be:

scripts/generate-shaders.js
scripts/generate-style-code.js
platform/ios/scripts/generate-shaders.js
platform/ios/scripts/generate-style-code.js
platform/ios/platform/darwin/scripts/generate-style-code.js
platform/android/scripts/generate-style-code.js
platform/android/scripts/generate-test-code.js

@wipfli
Copy link
Contributor Author

wipfli commented Jun 7, 2022

This commit from @petr-pokorny-1 could be relevant: 672f2d8

@wipfli
Copy link
Contributor Author

wipfli commented Jul 2, 2022

https://github.com/maplibre/maplibre-gl-native/pull/6/files shows some things from GL JS which are used in native...

@wipfli
Copy link
Contributor Author

wipfli commented Jul 2, 2022

Lot's of testing stuff seems shared, @HarelM

@wipfli
Copy link
Contributor Author

wipfli commented Jul 2, 2022

#339

@wipfli
Copy link
Contributor Author

wipfli commented Jul 5, 2022

The commit hashes in the src/shaders folder before typescript are

eeefc070d7333b21b205be3882df9b2e3cd444d1
d8cf29023392b7de85b365cdc17e50ae263a357c
e27049d87be346a4f67ef6956a5113d977de63d8
aa6b4b2e51ac19156ba1537baf32a63551313612
58a73906d6f467762e486fcddf0643b27193ba89
842ea2e08f57dbc4c70537dc9988512439d05f9d
12991995fa1bb62c5ecdb0282c7c142578b897e5

@wipfli
Copy link
Contributor Author

wipfli commented Jul 5, 2022

I would like to run the update-gl-js.yml action on those and see at what point stuff breaks...

@ntadej
Copy link
Collaborator

ntadej commented Jul 5, 2022

Can you also confirm that the same set of tests is ignored in JS than in native?

@wipfli
Copy link
Contributor Author

wipfli commented Jul 5, 2022

In native, depending on the platform different tests are ignored. For a full list, see https://github.com/maplibre/maplibre-gl-native/tree/main/metrics/ignores

In GL JS, we might actually run all the render tests. See https://github.com/maplibre/maplibre-gl-js/blob/main/test/integration/render/render.test.ts ...

@wipfli
Copy link
Contributor Author

wipfli commented Jul 5, 2022

@HarelM How do we handle ignored tests again in GL JS? I remember there was some discussion about removing ignored tests...

@wipfli
Copy link
Contributor Author

wipfli commented Jul 5, 2022

eeefc070d7333b21b205be3882df9b2e3cd444d1 is in #360

@wipfli
Copy link
Contributor Author

wipfli commented Jul 5, 2022

d8cf29023392b7de85b365cdc17e50ae263a357c is in #361

@wipfli
Copy link
Contributor Author

wipfli commented Jul 5, 2022

e27049d87be346a4f67ef6956a5113d977de63d8 is in #362

@HarelM
Copy link
Collaborator

HarelM commented Jul 5, 2022

We removed ignored tests.

@wipfli wipfli mentioned this issue Jul 6, 2022
@louwers louwers self-assigned this Jan 23, 2023
@louwers
Copy link
Collaborator

louwers commented Jan 23, 2023

This seems like a difficult but important ticket. I assigned myself because I am going to spend some time looking into this, but don't let that hold anyone back working on this or assigning themselves as well.

@acalcutt
Copy link
Collaborator

acalcutt commented Jan 23, 2023

Is this still only trying to get MapLibre GL JS to lastest in the 1.X branch and not the current main 2.x/3.x release? If it was going to 2.x I also wanted to note the path to tests seems to have changed. If it is just to 1.X this is not an issue (yet)

For example, in a linux build you can run tests with
xvfb-run -a ./build/mbgl-render-test-runner --manifestPath metrics/macos-xcode11-release-style.json
and it uses a bunch of tests from
maplibre-gl-js\test\integration\render-tests

But in the current main branch of maplibre-gl-js that path has moved to
maplibre-gl-js\test\integration\render\tests
https://github.com/maplibre/maplibre-gl-js/tree/main/test/integration/render/tests

I know there was also some discussion of moving these tests into this project...so maybe this would be fixed by doing that.

@ntadej
Copy link
Collaborator

ntadej commented Jan 23, 2023

The biggest issue is that we've observed breakage of tests when trying to update the version even in the 1.X branch. See some of the old open MRs.

@wipfli
Copy link
Contributor Author

wipfli commented Jan 23, 2023

The strategy should probably be to update the GL JS submodule commit by commit and make sure everything works every time. So you would first go from Mapbox v1.x in March 2020 (where it is now) to something near December 2020 (time or fork). This period has some new features in GL JS from Mapbox but not so many. Then you would follow MapLibre v1 until something like August 2021 (TypeScript and v2). This period has almost no new features from MapLibre. And finally, you would follow MapLibre v2 until today. This time period also has very few new features.

So the good news is that there is some work to be done around tooling, but the style spec did not change much since March 2020 and so there is not that crazy much work to be done feature-wise...

@louwers louwers changed the title Update MapLibre GL JS Update MapLibre GL JS submodule to v1.15.2 Jan 31, 2023
@louwers
Copy link
Collaborator

louwers commented Feb 3, 2023

$ cd maplibre-gl-js
$ git rev-list HEAD^..$(git rev-list -n 1 "v1.14.0") | wc -l
189

188 commits until v1.14.0

I am looking what kind of issues we run into and will make more issues for the milestone.

@louwers
Copy link
Collaborator

louwers commented Feb 17, 2023

Note that the workflow does not run

platform/android/scripts/generate-style-code.js
platform/android/scripts/generate-test-code.js

https://github.com/maplibre/maplibre-gl-native/blob/main/.github/workflows/update-gl-js.yml

@wipfli wipfli mentioned this issue Feb 23, 2023
@louwers
Copy link
Collaborator

louwers commented Mar 23, 2023

Change of plans. #938

@louwers louwers closed this as not planned Won't fix, can't repro, duplicate, stale Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

6 participants