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

Support for Tcl TIP477 nmake-based builds #15

Merged
merged 2 commits into from Mar 5, 2021
Merged

Support for Tcl TIP477 nmake-based builds #15

merged 2 commits into from Mar 5, 2021

Conversation

apnadkarni
Copy link
Contributor

The changes allow building with Visual Studio and nmake as described in TIP 477

One point to note - the nmake makefile currently sets the tclcurl package version number to the same as the curl version against which it is built. It is not clear this is the right thing to do. On the other hand, not sure what the right thing to is as tclcurl versions seem tied to curl versions.

Question W.r.t. tests - the tests directory has many sample .tcl files. Are tests using these automated ? (The all.tcl passes but does not seem to make use of the example tcl files)

I am also curious as to the relation of this repository with the one from jdc.

@bovine
Copy link
Member

bovine commented Feb 22, 2021

it looks like the jdc repo was forked from some other source in Jan 2014.

this repo was forked from one by Steve Havelka https://bitbucket.org/smh377/tclcurl/, who started his in Feb 2014.

both of those repos were ultimately from work originally created by Andres Garcia (fandom@telefonica.net).

@bovine
Copy link
Member

bovine commented Feb 22, 2021

@apnadkarni do you think you are able to include a GitHub Workflow action to build this under Windows?

@apnadkarni
Copy link
Contributor Author

I have not used workflow actions before, will look into it and put it on my to-do list.

@bovine
Copy link
Member

bovine commented Feb 23, 2021

Here is an example of a Windows workflow for another of our projects (though it uses cmake instead of nmake): https://github.com/flightaware/mdsplib/blob/master/.github/workflows/windows-ci.yml

You will likely need to figure out how to download, extract, and install the headers/libs for curl itself.

There are some good docs at https://docs.github.com/en/actions

@bovine bovine merged commit 4e79a99 into flightaware:master Mar 5, 2021
@bovine
Copy link
Member

bovine commented Mar 5, 2021

I've merged this, but it would still be great to get CI builds for Windows happening eventually.

@apnadkarni
Copy link
Contributor Author

Sorry about the CI builds. It is still on my list but between work and other projects in progress...

@bovine
Copy link
Member

bovine commented Mar 8, 2021

@apnadkarni I think I have a build file for Windows here in this fork: https://github.com/bovine/tclcurl-fa/blob/master/.github/workflows/windows-ci.yml

However, it is failing with:
rules-ext.vc(115) : fatal error U1050: *** Could not locate rules.vc in .
https://github.com/bovine/tclcurl-fa/runs/2053790869?check_suite_focus=true

(note that it is using Tcl 9 instead of Tcl 8.6 because that is apparently the only version supported by vcpkg? https://github.com/microsoft/vcpkg/tree/master/ports/tcl)

@apnadkarni
Copy link
Contributor Author

A little surprised that the tcl in vcpkg is 9.0 given that is far from actually releasing. But I don't think that is the problem. I suspect the issue is similar to what the port to Tcl describes in microsoft/vcpkg#12987 (comment). That is for some reason nmake returns false under vcpkg when checking for the existence of a file.

The makefile for tclcurl-fa checks for rules.vc in the Tcl installation directory and barfs because it is not able to locate it there. I suspect this is due to the nmake issue above although it is possible the tcl vcpkg moves those files (rules.vc etc.) to some other location.

I'm still a ways off from digging into this further. If it is a high priority for you, please let me know accordingly since you have already merged.

@apnadkarni
Copy link
Contributor Author

Instead of using the vcpkg Tcl, would it be OK to download and build Tcl as part of the action? I'm going to try work on it this week.

@bovine
Copy link
Member

bovine commented Mar 8, 2021

It would be fine to use something other than vcpkg, but I just thought that it might be the easiest way forward. Perhaps it might be worthwhile to even submit a PR to vcpkg to add tcl86 support, or resolve whatever problems it is having?

This is not an urgent issue, but I'd like to ensure it is not forgotten. Figuring out the best/easiest way to do Windows CI of Tcl packages would allow us to add Windows CI to our other Tcl repos.

@apnadkarni
Copy link
Contributor Author

apnadkarni commented Mar 12, 2021

I now have a "almost working" windows-ci.yml. It builds but two tests fail because of version checks - the vcpkg curl shows its version as "7.74.0-DEV" while the tests expect a pure numeric x.y.z version and the "-DEV" causes them to fail. Choices for fixing are to permit the non-numeric in the test, do our own curl build or only do build, not test as part of the CI.

What's your inclination?

PS https://github.com/apnadkarni/tclcurl-fa/actions

@bovine
Copy link
Member

bovine commented Mar 12, 2021

Your branch looks pretty good.

It's strange that vcpkg is installing that "7.74.0-DEV" version of curl rather than a stable version.

Go ahead and relax the version checks to allow non-numeric suffixes, and submit a PR back to this repo. Thanks!

@apnadkarni
Copy link
Contributor Author

Have added a separate pull request for the workflow.

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

2 participants