Skip to content
This repository has been archived by the owner on Jan 11, 2019. It is now read-only.

uncrustify: 0.66.1-0 in 'bouncy/distribution.yaml' [bloom] #96

Merged
merged 1 commit into from
Jun 18, 2018

Conversation

mikaelarguedas
Copy link
Member

Increasing version of package(s) in repository uncrustify to 0.66.1-0:

@mikaelarguedas mikaelarguedas merged commit 7711518 into ros2:ros2 Jun 18, 2018
@mikaelarguedas mikaelarguedas deleted the bloom-uncrustify-0 branch June 18, 2018 10:38
@dirk-thomas
Copy link
Member

dirk-thomas commented Jun 26, 2018

This Debian package shadows the package available from the Ubuntu repos: https://packages.ubuntu.com/en/bionic/uncrustify

Can we just rely on that instead (on Ubuntu)?

Actually the Ubuntu package shadows ours because it has a version suffix: 0.66.1+dfsg1-1

@mikaelarguedas
Copy link
Member Author

Yeah ideally we would not need to release a custom version on Ubuntu.
We currently use a specific hash (needed for e.g. Windows builds) and build it on all platforms for consistency in testing.
A better outcome would be to specify the minimal version we need on Ubuntu for example and just vendor it if the system version is older.

I didn't test if the bionic version 0.66.1allows us to pass all tests.
Results in that direction would be valuable and would allow us to use upstream wherever we can.

Another good news is that upstream just tagged 0.67 so we could start using that instead of a soecific commit hash

@dirk-thomas
Copy link
Member

Another good news is that upstream just tagged 0.67 so we could start using that instead of a soecific commit hash

We don't want to jump forward just "because we can". I think it is more valuable to stick to the version available in Ubuntu.

So what do you propose to do for the shadowing problem?

@mikaelarguedas
Copy link
Member Author

We don't want to jump forward just "because we can".

No but we should not track random commits when we can track released versions that include the fixes we need. So we should move to 0.67 that includes the fixes that motivated the switch to 0.66.1 + some commits.

So what do you propose to do for the shadowing problem?

I suggest confirming that 0.66.1 available in bionic satisfies our needs on Linux and to create a vendor package for uncrustify. That vendor package will use / depend on the system version if high enough, otherwise builds and install a recent enough version

@dirk-thomas
Copy link
Member

but we should not track random commits

Can you clarify what "random commits" you are referring to?

@mikaelarguedas
Copy link
Member Author

Can you clarify what "random commits" you are referring to?

What I mean by random commits is "all commits up to the fix we need on top of a released version".
Oh I think I understand what you mean.
As the version number we had in the package.xml was pointing to a specific date we imported uncrustify that was in between versions, I thought it was the current state. But what you mean is that we are tracking exactly 0.66.1 and only cherry-picked one fix we needed from upstream (namely uncrustify/uncrustify#1648), is that right ?.

I think the conclusion is still the same though: we should release it under a different name via a vendor package and build it from source only on platforms where we need it otherwise pull the upstream version if high enough. Do you agree?

@dirk-thomas
Copy link
Member

The diff to the released version should be minimal or even none. This would need to be checked to be sure.

we should release it under a different name via a vendor package and build it from source only on platforms where we need it otherwise pull the upstream version if high enough. Do you agree?

Yes, that is why I was wondering why this Debian release was needed.

@mikaelarguedas
Copy link
Member Author

Yes, that is why I was wondering why this Debian release was needed.

My guess is that because our debs are as close as what CI builds as possible and this package is never installed from upstream on CI and always built from source

@dirk-thomas
Copy link
Member

The problem is that the Ubuntu package if overlaying our custom package. In my opinion that should not stay this way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants