You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "martinzink (via GitHub)" <gi...@apache.org> on 2026/03/25 17:25:35 UTC
[PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
martinzink opened a new pull request, #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2989957571
##########
.github/workflows/create-release-artifacts.yml:
##########
@@ -40,3 +40,65 @@ jobs:
with:
name: ${{ matrix.platform.rpm-artifact }}
path: build/nifi-minifi-cpp-*.rpm
+ windows_VS2022:
+ name: "Windows Server 2025 x86_64"
+ runs-on: windows-2025
+ timeout-minutes: 240
+ steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
+ - name: Checkout project
+ uses: actions/checkout@v4
+ - name: Set up Python
+ run: choco -y install python & refreshenv
+ shell: cmd
+ - name: Install sqliteodbc driver
+ run: |
+ Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
+ if ((Get-FileHash 'sqliteodbc_w64.exe').Hash -ne "a4804e4f54f42c721df1323c5fcac101a8c7a577e7f20979227324ceab572d51") {Write "Hash mismatch"; Exit 1}
Review Comment:
out of the scope of this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2989836420
##########
extensions/sftp/tests/tools/SFTPTestServer.cpp:
##########
@@ -138,10 +138,10 @@ bool SFTPTestServer::stop() {
logger_->log_debug("Deleting port file {}", port_file_path_);
::unlink(port_file_path_.c_str());
}
-#endif
server_pid_ = -1;
Review Comment:
it failed to compiled because server_pid_ is not declared on windows, since we already throw I've moved the endif a bit
##########
extensions/sftp/tests/tools/SFTPTestServer.cpp:
##########
@@ -138,10 +138,10 @@ bool SFTPTestServer::stop() {
logger_->log_debug("Deleting port file {}", port_file_path_);
::unlink(port_file_path_.c_str());
}
-#endif
server_pid_ = -1;
Review Comment:
this is the bugfix part
it failed to compiled because server_pid_ is not declared on windows, since we already throw I've moved the endif a bit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2999188687
##########
.github/workflows/create-release-artifacts.yml:
##########
@@ -40,3 +40,65 @@ jobs:
with:
name: ${{ matrix.platform.rpm-artifact }}
path: build/nifi-minifi-cpp-*.rpm
+ windows_VS2022:
+ name: "Windows Server 2025 x86_64"
+ runs-on: windows-2025
+ timeout-minutes: 240
+ steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
+ - name: Checkout project
+ uses: actions/checkout@v4
+ - name: Set up Python
+ run: choco -y install python & refreshenv
+ shell: cmd
+ - name: Install sqliteodbc driver
+ run: |
+ Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
+ if ((Get-FileHash 'sqliteodbc_w64.exe').Hash -ne "a4804e4f54f42c721df1323c5fcac101a8c7a577e7f20979227324ceab572d51") {Write "Hash mismatch"; Exit 1}
+ Start-Process -FilePath ".\sqliteodbc_w64.exe" -ArgumentList "/S" -Wait
+ shell: powershell
+ - name: build
+ run: |
+ python -m venv venv && venv\Scripts\activate && pip install -r requirements.txt & python main.py --noninteractive --skip-compiler-install --minifi-options="-DCMAKE_BUILD_TYPE=Release -DCI_BUILD=OFF -DENABLE_ALL=ON -DMINIFI_FAIL_ON_WARNINGS=OFF -DSKIP_TESTS=ON"
Review Comment:
good idea, https://github.com/apache/nifi-minifi-cpp/pull/2145/commits/4918d5bef72d9dd2c89725ddf164a0216bd8e2cb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2993922116
##########
extensions/sftp/tests/tools/SFTPTestServer.cpp:
##########
@@ -138,10 +138,10 @@ bool SFTPTestServer::stop() {
logger_->log_debug("Deleting port file {}", port_file_path_);
::unlink(port_file_path_.c_str());
}
-#endif
server_pid_ = -1;
Review Comment:
It still has compile issues. I've checked and we dont enable SFTP on windows CI job, and locally it skips the tests if java or maven is missing, thats why it could fly under our radar.
Since this would require a bigger effort, this is not the PR to fix these I've created a new jira for this effort, and disabled tests for this job (we dont run tests anyway in these jobs so it makes sense to skip them here)
https://issues.apache.org/jira/browse/MINIFICPP-2759
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits closed pull request #2145: MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2994630587
##########
behave_framework/pyproject.toml:
##########
@@ -15,4 +15,7 @@ dependencies = [
[tool.setuptools]
package-dir = {"" = "src"}
-packages = ["minifi_test_framework"]
+
+[tool.setuptools.packages.find]
+where = ["src"]
+include = ["minifi_test_framework*"]
Review Comment:
this will conflict (in a way not detected by git) with #2146
##########
.github/workflows/create-release-artifacts.yml:
##########
@@ -40,3 +40,65 @@ jobs:
with:
name: ${{ matrix.platform.rpm-artifact }}
path: build/nifi-minifi-cpp-*.rpm
+ windows_VS2022:
+ name: "Windows Server 2025 x86_64"
+ runs-on: windows-2025
+ timeout-minutes: 240
+ steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
+ - name: Checkout project
+ uses: actions/checkout@v4
+ - name: Set up Python
+ run: choco -y install python & refreshenv
+ shell: cmd
+ - name: Install sqliteodbc driver
+ run: |
+ Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
+ if ((Get-FileHash 'sqliteodbc_w64.exe').Hash -ne "a4804e4f54f42c721df1323c5fcac101a8c7a577e7f20979227324ceab572d51") {Write "Hash mismatch"; Exit 1}
+ Start-Process -FilePath ".\sqliteodbc_w64.exe" -ArgumentList "/S" -Wait
+ shell: powershell
+ - name: build
+ run: |
+ python -m venv venv && venv\Scripts\activate && pip install -r requirements.txt & python main.py --noninteractive --skip-compiler-install --minifi-options="-DCMAKE_BUILD_TYPE=Release -DCI_BUILD=OFF -DENABLE_ALL=ON -DMINIFI_FAIL_ON_WARNINGS=OFF -DSKIP_TESTS=ON"
Review Comment:
Why is the third command separator a &? If & is non-shortcircuiting, I would use && there, too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2993922116
##########
extensions/sftp/tests/tools/SFTPTestServer.cpp:
##########
@@ -138,10 +138,10 @@ bool SFTPTestServer::stop() {
logger_->log_debug("Deleting port file {}", port_file_path_);
::unlink(port_file_path_.c_str());
}
-#endif
server_pid_ = -1;
Review Comment:
It still has compile issues. I've checked and we dont enable SFTP on windows CI job, and locally it skips the tests if java or maven is missing, thats why it could fly under our radar.
Since this would require a bigger effort, this is not the PR to fix these I've created a new jira for this effort.
https://issues.apache.org/jira/browse/MINIFICPP-2759
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2989879668
##########
.github/workflows/create-release-artifacts.yml:
##########
@@ -40,3 +40,65 @@ jobs:
with:
name: ${{ matrix.platform.rpm-artifact }}
path: build/nifi-minifi-cpp-*.rpm
+ windows_VS2022:
+ name: "Windows Server 2025 x86_64"
+ runs-on: windows-2025
+ timeout-minutes: 240
+ steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
+ - name: Checkout project
+ uses: actions/checkout@v4
+ - name: Set up Python
+ run: choco -y install python & refreshenv
+ shell: cmd
+ - name: Install sqliteodbc driver
+ run: |
+ Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
Review Comment:
The driver installer is downloaded over plain HTTP, which is vulnerable to MITM. Even with a hash check, using HTTPS materially improves supply-chain security and reduces exposure to interception/tampering attempts. Switch the download URL to HTTPS (or a trusted HTTPS mirror) if available.
```suggestion
Invoke-WebRequest -Uri "https://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
```
##########
.github/workflows/create-release-artifacts.yml:
##########
@@ -40,3 +40,65 @@ jobs:
with:
name: ${{ matrix.platform.rpm-artifact }}
path: build/nifi-minifi-cpp-*.rpm
+ windows_VS2022:
+ name: "Windows Server 2025 x86_64"
+ runs-on: windows-2025
+ timeout-minutes: 240
+ steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
+ - name: Checkout project
+ uses: actions/checkout@v4
+ - name: Set up Python
+ run: choco -y install python & refreshenv
+ shell: cmd
+ - name: Install sqliteodbc driver
+ run: |
+ Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
+ if ((Get-FileHash 'sqliteodbc_w64.exe').Hash -ne "a4804e4f54f42c721df1323c5fcac101a8c7a577e7f20979227324ceab572d51") {Write "Hash mismatch"; Exit 1}
+ Start-Process -FilePath ".\sqliteodbc_w64.exe" -ArgumentList "/S" -Wait
+ shell: powershell
+ - name: build
+ run: |
+ python -m venv venv & venv\Scripts\activate & pip install -r requirements.txt & python main.py --noninteractive --skip-compiler-install --minifi-options="-DCMAKE_BUILD_TYPE=Release -DCI_BUILD=OFF -DENABLE_ALL=ON -DMINIFI_FAIL_ON_WARNINGS=OFF"
Review Comment:
On Windows `cmd`, activating a venv from within a batch context typically requires calling `activate.bat` with `call`; otherwise control flow/environment updates can be unreliable and subsequent `pip`/`python` commands may run outside the venv. Update the command to explicitly `call venv\\Scripts\\activate.bat` (or avoid activation by invoking `venv\\Scripts\\python -m pip ...` and `venv\\Scripts\\python main.py ...`).
```suggestion
python -m venv venv & call venv\Scripts\activate.bat & pip install -r requirements.txt & python main.py --noninteractive --skip-compiler-install --minifi-options="-DCMAKE_BUILD_TYPE=Release -DCI_BUILD=OFF -DENABLE_ALL=ON -DMINIFI_FAIL_ON_WARNINGS=OFF"
```
##########
.github/workflows/create-release-artifacts.yml:
##########
@@ -40,3 +40,65 @@ jobs:
with:
name: ${{ matrix.platform.rpm-artifact }}
path: build/nifi-minifi-cpp-*.rpm
+ windows_VS2022:
+ name: "Windows Server 2025 x86_64"
+ runs-on: windows-2025
+ timeout-minutes: 240
+ steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
+ - name: Checkout project
+ uses: actions/checkout@v4
+ - name: Set up Python
+ run: choco -y install python & refreshenv
+ shell: cmd
+ - name: Install sqliteodbc driver
+ run: |
+ Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
+ if ((Get-FileHash 'sqliteodbc_w64.exe').Hash -ne "a4804e4f54f42c721df1323c5fcac101a8c7a577e7f20979227324ceab572d51") {Write "Hash mismatch"; Exit 1}
Review Comment:
`Write` is not a standard PowerShell cmdlet; this line can fail before exiting, which would obscure the actual hash mismatch error. Use a valid cmdlet (e.g., `Write-Error` or `Write-Host`) so the failure path is reliable and the logs clearly show the mismatch reason.
```suggestion
if ((Get-FileHash 'sqliteodbc_w64.exe').Hash -ne "a4804e4f54f42c721df1323c5fcac101a8c7a577e7f20979227324ceab572d51") { Write-Error "Hash mismatch"; Exit 1 }
```
##########
extensions/sftp/tests/tools/SFTPTestServer.cpp:
##########
@@ -138,10 +138,10 @@ bool SFTPTestServer::stop() {
logger_->log_debug("Deleting port file {}", port_file_path_);
::unlink(port_file_path_.c_str());
}
-#endif
server_pid_ = -1;
started_ = false;
port_file_path_ = "";
+#endif
Review Comment:
Moving `#endif` down causes `server_pid_`, `started_`, and `port_file_path_` resets to be compiled only when the corresponding `#if` condition is true. That changes `stop()` behavior and can leave the object in an inconsistent state on builds where the guard is false. Keep the `#endif` immediately after the platform-specific block (e.g., only guard the `unlink` portion), so the state reset always executes.
```suggestion
#endif
started_ = false;
port_file_path_ = "";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2989959060
##########
.github/workflows/create-release-artifacts.yml:
##########
@@ -40,3 +40,65 @@ jobs:
with:
name: ${{ matrix.platform.rpm-artifact }}
path: build/nifi-minifi-cpp-*.rpm
+ windows_VS2022:
+ name: "Windows Server 2025 x86_64"
+ runs-on: windows-2025
+ timeout-minutes: 240
+ steps:
+ - name: Support longpaths
+ run: git config --system core.longpaths true
+ - name: Checkout project
+ uses: actions/checkout@v4
+ - name: Set up Python
+ run: choco -y install python & refreshenv
+ shell: cmd
+ - name: Install sqliteodbc driver
+ run: |
+ Invoke-WebRequest -Uri "http://www.ch-werner.de/sqliteodbc/sqliteodbc_w64.exe" -OutFile "sqliteodbc_w64.exe"
Review Comment:
we should probably use choco to install this, but still out of the scope of this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2757 Build windows artifacts in create-release-artifacts CI [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2145:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2145#discussion_r2999197585
##########
behave_framework/pyproject.toml:
##########
@@ -15,4 +15,7 @@ dependencies = [
[tool.setuptools]
package-dir = {"" = "src"}
-packages = ["minifi_test_framework"]
+
+[tool.setuptools.packages.find]
+where = ["src"]
+include = ["minifi_test_framework*"]
Review Comment:
good eye, this will probably get merged first (since #2146 is built upon #2133 and that's needs more reviews) so I'll fix it there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org