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

projects: Fix Visual Studio projects SSH builds #4607

Closed
wants to merge 1 commit into from

Conversation

jay
Copy link
Member

@jay jay commented Nov 17, 2019

  • Generate VQUIC and VSSH filenames in Visual Studio project files.

Prior to this change generated Visual Studio project configurations that
enabled SSH did not build properly. Broken since SSH files were moved to
lib/vssh 3 months ago in 5b2d703.

Bug: #4492 (comment)
Reported-by: zogvm@users.noreply.github.com

Closes #xxxx

@bagder
Copy link
Member

bagder commented Nov 17, 2019

Oh right... is there any way we can add a build or two to appveyor using these?

Copy link
Contributor

@captain-caveman2k captain-caveman2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the Visual Studio project files when working out of the git repo and using Generate.bat.

The awk code in Makefile.am will also need to be updated to fix the release archive.

- Generate VQUIC and VSSH filenames in Visual Studio project files.

- Update checksrc.bat to check vquic and vssh directories.

Prior to this change generated Visual Studio project configurations that
enabled SSH did not build properly. Broken since SSH files were moved to
lib/vssh 3 months ago in 5b2d703.

Bug: curl#4492 (comment)
Reported-by: zogvm@users.noreply.github.com

Closes #xxxx
@jay jay force-pushed the fix_vs_projects_ssh_builds branch from a63eb2e to 315c650 Compare November 18, 2019 05:13
@jay
Copy link
Member Author

jay commented Nov 18, 2019

The awk code in Makefile.am will also need to be updated to fix the release archive.

Thanks, should be fixed.

is there any way we can add a build or two to appveyor using these?

We already have one 'DLL Debug - DLL Windows SSPI - DLL WinIDN'. This issue is specific to the SSH configurations, which I guess are not often used.

Steve, are the additional directories in the SSH configurations such as libssh2\build\Win32\VC10\DLL Debug intended as placeholders/examples or is there some build process that generates them there?

@bagder
Copy link
Member

bagder commented Nov 22, 2019

A related, if perhaps a bit sensitive, question here: do we still really need these files and way to build curl?

cmake can generate visual studio files that can be used to build (and we have builds in appveyor that make sure of it). Do these additional files provide additional value enough to warrant the time and effort they require for maintenance?

@MarcelRaad
Copy link
Member

MarcelRaad commented Nov 22, 2019

I think the CMake system needs a little improvement before deleting the Visual Studio projects. For example, there's currently no way to set the target Windows version and it doesn't support all feature flags. (But I also think that's something we should do.)

@jay
Copy link
Member Author

jay commented Nov 22, 2019

A related, if perhaps a bit sensitive, question here: do we still really need these files and way to build curl?

I use these files all the time in the DLL OpenSSL and DLL Windows SSPI configurations. It is easier to switch back and forth between configurations then build CMake to target a single SSL backend. I don't think I've ever built the libssh one though, there is no build script to set the directories afaik.

@captain-caveman2k
Copy link
Contributor

captain-caveman2k commented Nov 23, 2019

Me too - especially as I authored them for side by side builds using different versions of Visual Studio.

I don't mind moving them to my own repo if they are cluttering up curl.

If we keep them in curl I was contemplating moving them to the packages directory, along with the winbuild files. Thoughts?

@captain-caveman2k
Copy link
Contributor

captain-caveman2k commented Nov 23, 2019

The awk code in Makefile.am will also need to be updated to fix the release archive.

Thanks, should be fixed.

Cool - brilliant. It may be worth reworking the text of the commit message as there are changes for Quic in here.

I personally would also push the chksrc change separately rather than squashing it. The other change however I agree should be squashed.

is there any way we can add a build or two to appveyor using these?

Do any of the automated builds perform a dist - just like the overnight builds of curl? If not it may be worth adding one to double check that the Visual Studio project files get created correctly.

Steve, are the additional directories in the SSH configurations such as libssh2\build\Win32\VC10\DLL Debug intended as placeholders/examples or is there some build process that generates them there?

When I first authored these I created a bunch of libssh2 Visual Studio project files as well, to allow for the side by side builds, but I never got involved with that project to submit them back :(

I still have them somewhere although they probable only go up to VC11 or VC12.

@jay jay closed this in ee5c68a Nov 24, 2019
jay added a commit that referenced this pull request Nov 24, 2019
@jay jay deleted the fix_vs_projects_ssh_builds branch November 24, 2019 08:17
@jay
Copy link
Member Author

jay commented Nov 24, 2019

It may be worth reworking the text of the commit message as there are changes for Quic in here.

done

I personally would also push the chksrc change separately rather than squashing it. The other change however I agree should be squashed.

done

is there any way we can add a build or two to appveyor using these?

Do any of the automated builds perform a dist - just like the overnight builds of curl? If not it may be worth adding one to double check that the Visual Studio project files get created correctly.

well there's already a WinSSL build in appveyor, but if you mean you want to test project generation via autotools make -s vc-ide then no we haven't done that since it's hardly modified. I tested it with these changes though.

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Jan 14, 2020
Previously, it was only possible to set it to Windows Vista or XP by
setting the option `ENABLE_INET_PTON` to `ON` resp. `OFF`.
Use a new cache variable `CURL_TARGET_WINDOWS_VERSION` to be able to
explicitly set the target Windows version. `ENABLE_INET_PTON` is
ignored in this case.

Ref: curl#1639 (comment)
Ref: curl#4607 (comment)
Closes
MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Jan 20, 2020
Previously, it was only possible to set it to Windows Vista or XP by
setting the option `ENABLE_INET_PTON` to `ON` resp. `OFF`.
Use a new cache variable `CURL_TARGET_WINDOWS_VERSION` to be able to
explicitly set the target Windows version. `ENABLE_INET_PTON` is
ignored in this case.

Ref: curl#1639 (comment)
Ref: curl#4607 (comment)
Closes curl#4815
@lock lock bot locked as resolved and limited conversation to collaborators Feb 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

None yet

4 participants