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

CMake: correct non-POSIX friendly 'sed-i' arg in FindPySide2Tools.cmake #4592

Merged
merged 1 commit into from Mar 8, 2021

Conversation

luzpaz
Copy link
Contributor

@luzpaz luzpaz commented Mar 6, 2021

@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 7, 2021

@chennes got a moment to review?

Copy link
Member

@chennes chennes left a comment

Choose a reason for hiding this comment

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

The change looks fine, and in principle the two sed commands are the same -- I can't test it in a real application because I don't know where this macro is called from.

I personally didn't know that the -i option was non-POSIX, so you might consider tweaking the comment above this where it says "in-place" to specifically note that you aren't using -i for that reason.

resolves FreeCAD#4588  
ticket: https://tracker.freecadweb.org/view.php?id=4588  
Patch provided by 'garya'  
Tweaked comment to reflect the patch change. [skip ci]
@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 7, 2021

Thanks @chennes I tweaked the comment per your advice. Good catch. I added a [skip ci] the commit message to avoid an unnecessary compile due to a comment change.

@wwmayer this should be ready for merge.

@luzpaz
Copy link
Contributor Author

luzpaz commented Mar 8, 2021

Note: this PR was a patch that was submitted from the FreeBSD folks to make their already complex packaging tasks easier 😉

@wwmayer
Copy link
Contributor

wwmayer commented Mar 8, 2021

Here some background information:
The sed command has been introduced with 2b9da83#diff-70c718cc8ed63da585359f36123ba88fb61574e5d4531161d76d954f94434f2b where the openSuse maintainers wanted to avoid changing checksums because of the build date of the generated file.

With a later change the execute_command function was replaced with a add_custom_command that uses the -i option: 75c698d#diff-70c718cc8ed63da585359f36123ba88fb61574e5d4531161d76d954f94434f2b

@wwmayer wwmayer merged commit 971bb66 into FreeCAD:master Mar 8, 2021
@luzpaz luzpaz deleted the Ticket-4588 branch March 8, 2021 16:32
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

3 participants