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

djgpp: build fixes #6382

Closed
wants to merge 12 commits into from
Closed

djgpp: build fixes #6382

wants to merge 12 commits into from

Conversation

gvanem
Copy link
Contributor

@gvanem gvanem commented Dec 27, 2020

Here are some fixes for lib/Makefile.dj, src/Makefile.dj and some packages/DOS/ files.
Not many are using djgpp nowadays, but I've had these changes locally for ages. So now hopefully the diffs are out of the way.

  Allow 'Makefile.dist' to build both 'lib' and 'src'.
  *) Allow using the Windows hosted djgpp cross compiler
     to build for MSDOS under Windows.
  *) 'USE_SSL' -> 'USE_OPENSSL'.
  *) Added a 'link_EXE' macro. Etc, etc.
  *) Linking 'curl.exe' needs '$(CURLX_CFILES)' too.
  *) Do not pick-up '../lib/djgpp/*.o' files. Recompile locally.
  *) Generate a gzipped 'tool_hugehelp.c' if 'USE_ZLIB=1'.
packages/DOS/README Outdated Show resolved Hide resolved
packages/DOS/common.dj Outdated Show resolved Hide resolved
Copy link
Member

@jay jay left a comment

Choose a reason for hiding this comment

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

.

@jay jay added the build label Dec 27, 2020

#
# Use zlib for contents encoding
# Use zlib for contents encoding. Needed for 'USE_OPENSSL=1' too.
Copy link
Member

Choose a reason for hiding this comment

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

Since when is zlib needed for OpenSSL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be when perl Configure enable-zlib is used (what I used last time I built OpenSSL with djgpp).
No use of zlib.dll on MSDOS you know.

Copy link
Member

Choose a reason for hiding this comment

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

It could be when perl Configure enable-zlib is used (what I used last time I built OpenSSL with djgpp).

Then I think you could say something like # May be needed if using static OpenSSL built with zlib enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. But how about the syntax USE_ZLIB ?= 0. So one could do a make -f Makefile.dj USE_ZLIB=1 clean all.

Copy link
Member

Choose a reason for hiding this comment

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

But how about the syntax USE_ZLIB ?= 0

They should probably all be like that since there is no configure script

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done that now.

Copy link
Member

@jay jay left a comment

Choose a reason for hiding this comment

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

.

packages/DOS/common.dj Outdated Show resolved Hide resolved
packages/DOS/common.dj Outdated Show resolved Hide resolved
Previous warnings related to AFL fuzzy-functions are gone. Hence no need for that.
Clarify the 'conditional variable assignment' in 'common.dj'.
@gvanem gvanem marked this pull request as ready for review January 7, 2021 11:43
Copy link
Member

@jay jay left a comment

Choose a reason for hiding this comment

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

for next-feature-window

packages/DOS/README Show resolved Hide resolved
@jay jay added feature-window A merge of this requires an open feature window and removed feature-window A merge of this requires an open feature window labels Jan 26, 2021
@jay jay closed this in 3611f6a Jan 28, 2021
@jay
Copy link
Member

jay commented Jan 28, 2021

Thanks. I landed this now because IMO djgpp build is broken without it so I see no point in waiting until the next release.

I was not able to work with this PR the traditional way, it says it's from "unknown repository". Where is your curl fork?

For posterity I'll note found a workaround, that I'm sure to forget. I added .patch to the URL then git am'd it, like this

curl -OL https://github.com/curl/curl/pull/6382.patch
git am 6382.patch

then I squashed all the commits into one via interactive rebase (so I could combine and edit all the commit messages), fetched upstream, rebased on upstream/master, checkout my master, merge --ff-only, review then push upstream.

@gvanem
Copy link
Contributor Author

gvanem commented Jan 28, 2021

... it says it's from "unknown repository". Where is your curl fork?

Sorry about that. I messed up my fork and hence deleted it. I'm still very little into these more advanced Git commands.
Thanks for the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants