Closed twojstaryzdomu closed 1 year ago
@twojstaryzdomu Thanks for PR and sorry for delay.
I'm going to merge the PR and test CI builds and debian packages. However I have two small concerns:
bin/Release/eperftool usr/bin
bin/Release/simple_client usr/bin
bin/Release/simple_server usr/bin
bin/Release/test_code_params usr/bin
bin/Release/test_create_instance usr/bin
bin/Release/test_encoder_instance usr/bin
Do we ever need to install this tools? I think they are internal tool + examples + tests and are needed only during development on developer machine.
Maybe we should remove libopenfec-tools at all?
And another small question regarding libopenfec-1.4.2.4.spec
. It contains version in its name. Does it mean we need to manually create a new spec file each release? Or rename it? Maybe this could be automated?
If you think some of these concerns should be addressed, it would be great if you could do it in subsequent PRs.
Follow-up commit: d89dba2dd2c3138ce61087f0cceb23937230213d
Now CI runs simple build (cmake + make) on any push and pull request, and runs packaging steps only on push to versioned tag.
I created a test tag v9.9.9. The build succeeded: https://github.com/roc-streaming/openfec/actions/runs/3554057286
It generated github release openfec_9.9.9: https://github.com/roc-streaming/openfec/releases/tag/untagged-d4f9158b4e447065ff7f
I was not able to install the generated debian package:
sudo dpkg -i ./libopenfec_9.9.9_amd64.deb
dpkg-deb: error: archive './libopenfec_9.9.9_amd64.deb' uses unknown compression for member 'control.tar.zst', giving up
dpkg: error processing archive ./libopenfec_9.9.9_amd64.deb (--install):
dpkg-deb --control subprocess returned error exit status 2
Errors were encountered while processing:
./libopenfec_9.9.9_amd64.deb
https://unix.stackexchange.com/questions/669004/zst-compression-not-supported-by-apt-dpkg
I'm running debian stable. Maybe we could make packages compatible with it?
I was able to install packages inside docker container with ubuntu.
dpkg -L libopenfec
/.
/usr
/usr/lib
/usr/lib/x86_64-linux-gnu
/usr/lib/x86_64-linux-gnu/libopenfec.so.1.4.2
/usr/share
/usr/share/doc
/usr/share/doc/libopenfec
/usr/share/doc/libopenfec/changelog.gz
/usr/lib/x86_64-linux-gnu/libopenfec.so
/usr/lib/x86_64-linux-gnu/libopenfec.so.1
dpkg -L libopenfec-dev
/.
/usr
/usr/include
/usr/include/openfec
/usr/include/openfec/lib_advanced
/usr/include/openfec/lib_advanced/ldpc_from_file
/usr/include/openfec/lib_advanced/ldpc_from_file/of_codec_profile.h
/usr/include/openfec/lib_advanced/ldpc_from_file/of_ldpc_ff.h
/usr/include/openfec/lib_advanced/ldpc_from_file/of_ldpc_ff_api.c
/usr/include/openfec/lib_advanced/ldpc_from_file/of_ldpc_ff_api.h
/usr/include/openfec/lib_advanced/ldpc_from_file/of_ldpc_ff_includes.h
/usr/include/openfec/lib_advanced/ldpc_from_file/of_ldpc_includes.h
/usr/include/openfec/lib_common
/usr/include/openfec/lib_common/linear_binary_codes_utils
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_hamming_weight.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_hamming_weight.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_matrix_convert.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_matrix_convert.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_matrix_dense.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_matrix_dense.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_matrix_sparse.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_matrix_sparse.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_tools.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/binary_matrix/of_tools.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/it_decoding
/usr/include/openfec/lib_common/linear_binary_codes_utils/it_decoding/of_it_decoding.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/it_decoding/of_it_decoding.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/ml_decoding
/usr/include/openfec/lib_common/linear_binary_codes_utils/ml_decoding/.DS_Store
/usr/include/openfec/lib_common/linear_binary_codes_utils/ml_decoding/._.DS_Store
/usr/include/openfec/lib_common/linear_binary_codes_utils/ml_decoding/of_ml_decoding.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/ml_decoding/of_ml_decoding.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/ml_decoding/of_ml_tool.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/ml_decoding/of_ml_tool.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/of_create_pchk.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/of_create_pchk.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/of_linear_binary_code.h
/usr/include/openfec/lib_common/linear_binary_codes_utils/of_symbol.c
/usr/include/openfec/lib_common/linear_binary_codes_utils/of_symbol.h
/usr/include/openfec/lib_common/of_cb.h
/usr/include/openfec/lib_common/of_debug.h
/usr/include/openfec/lib_common/of_mem.c
/usr/include/openfec/lib_common/of_mem.h
/usr/include/openfec/lib_common/of_openfec_api.c
/usr/include/openfec/lib_common/of_openfec_api.h
/usr/include/openfec/lib_common/of_openfec_profile.h
/usr/include/openfec/lib_common/of_rand.c
/usr/include/openfec/lib_common/of_rand.h
/usr/include/openfec/lib_common/of_types.h
/usr/include/openfec/lib_common/statistics
/usr/include/openfec/lib_common/statistics/of_statistics.c
/usr/include/openfec/lib_common/statistics/of_statistics.h
/usr/include/openfec/lib_stable
/usr/include/openfec/lib_stable/2d_parity_matrix
/usr/include/openfec/lib_stable/2d_parity_matrix/of_2d_parity.h
/usr/include/openfec/lib_stable/2d_parity_matrix/of_2d_parity_api.c
/usr/include/openfec/lib_stable/2d_parity_matrix/of_2d_parity_api.h
/usr/include/openfec/lib_stable/2d_parity_matrix/of_2d_parity_includes.h
/usr/include/openfec/lib_stable/2d_parity_matrix/of_codec_profile.h
/usr/include/openfec/lib_stable/ldpc_staircase
/usr/include/openfec/lib_stable/ldpc_staircase/of_codec_profile.h
/usr/include/openfec/lib_stable/ldpc_staircase/of_ldpc_includes.h
/usr/include/openfec/lib_stable/ldpc_staircase/of_ldpc_staircase.h
/usr/include/openfec/lib_stable/ldpc_staircase/of_ldpc_staircase_api.c
/usr/include/openfec/lib_stable/ldpc_staircase/of_ldpc_staircase_api.h
/usr/include/openfec/lib_stable/ldpc_staircase/of_ldpc_staircase_pchk.c
/usr/include/openfec/lib_stable/reed-solomon_gf_2_8
/usr/include/openfec/lib_stable/reed-solomon_gf_2_8/of_codec_profile.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_8/of_reed-solomon_gf_2_8.c
/usr/include/openfec/lib_stable/reed-solomon_gf_2_8/of_reed-solomon_gf_2_8.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_8/of_reed-solomon_gf_2_8_api.c
/usr/include/openfec/lib_stable/reed-solomon_gf_2_8/of_reed-solomon_gf_2_8_api.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_8/of_reed-solomon_gf_2_8_includes.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/galois_field_codes_utils
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/galois_field_codes_utils/algebra_2_4.c
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/galois_field_codes_utils/algebra_2_4.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/galois_field_codes_utils/algebra_2_8.c
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/galois_field_codes_utils/algebra_2_8.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/galois_field_codes_utils/of_galois_field_code.c
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/galois_field_codes_utils/of_galois_field_code.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/of_codec_profile.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/of_reed-solomon_gf_2_m.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/of_reed-solomon_gf_2_m_api.c
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/of_reed-solomon_gf_2_m_api.h
/usr/include/openfec/lib_stable/reed-solomon_gf_2_m/of_reed-solomon_gf_2_m_includes.h
/usr/share
/usr/share/doc
/usr/share/doc/libopenfec-dev
/usr/share/doc/libopenfec-dev/changelog.gz
I see two minor problems with -dev
package:
.c
files should be excluded from package.pc
file is not installedAlso I think here:
SET(PKG_CONFIG_CFLAGS
"-I\${includedir}/lib_common -I\${includedir}/lib_stable"
)
we should also add lib_advanced
.
One more follow-up commit: 695498ee6906833b172e6364b95bcd70286d7045
Package version in cmake is now automatically extracted from latest git tag.
I've summarized issues with debian packages in #7.
Do we ever need to install this tools? I think they are internal tool + examples + tests and are needed only during development on developer machine.
Maybe we should remove libopenfec-tools at all?
In keeping tools as a package I followed a convention for most debian builds whereby any binaries for library builds are packaged either as -tools or -utils. I recognise they're not strictly necessary but there is little cost to keeping them included.
And another small question regarding libopenfec-1.4.2.4.spec. It contains version in its name. Does it mean we need to manually create a new spec file each release? Or rename it? Maybe this could be automated?
The version is sadly hardcoded in the contents of the file. Keeping the version in the filename was my way of indicating the contents of the file are specific to that version. Whenever the version is bumped up, the record for the Version inside the file requires an update. So with your raising the minor version number, the spec file needs to be renamed and updated accordingly.
What is needed for the update to be automatic is a GH-specific CI running before the release runs, so that the updated spec file is included in the GH sources accompanying the release. The CI needs to look up the variable for the latest version and update it if needed inside the file. Is this supplementary package going to be updated often though to be worth the effort of putting a CI in place? Any users looking for a spec file will know how to update it.
In keeping tools as a package I followed a convention for most debian builds whereby any binaries for library builds are packaged either as -tools or -utils. I recognise they're not strictly necessary but there is little cost to keeping them included.
Yeah, I see. But I think they are not just unnecessary, but may confuse the users? Because I don't see any use case for them outside of the source tree (when you're developing openfec itself). And also if you install them, they will pollute your PATH with strange names like "simple_server". I believe they were not ever intended for system-wide installation.
The version is sadly hardcoded in the contents of the file. Keeping the version in the filename was my way of indicating the contents of the file are specific to that version. Whenever the version is bumped up, the record for the Version inside the file requires an update. So with your raising the minor version number, the spec file needs to be renamed and updated accordingly.
Ah, got it. I agree, this repo doesn't release frequently, no need for full automation.
However I think it would be useful and add a check to CI: if rpm spec does not correspond to the latest git tag, CI should fail. Also it would be useful to add a bash script that updates the spec to the new version.
This way, when creating a new release, I'll need to run the script and make a tag. If I forgot to run the script, CI will remind me.
I've created issues: #11, #12
@twojstaryzdomu BTW could you also take a look at the question in https://github.com/roc-streaming/openfec/issues/9#issue-1466493548 ?
Produces rpms files required by the
roc-toolkit
spec.