[VTK] VTK ExprTk cmake patch #30058

ArashPartow · 2023-03-07T03:06:37Z

Changes comply with the maintainer guide
SHA512s are updated for each updated download
The "supports" clause reflects platforms that may be fixed by this new version
The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
Only one version is added to each modified port's versions file.

This patch is intended to unblock the following PR: #29665

and is based on the following VTK MR: https://gitlab.kitware.com/vtk/vtk/-/merge_requests/9981

dg0yt · 2023-03-07T06:34:18Z

Previously submitted as #29801 and #29955. Again, please don't restart fresh PRs unless really needed.

ArashPartow · 2023-03-07T09:44:33Z

@MonicaLiu0311 this PR is currently blocking #29665 and an upcoming VTK version refresh.

BillyONeal

I agree with @dg0yt , please do not close and create new PRs with no changes in content, and/or at least describe the history of how the changes got there.

It seems that the patch could be reduced to one line, removing the "2.71828"?

ports/vtk/FindExprTk.patch

BillyONeal · 2023-03-07T19:03:35Z

ports/vtk/FindExprTk.patch

+      break()
+    endif ()
+  endforeach ()
+  if (NOT ExprTk_VERSION)


ExprTk_VERSION seems to be unused; could you just remove this entirely rather than trying to fix it in a patch?

(To clarify, I don't mean patching this out, I mean just not doing this part of the patch since it seems like nobody is listening to the answer)

https://github.com/Kitware/VTK/search?q=ExprTk_VERSION

Objective is to unblock and merge the following PR: #29665 which has been in limbo for nearly 2 months.

The primary reason for this seems to be an implicit cyclic dependency that comes about due to how vcpkg has been architected.

ports/vtk/FindExprTk.patch

ArashPartow · 2023-03-07T22:59:03Z

@BillyONeal : We've gone from an all builds and green ticks to all failures post your proposed changes. As such all your proposed changes will now have to reverted as they fail when being applied to the main VTK cmake.

2023-03-07T20:26:49.9269762Z -- Applying patch FindExprTk.patch
2023-03-07T20:26:49.9512547Z CMake Error at scripts/cmake/z_vcpkg_apply_patches.cmake:34 (message):
2023-03-07T20:26:49.9518373Z   Applying patch failed: error: corrupt patch at line 11
2023-03-07T20:26:49.9521677Z 
2023-03-07T20:26:49.9527802Z Call Stack (most recent call first):
2023-03-07T20:26:49.9534153Z   scripts/cmake/vcpkg_extract_source_archive.cmake:153 (z_vcpkg_apply_patches)
2023-03-07T20:26:49.9540264Z   scripts/cmake/vcpkg_extract_source_archive_ex.cmake:8 (vcpkg_extract_source_archive)
2023-03-07T20:26:49.9546238Z   scripts/cmake/vcpkg_from_github.cmake:113 (vcpkg_extract_source_archive_ex)
2023-03-07T20:26:49.9552209Z   ports/vtk/portfile.cmake:17 (vcpkg_from_github)
2023-03-07T20:26:49.9558579Z   scripts/ports.cmake:147 (include)
2023-03-07T20:26:49.9561814Z

BillyONeal · 2023-03-15T22:41:27Z

@BillyONeal : We've gone from an all builds and green ticks to all failures post your proposed changes. As such all your proposed changes will now have to reverted as they fail when being applied to the main VTK cmake.

I was saying "these are the changes that should happen in the source"; I agree with @dg0yt that you are intending to search for . but are actually searching for "any character"

Objective is to unblock and merge the following PR: #29665 which has been in limbo for nearly 2 months.

Doing things like closing and reopening the PR as a different number makes that situation worse as you go to the back of the queue each time.

That PR being blocked for months doesn't alter our responsibilities to be extremely limited in the patches we author because we are not the owners of the code bases in question. (We do not want to create a CVE-2008-0166 / https://wiki.debian.org/SSLkeys situation)

I didn't write this patch, it was done by VTK group: https://gitlab.kitware.com/vtk/vtk/-/merge_requests/9981
I'd rather keep things as-is and minimise the difference between what is in the VTK project and in vcpkg

OK, can you download the patch ( https://gitlab.kitware.com/vtk/vtk/-/merge_requests/9981.patch) and apply it that way instead so that it's clear any problems with it are out of our hands?

ArashPartow · 2023-03-16T05:18:39Z

@BillyONeal that was the very patch I was originally applying, The same one people in other (and this one) PRs keep on preventing said PRs from being merged by requesting changes and needless commons - which I've mentioned over and over again.

ToDo:

I'm going to close this PR and start a fresh one, Once a successful builds and all green ticks, lets merge it without anymore handwaiving and dramas.

@JonLiu1993 hopefully we are in agreement. 👍

JonLiu1993 · 2023-03-16T06:30:46Z

@BillyONeal that was the very patch I was originally applying, The same one people in other (and this one) PRs keep on preventing said PRs from being merged by requesting changes and needless commons - which I've mentioned over and over again.

ToDo:

I'm going to close this PR and start a fresh one, Once a successful builds and all green ticks, lets merge it without anymore handwaiving and dramas.

@JonLiu1993 hopefully we are in agreement. 👍

@ArashPartow, Thanks your PR again, but we really don't agree to frequently close new PRs. This is not only not conducive to your PR being merged as soon as possible, but also increases the workload of CI. I hope you understand.

ArashPartow · 2023-03-16T08:31:01Z

@JonLiu1993 I'm happy to continue with this PR - though I'm not sure how it increases the CI workload as the jobs will run on this PR or on a new one so same number of clock cycles either way for every change made to the PR(s).

My only concern is that a fresh PR with only 1 comment will be quicker to review/ merge than having others go through all the comments in say this PR to only come to the conclusion that the initial commit was what was needed all along.

In any case I'll clean this PR up and hopefully we can get this merged as soon as possible as the following PR: #29665 is still waiting on this PR to be merged.

JonLiu1993 · 2023-03-28T02:21:19Z

Pinging @ArashPartow for response. Is work still being done for this PR?

JonLiu1993 · 2023-03-31T08:19:57Z

@BillyONeal @vicroms , the VTK upstream master currently contains the FindExprTk.patch changes,
But VTK has not released the latest stable version at present. I use the latest commit of VTK to update VTK on the local computer and try to upgrade the port exprtk in PR #29665 successfully.
I wonder if we now wait for VTK to release the latest stable version, or make a patch for this fix and apply it to the current version of VTK?

BillyONeal · 2023-04-14T04:52:20Z

I wonder if we now wait for VTK to release the latest stable version, or make a patch for this fix and apply it to the current version of VTK?

I would be willing to take the patch provided it is demonstrated that upstream is already likely to ship the thing updated soon but build needs to be working first of course :)

JonLiu1993 · 2023-05-06T10:21:34Z

Convert this PR to draft since there is no progress. Please ping us if this PR is ready for review again.

dan-shaw · 2023-06-07T20:25:15Z

Closing this PR as it has not been updated for the past few months

ArashPartow mentioned this pull request Mar 7, 2023

[exprtk] Update ExprTk #29665

Closed

Adela0814 assigned MonicaLiu0311 Mar 7, 2023

MonicaLiu0311 assigned JonLiu1993 Mar 7, 2023

JonLiu1993 previously approved these changes Mar 7, 2023

View reviewed changes

JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Mar 7, 2023

ArashPartow requested a review from JonLiu1993 March 7, 2023 12:03

BillyONeal requested changes Mar 7, 2023

View reviewed changes

BillyONeal reviewed Mar 7, 2023

View reviewed changes

ports/vtk/FindExprTk.patch Outdated Show resolved Hide resolved

BillyONeal reviewed Mar 7, 2023

View reviewed changes

ports/vtk/FindExprTk.patch Outdated Show resolved Hide resolved

BillyONeal reviewed Mar 7, 2023

View reviewed changes

ports/vtk/FindExprTk.patch Outdated Show resolved Hide resolved

ArashPartow dismissed JonLiu1993’s stale review via e0bd915 March 7, 2023 20:13

[VTK] VTK ExprTk cmake patch

3bfdbb8

ArashPartow requested review from BillyONeal and removed request for JonLiu1993 March 7, 2023 20:23

dan-shaw added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Mar 7, 2023

ArashPartow requested review from JonLiu1993 and removed request for BillyONeal March 7, 2023 23:00

ArashPartow mentioned this pull request Mar 7, 2023

Assistance required in getting PRs merged into vcpkg #30078

Closed

PhoebeHui assigned jimwang118 Mar 30, 2023

JonLiu1993 added 5 commits March 30, 2023 17:08

fix conflict

a79bf37

update version

fc8de5d

update version

edacb68

fix conflict

ffd6706

update version

b52f75f

JonLiu1993 removed the requires:author-response label Mar 31, 2023

JonLiu1993 added 5 commits March 31, 2023 18:46

update patch

0667dae

update version

4ddf902

fix conflict

28ba914

fix conflict

e8eb383

update version

b235715

JonLiu1993 marked this pull request as draft May 6, 2023 10:21

dan-shaw closed this Jun 7, 2023

This was referenced Dec 30, 2024

[exprtk] Add versions 0.0.2 #42996

Closed

[exprtk] Add version 0.0.3 2024-01-01 #43036

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VTK] VTK ExprTk cmake patch #30058

