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

Update curl version list #104

Merged
merged 9 commits into from
Nov 5, 2020
Merged

Update curl version list #104

merged 9 commits into from
Nov 5, 2020

Conversation

SirLynix
Copy link
Member

@SirLynix SirLynix commented Nov 5, 2020

See #101

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

Merged with base branch.

I'm wondering, should we disable test building? It doesn't seem really useful here (and is causing compilation issues with msvc and libcurl 7.64.0 because of an empty file)

@waruqi waruqi changed the base branch from master to dev November 5, 2020 10:20
@waruqi
Copy link
Member

waruqi commented Nov 5, 2020

If it is a test build of libcurl itself, as long as cmakelist provides an option to disable them, we usually agree to disable them

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

Yes it is, I'll make a separate PR for this.

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

Following #101

I made the changes, except for two things:

  • I used includes directly instead of declaring a function to prevent declaring a global add_versions_list variable which would then pollute the global namespace.
  • I removed the unpatched minor versions (example: instead of keeping libcurl 7.65.0 I kept 7.65.3), it seems to make more sense this way.

Sorry about all the failing checks from the previous commit.

@waruqi
Copy link
Member

waruqi commented Nov 5, 2020

I used includes directly instead of declaring a function to prevent declaring a global add_versions_list variable which would then pollute the global namespace.

No, xmake.lua of each package is completely isolated and it will also not affect on_install.

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

Oh alrighty then. Another thing I didn't know. I'm making the changes as requested.

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

Well this time I don't understand why it's failing, maybe it's not including the good versions.lua file? Should I use includes("./versions.lua")?

@waruqi
Copy link
Member

waruqi commented Nov 5, 2020

Oh, it's the first time I use include in package/xmake.lua, I need to look at it, please wait. : (

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

No problem, I'm trying some stuff on my side too. It seems the versions.lua file is not executed (adding print statements inside this file has no effect), where is includes defined?

@waruqi
Copy link
Member

waruqi commented Nov 5, 2020

You should write:

includes(path.join(os.scriptdir(), "versions.lua"))

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

Perfect! Thank you.

@waruqi
Copy link
Member

waruqi commented Nov 5, 2020

ci fails, maybe we need add --without-zstd for macos

        if is_plat("macosx") then
            -- ...
            table.insert(configs, "--without-zstd")
        end

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

Should I make another PR?

@waruqi
Copy link
Member

waruqi commented Nov 5, 2020

No need for a new one, please modify it directly on the current pr

@waruqi waruqi merged commit 3eae5e3 into xmake-io:dev Nov 5, 2020
@waruqi
Copy link
Member

waruqi commented Nov 5, 2020

Thank you very much for your contribution. 👍

@SirLynix
Copy link
Member Author

SirLynix commented Nov 5, 2020

No problem 😸 I think you can expect a few more soon!

@SirLynix SirLynix deleted the curl-versions branch November 5, 2020 16:13
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.

2 participants