Skip to content

Conversation

@saiarcot895
Copy link

This PR restructures the patches to use one patch file per logical change and to use quilt, so that it's easier to maintain and modify in the future. In addition, configure.ac in klish source code is now modified to properly check for python, curl, and cjson, instead of having include directories/linker flags be passed in. (Note, however, that because the cjson package in Buster does not provide pkg-config files, the include directories and linker flags are still passed in. This can be removed for Bullseye.)

This also makes some changes for the armhf cross-compile build (sonic-net/sonic-buildimage#8035).

The current patching infra for klish involved a custom script as well as
a directory tree of the patches to apply, split for each source file.
This, IMO, is not good for maintainability, since the context/purpose
for each change is split across multiple files and it's a little bit
harder to correlate changes across files. In addition, quilt and stg are
already used for patching elsewhere.

Therefore, replace the individual file diffs with two patches (one for a
possible build fix with libxml, and the other being the Python 3 support
and klish enhancements). Some cleanup was done in the latter patch to
reduce the size of the diff, but should be otherwise functionally
equivalent.

Signed-off-by: Saikrishna Arcot <[email protected]>
Instead of hardcoding the include paths and library paths in the
Makefile, add dynamic checks for curl, python, and libcjson into
configure.ac.

Signed-off-by: Saikrishna Arcot <[email protected]>
@saiarcot895 saiarcot895 force-pushed the clean-up-klish-patches-and-build branch from 9a2f6ca to 1206ca7 Compare August 18, 2022 18:00
@saiarcot895
Copy link
Author

@praveen-li could you help review this PR?

@saiarcot895
Copy link
Author

@praveen-li Would you be able to review this PR?

@joyas-joseph
Copy link
Contributor

Looks good to me.

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.

2 participants