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

fix build by including a copy of python dev config #362

Conversation

kopp
Copy link

@kopp kopp commented Oct 7, 2024

This fixes long standing build issues as described in #361

@kopp kopp mentioned this pull request Oct 7, 2024
@immel-f
Copy link
Contributor

immel-f commented Oct 9, 2024

Thanks a lot for fixing that error!

@immel-f
Copy link
Contributor

immel-f commented Oct 9, 2024

If I understood it correctly, you hard copied the entire python_dev_config repo into this repository to fix the dependency issues. Would it be possible to pull the correct version in the dockerfile from git during the build instead? That would make things more intuitively documented and nicer from a dependency perspective

@kopp
Copy link
Author

kopp commented Oct 9, 2024

@immel-f , I had to adapt the package in two ways:

  • I adapted to the 'new' conan api (I had to make export a function rather than a field)
  • I removed the tests of that package because they generated issues during the lanelet2 test stage
    So we could create a separate repo hosting that modified code and pull and build that, but I think that will add more complexity. But if you really want that, it can be done.

As an alternative, we would host the conan artifact somewhere (as it was done by bincrafters), e.g. by using https://github.com/thejohnfreeman/redirectory and just pull that in as drop-in replacement for the bincrafters one. But I am not a conan expert and this would probably take some more time to set up/get it to work.

@poggenhans
Copy link
Contributor

thank you for pushing this topic! You are right, we definitively need to renew our conan code. I actually started this already a while ago on this branch: https://github.com/poggenhans/Lanelet2/tree/update_conan, and I picked this up again now.

This branch will add support for conan >2.0 and also remove the need for the old python_dev_config dependency so it should be a bit cleaner than this PR. I just need a few days to get the CI green.

@kopp
Copy link
Author

kopp commented Oct 10, 2024

@poggenhans Cool, and it also brings python 3.12!
I'm happy with that as well, if it does not happen too far in the future ;) . I actually started my digging to get python 3.12 support, which I need.

If your cleaner solution takes longer then expected, could we either

  • use my proposed branches to generate a release 1.2.2 that also supports python 3.12 -- even without merging it or
  • merge my changes as an intermediate solution until your cleaner solution is done?

@poggenhans
Copy link
Contributor

I hope it should be just a matter of a few days. There shouldn't be many problems left. Building for python 3.12 works already, I just need to convince our CI of it.

@kopp
Copy link
Author

kopp commented Oct 18, 2024

not necessary after #364

@kopp kopp closed this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants