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

Add project() to sqlite3_cmakelists.txt #324

Merged
merged 2 commits into from
Mar 18, 2020

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Mar 16, 2020

Attempting to address issue #323. I'm not sure if this is the correct way of going about this since it's being included from another cmakelists.txt. I'm also open to naming the project whatever you all think is appropriate. I just didn't want to name it sqlite3_vendor (already taken) nor sqlite3 (may conflict with another cmake project?).

Signed-off-by: Stephen Brawner brawner@gmail.com

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Contributor Author

brawner commented Mar 16, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987
Copy link
Collaborator

You repos file is incorrect I believe:

  ros2/rosbag2:
    type: git
    url: git@github.com:brawner/rosbag2
    version: sqlite3_cmakelists

the url should be provided in form of https://github.com/...

@Karsten1987
Copy link
Collaborator

I actually think naming it sqlite3 is okay as it should not conflict with any CMake context in this case.

@brawner
Copy link
Contributor Author

brawner commented Mar 16, 2020

Arg, thought I could be clever and just use vcs export

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Signed-off-by: Stephen Brawner <brawner@gmail.com>
Copy link
Collaborator

@Karsten1987 Karsten1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like the osrf testing tools are giving the windows CI a hard time. Maybe you can retrigger CI and explicitly skip that package? Should be unrelated to this change here anyway.

install(FILES sqlite3.h sqlite3ext.h DESTINATION include)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't quite see it, but what's the diff here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A newline was added at the end of the file (my editor does this automatically). Let me know if this needs to be changed back

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's all good. I was just wondering as GitHub doesn't really seem to render this change.

@brawner
Copy link
Contributor Author

brawner commented Mar 17, 2020

Looks like Dirk got the fix in for osrf_testing_tools. Trying again

Windows Build Status

@Karsten1987
Copy link
Collaborator

Does it make sense to run the windows CI job without cyclone and connext? I think these CI failures are addressed more appropriately at another place and shouldn't really block to move this PR further.

@brawner
Copy link
Contributor Author

brawner commented Mar 17, 2020

We've temporarily set buildtools back to 16.4.5. Trying on windows again.

Build Status

@brawner
Copy link
Contributor Author

brawner commented Mar 18, 2020

Building on windows finally passed. Failing test is a known issue (#305)

@Karsten1987 Karsten1987 merged commit 4bef455 into ros2:master Mar 18, 2020
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.

None yet

2 participants