[VTK] VTK ExprTk cmake patch #30058

ArashPartow commented Mar 7, 2023

dg0yt commented Mar 7, 2023 •

edited

Loading

ArashPartow commented Mar 7, 2023 •

edited

Loading

BillyONeal left a comment

BillyONeal Mar 7, 2023

BillyONeal Mar 7, 2023

BillyONeal Mar 7, 2023

ArashPartow Mar 7, 2023 •

edited

Loading

ArashPartow commented Mar 7, 2023 •

edited

Loading

BillyONeal commented Mar 15, 2023

ArashPartow commented Mar 16, 2023 •

edited

Loading

JonLiu1993 commented Mar 16, 2023

ArashPartow commented Mar 16, 2023 •

edited

Loading

JonLiu1993 commented Mar 28, 2023

JonLiu1993 commented Mar 31, 2023

BillyONeal commented Apr 14, 2023

JonLiu1993 commented May 6, 2023

dan-shaw commented Jun 7, 2023

[VTK] VTK ExprTk cmake patch #30058

[VTK] VTK ExprTk cmake patch #30058

Conversation

ArashPartow commented Mar 7, 2023

dg0yt commented Mar 7, 2023 • edited Loading

ArashPartow commented Mar 7, 2023 • edited Loading

BillyONeal left a comment

Choose a reason for hiding this comment

BillyONeal Mar 7, 2023

Choose a reason for hiding this comment

BillyONeal Mar 7, 2023

Choose a reason for hiding this comment

BillyONeal Mar 7, 2023

Choose a reason for hiding this comment

ArashPartow Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

ArashPartow commented Mar 7, 2023 • edited Loading

BillyONeal commented Mar 15, 2023

ArashPartow commented Mar 16, 2023 • edited Loading

JonLiu1993 commented Mar 16, 2023

ArashPartow commented Mar 16, 2023 • edited Loading

JonLiu1993 commented Mar 28, 2023

JonLiu1993 commented Mar 31, 2023

BillyONeal commented Apr 14, 2023

JonLiu1993 commented May 6, 2023

dan-shaw commented Jun 7, 2023

dg0yt commented Mar 7, 2023 •

edited

Loading

ArashPartow commented Mar 7, 2023 •

edited

Loading

ArashPartow Mar 7, 2023 •

edited

Loading

ArashPartow commented Mar 7, 2023 •

edited

Loading

ArashPartow commented Mar 16, 2023 •

edited

Loading

ArashPartow commented Mar 16, 2023 •

edited

Loading