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

Move libapps.a source to apps/lib #9723

Closed
wants to merge 2 commits into from

Conversation

@levitte
Copy link
Member

levitte commented Aug 28, 2019

This makes it clearer what's what. The 'openssl' application and its
sub-commands remain in apps/

This makes it clearer what's what.  The 'openssl' application and its
sub-commands remain in apps/
@levitte levitte force-pushed the levitte:apps-rearrangement-20190828 branch from eb84185 to e0e4571 Aug 28, 2019
@richsalz

This comment has been minimized.

Copy link
Contributor

richsalz commented Aug 28, 2019

{Removing my comment text because it wasn't nice. Apologies if it bothered anyone.}

@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Aug 28, 2019

On demand from discussion in #9331

Note that more work is needed. apps/openssl.c contains library functions, and apps/lib/apps.c refers to some of those functions, making it impossible to re-use libapps.a with the test programs.
That, however, is work for another PR.

@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Aug 28, 2019

Being an OMC member doesn't necessarily mean that everything gets automatic high priority. Have a look at the number of open PRs I have hanging around....

@richsalz

This comment has been minimized.

Copy link
Contributor

richsalz commented Aug 28, 2019

I think it's an anti-pattern to have things go "up and over" That is, -I flags shouldn't go up and then down; header files used by files within .../a and .../b shouldn't be in either the a or b directories, but should rather be at a separate subdir at least at the same level as a and b. YMMV.

@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Aug 28, 2019

More or less all -I values go "up and over", because the top include demands it and all file or directory references in build.info files are relative to the location of that build.info file.

@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Aug 28, 2019

As for your idea of making libapps a top level thing, I'm not too keen, because I fear that would make it look like something that we release alongside libcrypto and libssl, and I'd rather that library stay internal.

@t8m
t8m approved these changes Sep 4, 2019
@t8m t8m added the approval: done label Sep 4, 2019
levitte added a commit that referenced this pull request Sep 4, 2019
This makes it clearer what's what.  The 'openssl' application and its
sub-commands remain in apps/

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #9723)
@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 4, 2019

Merged.

2ad75c6 Move libapps.a source to apps/lib

@levitte levitte closed this Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.