[Openvpn-devel] GHA: Add work-around for rst2*.py not being directly executable on Windows

Message ID 20230706100407.231175-1-frank@lichtenheld.com
State Rejected
Headers show
Series [Openvpn-devel] GHA: Add work-around for rst2*.py not being directly executable on Windows | expand

Commit Message

Frank Lichtenheld July 6, 2023, 10:04 a.m. UTC
We write a small .bat file wrapper and then force CMake
to use that.

Note that we need to specify the path with / instead of \
as path separator because otherwise run-cmake action will
mangle it.

Change-Id: I05d3f7f3f7f7418d1977e523c6dcfb6fa5feb604
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
 .github/workflows/build.yaml | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

This is an alternate patch to "CMake: Support doc builds on
Windows machines that do not have .py file association" since
there was some doubt on IRC whether we want to have the complexity
inside of CMake or should delegate that to the caller.

Let me know your opinions.

Comments

Frank Lichtenheld July 6, 2023, 10:21 a.m. UTC | #1
On Thu, Jul 06, 2023 at 12:04:07PM +0200, Frank Lichtenheld wrote:
> We write a small .bat file wrapper and then force CMake
> to use that.
> 
> Note that we need to specify the path with / instead of \
> as path separator because otherwise run-cmake action will
> mangle it.
> 
> Change-Id: I05d3f7f3f7f7418d1977e523c6dcfb6fa5feb604
> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
> ---
>  .github/workflows/build.yaml | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> This is an alternate patch to "CMake: Support doc builds on
> Windows machines that do not have .py file association" since
> there was some doubt on IRC whether we want to have the complexity
> inside of CMake or should delegate that to the caller.
> 
> Let me know your opinions.

Personally I prefer the original solution inside CMake.
It just solves the problem in a wide range of potential setups
without additional effort required by the caller. I think the
slightly increased complexity can be accepted for that.

Regards,
Arne Schwabe July 6, 2023, 10:45 a.m. UTC | #2
Am 06.07.23 um 12:21 schrieb Frank Lichtenheld:
> On Thu, Jul 06, 2023 at 12:04:07PM +0200, Frank Lichtenheld wrote:
>> We write a small .bat file wrapper and then force CMake
>> to use that.
>>
>> Note that we need to specify the path with / instead of \
>> as path separator because otherwise run-cmake action will
>> mangle it.
>>
>> Change-Id: I05d3f7f3f7f7418d1977e523c6dcfb6fa5feb604
>> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
>> ---
>>   .github/workflows/build.yaml | 21 ++++++++++++++++++++-
>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> This is an alternate patch to "CMake: Support doc builds on
>> Windows machines that do not have .py file association" since
>> there was some doubt on IRC whether we want to have the complexity
>> inside of CMake or should delegate that to the caller.
>>
>> Let me know your opinions.
> 
> Personally I prefer the original solution inside CMake.
> It just solves the problem in a wide range of potential setups
> without additional effort required by the caller. I think the
> slightly increased complexity can be accepted for that.

I concur for the same reasons. This is also harder for people to 
replicate on their own setups. They will have to manually write scripts 
instead of jsut having cmake dealing with the problem.

Arne
Lev Stipakov July 6, 2023, 10:57 a.m. UTC | #3
I had this problem on both arm64 and x64 machines and CMake patch
fixed it. So I'd also go with that.

to 6. heinäk. 2023 klo 13.46 Arne Schwabe (arne@rfc2549.org) kirjoitti:
>
> Am 06.07.23 um 12:21 schrieb Frank Lichtenheld:
> > On Thu, Jul 06, 2023 at 12:04:07PM +0200, Frank Lichtenheld wrote:
> >> We write a small .bat file wrapper and then force CMake
> >> to use that.
> >>
> >> Note that we need to specify the path with / instead of \
> >> as path separator because otherwise run-cmake action will
> >> mangle it.
> >>
> >> Change-Id: I05d3f7f3f7f7418d1977e523c6dcfb6fa5feb604
> >> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
> >> ---
> >>   .github/workflows/build.yaml | 21 ++++++++++++++++++++-
> >>   1 file changed, 20 insertions(+), 1 deletion(-)
> >>
> >> This is an alternate patch to "CMake: Support doc builds on
> >> Windows machines that do not have .py file association" since
> >> there was some doubt on IRC whether we want to have the complexity
> >> inside of CMake or should delegate that to the caller.
> >>
> >> Let me know your opinions.
> >
> > Personally I prefer the original solution inside CMake.
> > It just solves the problem in a wide range of potential setups
> > without additional effort required by the caller. I think the
> > slightly increased complexity can be accepted for that.
>
> I concur for the same reasons. This is also harder for people to
> replicate on their own setups. They will have to manually write scripts
> instead of jsut having cmake dealing with the problem.
>
> Arne
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Patch

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index da2a427c..72edb089 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -286,8 +286,25 @@  jobs:
       - uses: actions/checkout@v3
       - uses: lukka/get-cmake@latest
 
+      # since we can't execute .py files directly, do a workaround for rst2html,rst2man
+      - name: Determine Python script path
+        run: |
+          import os
+          import sysconfig
+          scripts_path = sysconfig.get_path("scripts",f"{os.name}")
+          with open(os.environ["GITHUB_ENV"], "a") as env:
+            print(f"PYTHON_SCRIPTS_DIR={scripts_path}", file=env)
+        shell: python
       - name: Install rst2html
-        run: python -m pip install --upgrade pip docutils
+        run: |
+          python -m pip install --upgrade pip docutils
+          $install_dir = "${{ runner.workspace }}\docutils"
+          mkdir $install_dir
+          echo "python $Env:PYTHON_SCRIPTS_DIR\rst2html.py %*" > $install_dir\rst2html.bat
+          cat $install_dir\rst2html.bat
+          echo "python $Env:PYTHON_SCRIPTS_DIR\rst2man.py %*" > $install_dir\rst2man.bat
+          $safe_path = $install_dir.replace("\", "/")
+          echo "DOCUTILS=$safe_path" >> "$Env:GITHUB_ENV"
 
       - name: Restore artifacts, or setup vcpkg (do not install any package)
         uses: lukka/run-vcpkg@v11
@@ -300,6 +317,7 @@  jobs:
         if: ${{ matrix.arch == 'arm64' }}
         with:
           configurePreset: win-${{ matrix.arch }}-release
+          configurePresetAdditionalArgs: "[`-DRST2HTML=${{ env.DOCUTILS }}/rst2html.bat`, `-DRST2MAN=${{ env.DOCUTILS }}/rst2man.bat`]"
           buildPreset: win-${{ matrix.arch }}-release
 
       - name: Run CMake with vcpkg.json manifest
@@ -307,6 +325,7 @@  jobs:
         if: ${{ matrix.arch != 'arm64' }}
         with:
           configurePreset: win-${{ matrix.arch }}-release
+          configurePresetAdditionalArgs: "[`-DRST2HTML=${{ env.DOCUTILS }}/rst2html.bat`, `-DRST2MAN=${{ env.DOCUTILS }}/rst2man.bat`]"
           buildPreset: win-${{ matrix.arch }}-release
           testPreset: win-${{ matrix.arch }}-release