Skip to content

Make ament_cmake_python symlink for symlink installs only#357

Merged
hidmic merged 1 commit intomasterfrom
hidmic/symlink-on-symlink-installs-only
Oct 13, 2021
Merged

Make ament_cmake_python symlink for symlink installs only#357
hidmic merged 1 commit intomasterfrom
hidmic/symlink-on-symlink-installs-only

Conversation

@hidmic
Copy link
Copy Markdown

@hidmic hidmic commented Oct 7, 2021

Precisely what the title says. Otherwise, #350 hits our Windows users.

@hidmic
Make ament_cmake_python symlink for symlink installs only
7462a12 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Copy Markdown
Author

hidmic commented Oct 7, 2021 •
edited
Loading

@gavanderhoorn give this one a shot and let me know.

gavanderhoorn
gavanderhoorn approved these changes Oct 7, 2021
View reviewed changes
Copy link
Copy Markdown

@gavanderhoorn gavanderhoorn left a comment

Choose a reason for hiding this comment

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

Seems to have fixed it.

Tested on Win10 + Python 3.8.x.

@gavanderhoorn
Copy link
Copy Markdown

gavanderhoorn commented Oct 7, 2021

I'll test it some more, but it seems like this has resolved the immediate issue.

Thanks for the fast turn-around @hidmic 👍

@gavanderhoorn
Copy link
Copy Markdown

gavanderhoorn commented Oct 7, 2021

Will this be backported to galactic? Seeing as it fixes something which seems like a regression?

@hidmic
Copy link
Copy Markdown
Author

hidmic commented Oct 7, 2021

Full CI:

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

@hidmic
Copy link
Copy Markdown
Author

hidmic commented Oct 7, 2021

Will this be backported to galactic? Seeing as it fixes something which seems like a regression?

I'm onboard with that. @cottsay ?

@gavanderhoorn
Copy link
Copy Markdown

gavanderhoorn commented Oct 12, 2021

Friendly ping?

@hidmic
Copy link
Copy Markdown
Author

hidmic commented Oct 13, 2021

I was out for a few days @gavanderhoorn. Sorry for the delay.

Hmm, those rclpy test failures on Windows look suspicious.

Running Windows CI up to rclpy: Build Status

@gavanderhoorn
Copy link
Copy Markdown

gavanderhoorn commented Oct 13, 2021

might be flaky tests?

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Oct 13, 2021

might be flaky tests?

They were fixed elsewhere in the stack in the meantime.

@hidmic
Copy link
Copy Markdown
Author

hidmic commented Oct 13, 2021

Alright then. Going in!

@hidmic hidmic merged commit f69df01 into master Oct 13, 2021
@delete-merged-branch delete-merged-branch bot deleted the hidmic/symlink-on-symlink-installs-only branch October 13, 2021 18:16
@gavanderhoorn
Copy link
Copy Markdown

gavanderhoorn commented Oct 13, 2021

Thanks for fixing this @hidmic 👍

@gavanderhoorn
Copy link
Copy Markdown

gavanderhoorn commented Oct 13, 2021

Oh. And the backport to Galactic?

@hidmic
Copy link
Copy Markdown
Author

hidmic commented Oct 13, 2021 •
edited
Loading

Oh. And the backport to Galactic?

You can cherry pick the commit and open a backport PR. Otherwise, I'll try to do it later today.

Edit: I wonder if we can fast-forward.

gavanderhoorn pushed a commit to Yaskawa-Global/ament_cmake that referenced this pull request Oct 13, 2021
@hidmic @gavanderhoorn
Make ament_cmake_python symlink for symlink installs only (ament#357)
74435f8 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@gavanderhoorn
Copy link
Copy Markdown

gavanderhoorn commented Oct 13, 2021

I can create the PR (I believe).

Not sure whether ff would work. Are all those commits supposed to end up on galactic as well?

@gavanderhoorn gavanderhoorn mentioned this pull request Oct 13, 2021
Merged
hidmic pushed a commit that referenced this pull request Oct 20, 2021
@gavanderhoorn
Make ament_cmake_python symlink for symlink installs only (#357) (#359)
a803f85 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@gavanderhoorn gavanderhoorn gavanderhoorn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hidmic @gavanderhoorn @clalancette