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

rename beman_iterator to beman_iterator_interface #1453

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

changkhothuychung
Copy link
Contributor

#1445

Previous PR: #1446

Per discussion in beman-project/iterator_interface#6, we would like to rename this library to beman_iterator_interface

bin/yaml/libraries.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@neatudarius neatudarius left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@neatudarius neatudarius left a comment

Choose a reason for hiding this comment

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

I actually found a possible bug, @changkhothuychung please do test this PR before merging this PR. We need a working deployment soon for Meeting C++ 2024 Berlin (this week).

@@ -1025,7 +1025,7 @@ libraries:
targets:
- trunk
type: github
beman_iterator:
beman_iterator_interface:
build_type: none
Copy link
Contributor

@neatudarius neatudarius Nov 12, 2024

Choose a reason for hiding this comment

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

I actually realized that cloning is not enough for iterator_interface! We need to run cmake to have config.hpp created and installed in install dir! CC: @camio @RaduNichita

Demo:

dariusn /tmp/godbolt/iterator_interface [main] $ git status
On branch main
dariusn /tmp/godbolt/iterator_interface [main] $ git log -1
commit d6efb25da89a628844bae590e3ceea7245a73bca (HEAD -> main, origin/main, origin/HEAD)
Date:   Tue Nov 12 09:02:31 2024 +0200

dariusn /tmp/godbolt/iterator_interface [main] $ ls -l include/beman/iterator_interface/config.hpp
ls: include/beman/iterator_interface/config.hpp: No such file or directory

TLDR: We need to run cmake commands to translate include/beman/iterator_interface/config.hpp.in into config.hpp!


Following this example,

build_type: cmake
extra_cmake_arg:
- -DCPPTRACE_STATIC=On
lib_type: static
package_install: true
,
I think we should have:

beman_iterator_interface:
        build_type: cmake
        extra_cmake_arg:
        - -DBUILD_TESTING=OFF
        check_file: README.md
        type: github
        repo: beman-project/iterator_interface
        method: nightlyclone
        targets:
        - main

The expected result would be that CE use these comands and inside /opt/compiler-explorer/libs/beman_iterator_interface/main/include it will export next tree (snippet from my local setup). Full expected commands / side effects attached:

dariusn@pc:~/git/Beman/iterator_interface$ rm -rf .build && cmake -B .build -S .
-- The CXX compiler identification is GNU 13.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Performing Test HAVE_DEDUCING_THIS
-- Performing Test HAVE_DEDUCING_THIS - Failed
-- The C compiler identification is GNU 13.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Configuring done (2.3s)
-- Generating done (0.0s)
-- Build files have been written to: /home/dariusn/git/Beman/iterator_interface/.build
dariusn@pc:~/git/Beman/iterator_interface$ cmake --build .build --config Asan --target all -- -k 0
gmake: *** No rule to make target '0'.
[  8%] Building CXX object CMakeFiles/beman.iterator_interface.dir/src/beman/iterator_interface/iterator_interface.cpp.o
[ 16%] Linking CXX static library libbeman.iterator_interface.a
[ 16%] Built target beman.iterator_interface
[ 25%] Building CXX object examples/CMakeFiles/beman.iterator_interface.examples.sample.dir/sample.cpp.o
[ 33%] Building CXX object examples/CMakeFiles/beman.iterator_interface.examples.sample.dir/__/src/beman/iterator_interface/iterator_interface.cpp.o
[ 41%] Linking CXX executable beman.iterator_interface.examples.sample
[ 41%] Built target beman.iterator_interface.examples.sample
[ 50%] Building CXX object _deps/googletest-build/googletest/CMakeFiles/gtest.dir/src/gtest-all.cc.o
[ 58%] Linking CXX static library ../../../lib/libgtest.a
[ 58%] Built target gtest
[ 66%] Building CXX object _deps/googletest-build/googletest/CMakeFiles/gtest_main.dir/src/gtest_main.cc.o
[ 75%] Linking CXX static library ../../../lib/libgtest_main.a
[ 75%] Built target gtest_main
[ 83%] Building CXX object tests/beman/iterator_interface/CMakeFiles/beman.iterator_interface.tests.dir/iterator_interface.test.cpp.o
[ 91%] Building CXX object tests/beman/iterator_interface/CMakeFiles/beman.iterator_interface.tests.dir/__/__/__/src/beman/iterator_interface/iterator_interface.cpp.o
[100%] Linking CXX executable beman.iterator_interface.tests
[100%] Built target beman.iterator_interface.tests
dariusn@pc:~/git/Beman/iterator_interface$ rm -rf  /opt/compiler-explorer/libs/beman_iterator_interface/main/
dariusn@pc:~/git/Beman/iterator_interface$ cmake --install .build --prefix /opt/compiler-explorer/libs/beman_iterator_interface/main/
-- Install configuration: ""
-- Installing: /opt/compiler-explorer/libs/beman_iterator_interface/main/lib/libbeman.iterator_interface.a
-- Installing: /opt/compiler-explorer/libs/beman_iterator_interface/main/include/beman/iterator_interface/iterator_interface.hpp
-- Installing: /opt/compiler-explorer/libs/beman_iterator_interface/main/include/beman/iterator_interface/iterator_interface_access.hpp
-- Installing: /opt/compiler-explorer/libs/beman_iterator_interface/main/include/beman/iterator_interface/detail/stl_interfaces/config.hpp
-- Installing: /opt/compiler-explorer/libs/beman_iterator_interface/main/include/beman/iterator_interface/detail/stl_interfaces/fwd.hpp
-- Installing: /opt/compiler-explorer/libs/beman_iterator_interface/main/include/beman/iterator_interface/detail/stl_interfaces/iterator_interface.hpp
-- Installing: /opt/compiler-explorer/libs/beman_iterator_interface/main/bin/beman.iterator_interface.examples.sample
dariusn@pc:~/git/Beman/iterator_interface$ tree /opt/compiler-explorer/libs/beman_iterator_interface/main
/opt/compiler-explorer/libs/beman_iterator_interface/main
├── bin
│   └── beman.iterator_interface.examples.sample
├── include
│   └── beman
│       └── iterator_interface
│           ├── detail
│           │   └── stl_interfaces
│           │       ├── config.hpp
│           │       ├── fwd.hpp
│           │       └── iterator_interface.hpp
│           ├── iterator_interface.hpp
│           └── iterator_interface_access.hpp
└── lib
    └── libbeman.iterator_interface.a

8 directories, 7 files

TLDR: If current PR does NOT ensure config.hpp is installed in this directory, the deployment is fully broken:
image

Notes: When trying the use cmake to convert config.hpp.in into config.hpp, we don't actually care which compiler we use. It will use the "default" compiler found on the system: /usr/bin/c++.

@changkhothuychung , please try to apply suggested changes and try to test them with a local CE deployment - check https://github.com/compiler-explorer/compiler-explorer/blob/main/docs/AddingALibrary.md#adding-a-new-library-locally.

@partouf partouf merged commit 1fd9636 into compiler-explorer:main Nov 12, 2024
4 checks passed
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.

4 participants