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

Allow disabling of more features #3844

Closed
wants to merge 5 commits into from

Conversation

@bagder
Copy link
Member

commented May 5, 2019

This is a patch set that

  1. Makes the #ifdefs for features disable more code that isn't used
  2. Adds more defines to disable more specific features if desired (not all yet available via easy configure options)
  3. Adjusts tool code to be able to run with a libcurl built with features disabled
  4. The API/ABI is maintained but will return errors for disabled features.
@bagder

This comment has been minimized.

Copy link
Member Author

commented May 5, 2019

Blah, silly me. It broke because it relies on something like #3846 so I'll get that fixed and landed first, then fixup this branch again and push a fix.

@bagder bagder force-pushed the bagder/disable-more-features branch from 15cdb98 to bfb46bc May 6, 2019
@bagder bagder force-pushed the bagder/disable-more-features branch from b54a97b to 6618efb May 9, 2019
@bagder

This comment has been minimized.

Copy link
Member Author

commented May 12, 2019

I'm having a hard time figuring out why the windows CI builds fail like this, so I'm more or less bisecting in the separate branch bagder/ci. I might then land some of the commits piecemeal instead once proven to pass the CI.

@bagder bagder force-pushed the bagder/disable-more-features branch from 6618efb to 07be034 May 13, 2019
@MarcelRaad

This comment has been minimized.

Copy link
Member

commented May 14, 2019

Ah, here is the PR I was looking for. Did you forget a "Closes" line? These are in master now and broke the HTTP-only AppVeyor tests.

@bagder

This comment has been minimized.

Copy link
Member Author

commented May 14, 2019

Only some of them are in master, the ones I (wrongly apparently) thought didn't break master...

bagder added 5 commits May 5, 2019
Closes #3844
@bagder bagder force-pushed the bagder/disable-more-features branch from 07be034 to 1aa1e50 May 17, 2019
@bagder bagder closed this in 697b1f9 May 18, 2019
@bagder bagder deleted the bagder/disable-more-features branch May 18, 2019
@gvanem

This comment has been minimized.

Copy link
Member

commented on e91e481 May 23, 2019

I think:

#define HAVE_GETSOCKNAME 1

should be added to config-win32.h also.

This comment has been minimized.

Copy link
Member Author

replied May 23, 2019

Yes thanks, fixed in 170bd04 via #3923 I think.

This comment has been minimized.

Copy link
Member

replied May 23, 2019

More fishy use of if(getsockname(xx).

E.g. in ftp.c, it's used w/o any test for HAVE_GETSOCKNAME. We can assume Winsock at least has always had this function.

This comment has been minimized.

Copy link
Member Author

replied May 23, 2019

Ah, and in numerous places too. I guess this shows that when I worked with the TCP stack that lacks this function, I also had FTP disabled... 😁

@lock lock bot locked as resolved and limited conversation to collaborators Aug 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.