[WIP] Fix bug in internal PBF deserialization of locations

[ianthetechie]

Issue

This will close #1448 and it will close #1468

I'll try to write up a test for this in the next day or two, and have marked it WIP until then. I didn't see a CONTRIBUTING doc (just a paragraph or 2 in the main README), so let me know if I am missing anything.

Tasklist

• Add tests
• Review - you must request approval to merge any PR to master
• Add #fixes with the issue number that this PR addresses
• Generally use squash merge to rebase and clean comments before merging
• Update the changelog
• Update relevant documentation

[ianthetechie]

Note that I was unable to get the tidy script to run on mac. I'll run it one way or another before removing the WIP flag.

[kevinkreiser]

no worries add a test and we'll review it when you're ready. if you cant get the tidy script to run thats also no problem we can run it for you. just ping back when you are ready for us to take a look!

[ianthetechie]

Hey @kevinkreiser mind taking a look now?

• I was able to get the tidy script to run on a Linux box.
• I wrote a test for the patch, and in the process touched one of the operator implementations and added a method since the original behavior seemed rather strange (fortunately, it appears to have only been used in a test, which incidentally documented the behavior better than the method docstring). If you'd prefer this be implemented differently, let me know. I identified usages by adding delete = to the operator to make sure this wouldn't have any unintended performance side-effects.
• My test does pass, but it looks like either master is busted, something externally changed, etc... I'm assuming this isn't my fault based on the string of red X's on the PR list, but if you need me to pull in some new changes or anything, let me know.
• I'd recommend backporting this to 2.x, as this issue affects all release builds since February.

[kevinkreiser]

related to #1468 i think

[kevinkreiser]

@ianthetechie i believe ive fixed the issue with the failing node tests 🤞 that CI passes as well

[kevinkreiser]

just for posterity, this is where the real bug lies. kBreak == 0 and therefore everything gets set to through. im pretty sure that this bug is my fault.. sorry all!

[blackgate]

I already checked this and it fixes the #1468
Thank you

[darbultr]

problem also exists on docker

[ianthetechie]

From the looks of it, dockerized builds are still very much WIP. I would recommend building the v2.7.0 from source.

[kevinkreiser]

just a freindly reminder, master is no longer unstable. feel free to build master from source instead of sticking on 2.7.0. and yeah we haven't given docker much love in a while.. need to find time to get back to that at some point!