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

Port to Plan 9 #510

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Port to Plan 9 #510

wants to merge 28 commits into from

Conversation

lufia
Copy link

@lufia lufia commented Mar 24, 2019

I've ported to Plan 9(9legacy).

Plan 9 don't have GNU autotools, and porting it is very hard, so I decide that create mkfiles manually.
Also I created few patches in plan9 directory because Plan 9's ANSI/POSIX C compiler supports only a part of C99 standard.

@lufia lufia marked this pull request as ready for review April 8, 2019 14:39
@lufia
Copy link
Author

lufia commented Apr 29, 2019

I think this p-r is ready for review.
Is there something to prepare here?

@lufia
Copy link
Author

lufia commented May 14, 2019

I will give it a try to generate mkfiles from Makefile.am.

@busterb
Copy link
Contributor

busterb commented May 17, 2019

Great, thanks!

@lufia lufia force-pushed the plan9 branch 4 times, most recently from b4a4427 to cd79456 Compare May 22, 2019 12:58
@lufia lufia force-pushed the plan9 branch 6 times, most recently from 6fb8a3d to 27ad9fc Compare May 31, 2019 18:56
@lufia
Copy link
Author

lufia commented Jun 24, 2019

I added a commit to my pull request.
This commit changes update.sh to be able to generate obj_mac.h and obj_dat.h without perl in Plan 9.

I use usually 9legacy but I think 9front could be same.

@lufia
Copy link
Author

lufia commented Jul 27, 2019

Is there any update on this?

@kinichiro
Copy link
Contributor

I don't have the plan9 environment, and just my personal thoughts from looking at PR, though,

  • To separate files relate to plan9, what about moving autogen.rc, gen-mkfile.sh, mkfile and apps/mkfile under plan9/ ?
    mkfile and apps/mkfile can be renamed to, for example, plan9/mkfile.main and plan9/mkfile.apps, something like that.
    And modify plan9/autogen.rc to copy plan9/mkfile.main and plan9/mkfile.apps to appropriate path as mkfile while executing plan9/autogen.rc.
    If apply this, .gitignore should just ignore mkfile only.
  • Instead of directly changing update.sh, how do you like applying plan9/update.sh.patch to update.sh while executing plan9/autogen.rc.

@kinichiro
Copy link
Contributor

I have questions about huge *.patch files.
I think it is good to keep those patch files small to maintain easily.

  1. Additional include file required ?
    In crypto.patch and apps.patch, many <openssl/asn1t.h> and one <openssl/poly1305.h> are additionally included.
    I wonder why these are required on plan9, since other platforms does not require them.
    I also want to know what requires stddef.h in chacha-merged.c and sys/types.h in cryptlib.c.

  2. cast required ?
    In crypto.patch, ssl.patch and tls.patch, there are many additional cast.
    Does plan9 C compiler have option like -Wno-pointer-sign or something like that ?
    Other platforms ignore these rather than adding cast.

@lufia
Copy link
Author

lufia commented Jan 4, 2020

@kinichiro

To separate files relate to plan9, what about moving autogen.rc, gen-mkfile.sh, mkfile and apps/mkfile under plan9/ ?

ok, I can do this.

Instead of directly changing update.sh, how do you like applying plan9/update.sh.patch to update.sh while executing plan9/autogen.rc.

I can do this but ... what kind of reasons?

@lufia
Copy link
Author

lufia commented Jan 4, 2020

I think it is good to keep those patch files small to maintain easily.

It's sounds good. I might drop .opt.value replacement. But I think it is hard to drop both #includes and casts unless C files under openbsd/src is changed. I described the reason.

Additional include file required ?

If it don't include <asn1t.h> Plan 9 C compiler outputs some errors like:

portable/crypto/ocsp/ocsp_cl.c:165[stdin:67991] structure not fully declared ASN1_ITEM_st
portable/crypto/ocsp/ocsp_cl.c:212[stdin:68037] structure not fully declared ASN1_ITEM_st

ocsp/ocsp_cl.c calls OCSP_REQUEST_sign macro

if (!OCSP_REQUEST_sign(req, key, dgst))

Its macro and related objects are defined in openssl/ocsp.h and openssl/ossl_typ.h

#define OCSP_REQUEST_sign(o,pkey,md) \
    ASN1_item_sign(&OCSP_REQINFO_it, \
    o->optionalSignature->signatureAlgorithm,NULL, \
    o->optionalSignature->signature,o->tbsRequest,pkey,md)

extern const ASN1_ITEM OCSP_REQINFO_it;

typedef struct ASN1_ITEM_st ASN1_ITEM;

OCSP_REQINFO_it is declared in ocsp/ocsp_asn.c

const ASN1_ITEM OCSP_REQINFO_it = { ... }

Also ASN1_ITEM_st is defined in openssl/asn1t.h

struct ASN1_ITEM_st {
	...
};

Thus, In the files that is added #include in these patches, like ocsp/ocsp_cl.c, the expression &OCSP_REQINFO_it can't resolve its address because the type ASN1_ITEM_st is the incomplete type.

cast required ?

Yes. Plan 9 C compiler makes an error if it find type mismatch; pointer to signed and pointer to unsigned are different. Unfortunately don't have the option to ignore its behavior.

@kinichiro
Copy link
Contributor

Hi, @lufia

Instead of directly changing update.sh, how do you like applying plan9/update.sh.patch to update.sh while executing plan9/autogen.rc.

I can do this but ... what kind of reasons?

This is my personal thoughts, and I don't mean to force.
update.sh should keep simple, and does not need to have the part for the specific platform.

@kinichiro
Copy link
Contributor

If it don't include <asn1t.h> Plan 9 C compiler outputs some errors like:
...
Yes. Plan 9 C compiler makes an error if it find type mismatch; pointer to signed and pointer to unsigned are different. Unfortunately don't have the option to ignore its behavior.

I understand your situation.
Thanks for clarification.

@kinichiro
Copy link
Contributor

@lufia, I have one question (not forcing) for this PR.

Are you thinking about build from tarball (.tar.gz) ?
This PR seems that it enables building from git repository, but not from tarball.
I think almost all of users will build from tarball, not from git repository, since public release is provided by a tarball via download site.
Building from repository gives us a benefit to get latest update, but on the other hand, build might unstable since source code is changed so often.
Tarball contains fixed source code, then you can provide the patches for specific release tarball version.

I understand this PR is a first step of porting, and again this is not forcing, just a question.

@lufia
Copy link
Author

lufia commented Jan 6, 2020

@kinichiro I've not be noticed. And I think I should add this to tarball. Which file do generate tarball?

@kinichiro
Copy link
Contributor

@lufia Makefile.am "EXTRA_DIST" is used to include files into tarball.

@staalmannen
Copy link

@lufia would you consider to roll a "release tarball" of your libressl port? I would like some of your work on ports (https://code.9front.org/hg/ports) - especially libressl and curl (there is an old curl port but your port is much nicer). Despite being hosted on 9front it is not 9front-specific and if you want to you could package your own work there (or I could do it for you).

Using master.zip from github is annoying since they will change over time, so a "release tarball" would be preferable.

@lufia
Copy link
Author

lufia commented Jun 1, 2021

Hi, @staalmannen

Today I uploaded the patched sources at https://9p.io/sources/contrib/lufia/

The libressl-portable-20210530.tgz archive can be built in Plan 9 applied many patches that is described in README.plan9 contained in that sources.

I checked that:

% tar zxf libressl-portable-20210530.tgz
% cd crypto
% mk

@Neustradamus
Copy link

What is the situation of this PR?

It will be merged soon?

@lufia
Copy link
Author

lufia commented Oct 14, 2023

I noticed this was conflicted today. I'm going to fix that.

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.

5 participants