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

Ensure all examples build without error #628

Merged

Conversation

yogsothoth
Copy link
Contributor

@yogsothoth yogsothoth commented Oct 29, 2021

Description

Add all examples to example/Jamfile
Add necessary 'using' for libpng
Bring small fixes to examples' source files (remove unused variables)

Environment:

  • clang 11.0.1
  • FreeBSD 13.0-RELEASE-p4
  • x86_64

References

See #436

Tasklist

  • Add examples' source files to example/Jamfile
  • Fix examples so they compile without warning using default toolset
  • Add documentation to each example as comments in source files
  • Add license text in each .cpp and .hpp in example/
  • Remove documentation of examples from Readme.md
  • Add Readme.md files for each example, indicating any specifics and requirements
  • Add test case(s) (no new test case)
  • Ensure all CI builds pass
  • Clean up example/Jamfile once CI builds pass (remove unnecessary libtiff)
  • Review and approve

@yogsothoth
Copy link
Contributor Author

I have a few questions:

  • Should we update the documentation to reference the missing examples?
  • Should we leave only the compilation targets (not the exe) in the final Jamfile?
  • I've had issues with linking libpng that I'm not sure how to fix. It's likely my own lack of knowledge here showing: I could not get b2 to pass -lpng to the compilation command, only -ljpeg would show. I ended up forcing it on the command line for the sake of checking the build with linkflags=-lpng. Could someone point me to some docs or examples to help me with this?

@mloskot mloskot added the example Examples of how to use GIL label Oct 30, 2021
@mloskot
Copy link
Member

mloskot commented Oct 30, 2021

Hi, first thanks for your contribution, it's helpful.

Should we update the documentation to reference the missing examples?

I don't think we should care about describing the examples in the documentation.
In fact, I'd clear the examples from the documentation.
Instead, examples should be described in comment block at the top of .cpp files (i.e. what an example is about, what feature it presents).
There also should be README.md files which describe any particularities about the examples (i.e. build requirements, specific data required, etc.).
Then, the documentation should only link to the documentation folder (i.e. on GitHub).

Should we leave only the compilation targets (not the exe) in the final Jamfile?

I'm not sure I understand.
We certainly want to let users to build (compile+link) examples using the b2 and the Jamfile-s we provide.

I've had issues with linking libpng that I'm not sure how to fix.

You'd have to be more specific what you are doing, what is the output, errors, etc.
Feel free to open new issue for that problem.

Here is (new home of) B2 with its documentation:
https://www.bfgroup.xyz/b2/manual/main/index.html

@yogsothoth
Copy link
Contributor Author

I don't think we should care about describing the examples in the documentation. In fact, I'd clear the examples from the documentation. Instead, examples should be described in comment block at the top of .cpp files (i.e. what an example is about, what feature it presents). There also should be README.md files which describe any particularities about the examples (i.e. build requirements, specific data required, etc.). Then, the documentation should only link to the documentation folder (i.e. on GitHub).

Alright, I will include that in the tasklist then.

I'm not sure I understand. We certainly want to let users to build (compile+link) examples using the b2 and the Jamfile-s we provide.

Apologies, I wasn't being super clear: should the default target in the top Jamfile also include producing the executable files for the examples now, or do we leave it as it is today (i.e. build-project test)?

I've had issues with linking libpng that I'm not sure how to fix.

You'd have to be more specific what you are doing, what is the output, errors, etc. Feel free to open new issue for that problem.

Indeed, I just didn't want to turn this thread into a support ticket. I will open a separate ticket.

Here is (new home of) B2 with its documentation: https://www.bfgroup.xyz/b2/manual/main/index.html

I read it a few times and couldn't find the section that would help me with my problem so far... I'm clearly missing something.

@yogsothoth
Copy link
Contributor Author

Regarding the issue with linking: it was a problem with the project-cache.jam not being updated. Deleting this file and rebuilding fixed it.

@mloskot
Copy link
Member

mloskot commented Nov 2, 2021

Yes, the GIL's top-level Jamfile can also build the examples.

AFAICT, all Boost's CI services do b2 libs/<name>/test so they won't spend any (unnecessary) time on building examples.

@yogsothoth yogsothoth force-pushed the bugfix/issue-436-build-examples-no-error branch from c22a18e to 537f8ae Compare November 5, 2021 16:52
@yogsothoth
Copy link
Contributor Author

I just updated the PR with some explanations for the examples.
I also remarked that some of them don't include any mention of any license. Shall I take the opportunity to ensure they all do?

@mloskot
Copy link
Member

mloskot commented Nov 5, 2021

Good catch, yes, all source files should have the licence/copyright header comment, like this:

//
// Copyright 2005-2007 Adobe Systems Incorporated
//
// Distributed under the Boost Software License, Version 1.0
// See accompanying file LICENSE_1_0.txt or copy at
// http://www.boost.org/LICENSE_1_0.txt
//

@yogsothoth yogsothoth force-pushed the bugfix/issue-436-build-examples-no-error branch from 537f8ae to bef4b91 Compare November 6, 2021 12:22
example/Jamfile Outdated Show resolved Hide resolved
example/convolve2d.cpp Outdated Show resolved Hide resolved
example/hessian.cpp Outdated Show resolved Hide resolved
example/histogram_equalization.cpp Outdated Show resolved Hide resolved
example/hvstack.hpp Outdated Show resolved Hide resolved
example/sobel_scharr.cpp Outdated Show resolved Hide resolved
@yogsothoth
Copy link
Contributor Author

Thank you for your remarks!

I've updated the attributions and the Jamfile indent.
I have prepared Readme files with a synopsis, and some indications of what the examples require, in a different commit; I will update the PR.

@yogsothoth
Copy link
Contributor Author

I noticed example/convolve2d.cpp was a bit off compared to the other examples: one output file had a png extension but a jpeg_tag{}, it ended with a cin(), contained redundant, commented out code, etc.

I cleaned it up.

This is the last of the changes I can see we can bring here: if you find all is well, then I think we can add the target example to the root Jamfile.

Copy link
Member

@mloskot mloskot left a comment

Choose a reason for hiding this comment

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

Great work! It all looks good to me. Thanks!

@yogsothoth
Copy link
Contributor Author

Thank you for your time!

I just added the target example to the root Jamfile; marking this as Ready for review.

@yogsothoth yogsothoth marked this pull request as ready for review November 10, 2021 16:26
@mloskot mloskot changed the title #436 Ensure all examples build without error Ensure all examples build without error Nov 10, 2021
@mloskot mloskot merged commit 0b24f4c into boostorg:develop Nov 10, 2021
@mloskot
Copy link
Member

mloskot commented Nov 10, 2021

Thank you for your contribution

@mloskot mloskot mentioned this pull request May 12, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat/documentation example Examples of how to use GIL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants