Skip to content

Conversation

mathomp4
Copy link
Collaborator

This is an attempt to do what @melven did in #259 but in a bit stricter sense.

We make both awk and m4 REQUIRED via find_program and rely on users to install in their base environment.

Will keep draft until @melven can test.

@mathomp4 mathomp4 added the enhancement New feature or request label Apr 21, 2025
@mathomp4 mathomp4 self-assigned this Apr 21, 2025
@mathomp4 mathomp4 mentioned this pull request Apr 21, 2025
@melven
Copy link

melven commented Apr 30, 2025

Works mostly fine:

  • shows errors when m4 or gawk are not available
  • finds gawk and m4 if they are in the PATH on windows

Tests don't work due to escaping console commands on Windows for gawk:

The following works on Windows:

    COMMAND ${AWK} "\"/<start>/,/<stop>/\"" ${tmp} > ${tmp2}
    COMMAND ${AWK} "\"NR>2 {print last} {last=$$0}\"" ${tmp2} > ${actual}

but seems to make problems on Linux where the (current) variant should work:

    COMMAND ${AWK} '/<start>/,/<stop>/' ${tmp} > ${tmp2}
    COMMAND ${AWK} 'NR>2 {print last} {last=$$0}' ${tmp2} > ${actual}

@mathomp4
Copy link
Collaborator Author

mathomp4 commented May 1, 2025

@melven I just pushed a change that I hope makes it generic.

Can you re-pull this branch and try again?

@mathomp4
Copy link
Collaborator Author

mathomp4 commented May 1, 2025

Whoops. I was too clever. Hold please...

@mathomp4
Copy link
Collaborator Author

mathomp4 commented May 1, 2025

Okay. Now it doesn't crash on all our tests. @melven can you test the Windows side?

@melven
Copy link

melven commented May 2, 2025

Okay. Now it doesn't crash on all our tests. @melven can you test the Windows side?

Works fine now on Windows in my setup. Thanks!

@mathomp4 mathomp4 marked this pull request as ready for review May 2, 2025 13:34
@mathomp4 mathomp4 requested a review from tclune May 2, 2025 13:35
@tclune tclune merged commit ef9eca1 into feature/update-cmake-min May 2, 2025
16 of 17 checks passed
@tclune tclune deleted the feature/no-download-win branch May 2, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants