Refactor includes #7
Conversation
This is required to allow other projects to include files from libff without explicitly putting the root of libff source into their include path.
|
@tromer, that is a valid point. I'm not entirely convinced that this is worse than the current situation, however --- as mentioned in the pull request, any project that has a file named "common/utils.hpp" effectively cannot use libff. Do you have suggestions for how to resolve both of these issues? |
|
@popoffka above refers to a comment I accidentally deleted. Original text:
|
|
Suggested alternative: move all include files into corresponding "namespace" directories, and then use etc. This keeps directory namespaces clean, and consistent regardless of which subdirectory or project you're in. |
|
Checking best practice empirically on So it seems like both options are in wide use. But looking at the actual projects, I see that "reputable" libraries like boost, Qt, opencv2, gtest, gstreamer, gnuradio, NTL all use directory-qualified includes. Exceptions include photon and some KDE libraries. So my tendency is still towards explicit directories. |
|
Okay, I will convert all includes to be absolute and use explicit directory names and submit new pull requests. Thanks! |
# This is the 1st commit message: bazel support, initial commit Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#2: gitignore .bazelrc, bazel-* Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#3: remove sha256 from rule_foreign_cc rule Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#4: rename target ff to libff Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#5: put curves/mnt/mnt4, 6 in separate packages Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#6: bn128: drop "depends" from include prefix, for bazel compatibility Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#7: fix refs to targets mnt4, mnt6, libff Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#8: fix refs to @ate_pairing//:libgmp Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#9: change @// to // Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#10: delete obsolete mnt4, mnt6 targets from curves/mnt/BUILD.bazel Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#11: add target scalar_multiplication:multiexp_profile Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#12: list headers explicitly Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#13: dead code elim Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#14: dead code elim Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#15: BUILD files: explicitate srcs/hdrs, DCE, buildifier reformat Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#16: change @ate_pairing//:zm to @ate_pairing//ate-pairing Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#17: switch obazl repos from local to git Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#18: pin xbyak, ate-pairing repos to versions, to match upstream Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#19: delete git submodules, not needed with Bazel Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#20: add sha256 for xbyak, ate-pairing external repos Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#21: remove xbyak dep - it's included in ate-pairing Signed-off-by: Gregg Reynolds <[email protected]> # This is the commit message scipr-lab#22: restore depends dirs Signed-off-by: Gregg Reynolds <[email protected]>
This pull request converts the paths in all local includes (i.e. those including files from within libff) from absolute to relative. Using absolute paths in local includes is suboptimal, because it means that any project trying to include a file from libff must also include libff's src directory in its include path.
In other words, for
#include "common/utils.hpp"to work inside a libff file, it will also have to work inside the files of the project that's trying to include libff, which might lead to ambiguities if that project also has acommon/utils.hppfile, which is not unreasonable.External projects should add
/usr/include(or wherever libff was installed) to their include paths, and then include libff as#include <libff/algebra/whatever>. Pull requests that implement this in libsnark and libfqfft will be submitted in a couple of minutes.This pull request (as well as the accompanying ones in libsnark and libfqfft) was largely generated using a Python script.