ISSUE #85 : Base64-Encoder Executable #93

nav-mohan · 2025-01-21T03:50:05Z

Fixes issue #85
Currently, we use xxd and a native cmake implementation to encode parameter files for linking into a shared-library.
base64-encode is an executable meant to replace both.
It will be installed in the CMAKE_INSTALL_RELOC_BINDIR.

A utility for base64 encoding and decoding of model and model-driver parameter files. This biary executable will be used for compiling shared-libraries of model and model-driver. This utility will be installed in the CMAKE_INSTALL_RELOC_BINDIR.

Specify build and installation instructions for kim_base64_encode. kim_base64_encode is a critical dependant for building model/model-driver shared-libraries. kim_base64_encode must be built before kim-api. kim_base64_encode will be installed in the CMAKE_INSTALL_RELOC_BINDIR for system-wide access during compile-time of model/model-driver shared-libraries.

…ible now. Stylistic changes: base64.hpp moved to KIM_Base64.hpp KIM_Base64.hpp edited to have 2 space indent Function names/variable names to follow PascalCase or camelCase (like other KIM-API files) Doxygen style docstrings TODO: Licensing issue discussion with Ryan

This is a work in progress and the code is not expected to compile at this stage. A number of small changes have been made to port the code from c++11 to c++98. A pending change is to replace the string-literals with escaped characters.

Create the command-line-interface for kim-base64-encode following the docopt.org format keeping inline with other utils/kim-api-* binary executables

ipcamit · 2025-01-21T16:31:55Z

@nav-mohan I think the Open suse error stems from the fact that it is using newer OpenSUSE with newer libasan, so now libasan6 should be updated to libasan8. Not sure if that will solve it but probably worth a shot.

See here: https://opensuse.pkgs.org/tumbleweed/opensuse-oss-x86_64/libasan8-14.2.1+git10750-1.2.x86_64.rpm.html

codecov · 2025-01-22T22:12:59Z

Codecov Report

Attention: Patch coverage is 93.50649% with 10 lines in your changes missing coverage. Please review.

Project coverage is 46.13%. Comparing base (4d125ff) to head (ab54495).
Report is 2 commits behind head on devel.

Files with missing lines	Patch %	Lines
cpp/src/third-party/libb64/src/cdecode.c	80.43%	7 Missing and 2 partials ⚠️
cpp/src/third-party/libb64/src/cencode.c	98.41%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            devel      #93      +/-   ##
==========================================
+ Coverage   45.29%   46.13%   +0.83%     
==========================================
  Files         140      145       +5     
  Lines       13059    13359     +300     
  Branches     1338     1354      +16     
==========================================
+ Hits         5915     6163     +248     
- Misses       6476     6518      +42     
- Partials      668      678      +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yafshar · 2025-01-22T22:33:51Z

cpp/src/base64-encode/README

+`base64-encode` is a simple xxd replacement to convert files
+into embeddable C++ source code. Instead of using binary array
+it used base64 strings, which makes it much more performant 
+for large files. Currently only -i option is supported for 
+compatibility. More options to be released in future. C++ 98 compliant.


Suggested change

`base64-encode` is a simple xxd replacement to convert files

into embeddable C++ source code. Instead of using binary array

it used base64 strings, which makes it much more performant

for large files. Currently only -i option is supported for

compatibility. More options to be released in future. C++ 98 compliant.

`base64-encode` is a straightforward replacement for xxd,

designed to convert files into embeddable C++ source code.

Unlike binary arrays, it utilizes base64 strings, significantly

enhancing performance for large files. Currently, only the -i

option is supported for compatibility, with more options

planned for future releases. It is compliant with C++98 standards.

CMakeLists.txt

ipcamit · 2025-02-03T03:54:46Z

@nav-mohan Regardinhe Fedora debug is failing while complaining about position independent code. I think we can fix it by including

set_target_properties(b64 PROPERTIES POSITION_INDEPENDENT_CODE ON)

in the cpp/libb64/CMakeLists.txt file.

The window test seems to be failing from CRLF file endings, that is bit strange but needs closer looking into. Perhaps I can try manually looking into the generated C++ files on windows when I get time. Perhaps a windows os based macro to write "\r\n" is needed.

nav-mohan · 2025-02-04T02:28:47Z

@nav-mohan Regardinhe Fedora debug is failing while complaining about position independent code. I think we can fix it by including
set_target_properties(b64 PROPERTIES POSITION_INDEPENDENT_CODE ON)
in the cpp/libb64/CMakeLists.txt file.

The window test seems to be failing from CRLF file endings, that is bit strange but needs closer looking into. Perhaps I can try manually looking into the generated C++ files on windows when I get time. Perhaps a windows os based macro to write "\r\n" is needed.

@ipcamit thanks for looking into it. if you cant' acquire a Windows machine, you could execute these Github Actions workflows on your own fork of the repo.

nav-mohan · 2025-02-05T04:21:17Z

@nav-mohan Regardinhe Fedora debug is failing while complaining about position independent code. I think we can fix it by including
set_target_properties(b64 PROPERTIES POSITION_INDEPENDENT_CODE ON)
in the cpp/libb64/CMakeLists.txt file.

The window test seems to be failing from CRLF file endings, that is bit strange but needs closer looking into. Perhaps I can try manually looking into the generated C++ files on windows when I get time. Perhaps a windows os based macro to write "\r\n" is needed.

@ipcamit Some updates:
I was able to reproduce the MINGW bug by installing MINGW on my Ubuntu machine (sudo apt install mingw-w64) and using the MINGW compiler in my CMAKE

set(CMAKE_C_COMPILER x86_64-w64-mingw32-gcc)
set(CMAKE_CXX_COMPILER x86_64-w64-mingw32-g++)
set(CMAKE_EXE_LINKER_FLAGS "-static -static-libgcc -static-libstdc++")

A quick screenshot to show the output from mingw and gcc when used to encode examples/model-drivers/ex_model_driver_P_LJ/item-compiled-with-version.txt

The closing-quote and the semi-colon are at the start of the line as opposed to end of the line.
You were right in that the root cause of this problem is in the difference between newline characters in mingw (\r\n) and gcc(\n).
The particular bug we're seeing is due to the step of rewinding by 1 character which resets the write-head back to start of the line.
I've found two possible solutions to this

open the output filestream in binary-mode -- this prevents any ambiguity between \r\n and \n. 1 byte in either case is \n
comment out this line in the libb64 library source-code.

As for the Fedora workflow, enabling the PIC flag worked and that build runs successfully now.

ipcamit · 2025-02-05T16:32:08Z

Thank you @nav-mohan for investigating this. I think 2 is a better solution. Then we can remove the seekp line as well. Please log the changes in the .origin file as well.

ilia-nikiforov-umn · 2025-02-06T20:20:42Z

@ellio167 do you know how to handle the billing issue with the MacOS runners? I looked at the openkim organization billing page and didn't see anything obvious, but as @ipcamit pointed out offline, it might be up to the PR author to pay (since otherwise a malicious actor could bill someone's account just by submitting PRs)

ellio167 · 2025-02-06T20:33:53Z

We've never setup billing for github, I don't think. So, I think the settings for the openkim org as such that nothing non-free can run.

I would guess that we could change this and add a payment method (a pcard from one of the AEM staff, I suppose). However, don't know if that's something we want to do... Plus, I don't know anything about setting this up or maintaining/securing it...

ilia-nikiforov-umn · 2025-02-06T20:35:13Z

We've never setup billing for github, I don't think. So, I think the settings for the openkim org as such that nothing non-free can run.

I would guess that we could change this and add a payment method (a pcard from one of the AEM staff, I suppose). However, don't know if that's something we want to do... Plus, I don't know anything about setting this up or maintaining/securing it...

Got it, I'll see if I can figure out what to do here.

nav-mohan · 2025-02-06T21:02:23Z

@ilia-nikiforov-umn @ellio167 We can control specific triggers for running the build workflow.
We could disable the pull_request trigger. I'll see if there's a way to disable it specifically for the MacOS builds.

ipcamit · 2025-02-11T06:42:21Z

So what is the the status for this PR? Just want to know if anything is to be done at my end?

ilia-nikiforov-umn · 2025-02-11T17:09:29Z

@ilia-nikiforov-umn @ellio167 We can control specific triggers for running the build workflow. We could disable the pull_request trigger. I'll see if there's a way to disable it specifically for the MacOS builds.

@nav-mohan can you try removing the -large from the MacOS runners and see if that will work?

https://github.com/orgs/community/discussions/102846#discussioncomment-9344474

macos-14 is free, macos-14-large using more CPUs is not.

nav-mohan · 2025-02-11T23:13:53Z

@ilia-nikiforov-umn thanks for pointing that out. That was one of the issues.
Some further changes were needed for the compilers.
@ipcamit @ellio167 all the builds are working now.

ellio167 · 2025-02-12T14:31:22Z

@nav-mohan Can you please revert most of the changes in the PR so as to minimize the differences between the master devel branch and this PR? In particular, you've merged in changes from an official release which has made changes to essentially all the files by putting a specific version number in the file headers. Changes like this

-# Release: This file is part of the kim-api.git repository.
+# Release: This file is part of the kim-api-2.3.0 package.

should be eliminated. Thank you.

…e fix

libasan6 is deprecated and has been updated to libasan8 MacOS 10//11 is not supported by Github Actions and has been updated to Mac 13/14 Fortran compiler is not included in mingw-toolchain and must be explicitly installed Fedora requires POSITION_INDEPENDENT_CODE ON when linking a static library (libb64) with a dynamic library (kim-api) Workflows can now be manually triggered through web UI.

Going forward we'll try to store all external libraries in this folder

A small modification to libb64 is required to ensure the output of kim-api-base64-encode returns a valid C++ source-file.

use MacOS-13, MacOS-14 instead of MacOS-13-large, MacOS-14-large use clang++ compiler use gfortran-14 compiler

nav-mohan · 2025-02-12T22:24:25Z

@ellio167 commits have been squashes/dropped and the branch has been cleaned up.

ellio167 · 2025-02-17T15:54:59Z

I think this is ready; @nav-mohan @ipcamit take a final look to see if any of my refactors are objectionable.

ipcamit · 2025-02-18T13:56:36Z

Looks good to me!

nav-mohan · 2025-02-18T16:04:33Z

Looks good to me as well!

ipcamit and others added 6 commits January 12, 2025 14:17

Implement kim_base64_encode

493a400

A utility for base64 encoding and decoding of model and model-driver parameter files. This biary executable will be used for compiling shared-libraries of model and model-driver. This utility will be installed in the CMAKE_INSTALL_RELOC_BINDIR.

Relocate KIM_Base64Encode from utils/ to cpp/src/

f882cdf

Work In Progress - Port code to C++98

dd72e70

This is a work in progress and the code is not expected to compile at this stage. A number of small changes have been made to port the code from c++11 to c++98. A pending change is to replace the string-literals with escaped characters.

CLI for kim-base64-encode

c0dcefa

Create the command-line-interface for kim-base64-encode following the docopt.org format keeping inline with other utils/kim-api-* binary executables

nav-mohan force-pushed the devel branch from bcfff0b to 1ddb0ed Compare January 21, 2025 04:56

yafshar reviewed Jan 22, 2025

View reviewed changes

nav-mohan force-pushed the devel branch 3 times, most recently from ef8c6f5 to 1ddb0ed Compare February 2, 2025 18:54

yafshar reviewed Feb 2, 2025

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

yafshar reviewed Feb 2, 2025

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

nav-mohan force-pushed the devel branch from 1a6f1cf to ab54495 Compare February 2, 2025 20:10

nav-mohan force-pushed the devel branch from 91f3d0b to 3d87910 Compare February 6, 2025 04:27

nav-mohan force-pushed the devel branch 2 times, most recently from 602948d to afd1afe Compare February 11, 2025 23:03

ipcamit and others added 10 commits February 12, 2025 17:00

Complete commit for C++98 compatible KIM-API large file

d6d4cfc

Working version, modified to remove some compile time warnings + Cmak…

bed0277

…e fix

Working base64 version based off libb64

cf26f11

Removed unused var noise

d00550d

Remove redundant files

ea77b87

Ryans work on refactoring base64 additions

a969890

Rename directory to third-party

ca89c9a

Going forward we'll try to store all external libraries in this folder

Remove new-line character

167254b

A small modification to libb64 is required to ensure the output of kim-api-base64-encode returns a valid C++ source-file.

Update build-workflows for MacOS

34bbc8b

use MacOS-13, MacOS-14 instead of MacOS-13-large, MacOS-14-large use clang++ compiler use gfortran-14 compiler

nav-mohan force-pushed the devel branch from afd1afe to 34bbc8b Compare February 12, 2025 22:07

Ryan's Refactor of base64 PR

0025372

ellio167 merged commit 6fe9881 into openkim:devel Feb 18, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISSUE #85 : Base64-Encoder Executable #93

ISSUE #85 : Base64-Encoder Executable #93

nav-mohan commented Jan 21, 2025 •

edited

Loading

ipcamit commented Jan 21, 2025

codecov bot commented Jan 22, 2025 •

edited

Loading

yafshar Jan 22, 2025

ipcamit commented Feb 3, 2025 •

edited

Loading

nav-mohan commented Feb 4, 2025

nav-mohan commented Feb 5, 2025 •

edited

Loading

ipcamit commented Feb 5, 2025

ilia-nikiforov-umn commented Feb 6, 2025

ellio167 commented Feb 6, 2025

ilia-nikiforov-umn commented Feb 6, 2025

nav-mohan commented Feb 6, 2025 •

edited

Loading

ipcamit commented Feb 11, 2025

ilia-nikiforov-umn commented Feb 11, 2025

nav-mohan commented Feb 11, 2025

ellio167 commented Feb 12, 2025

nav-mohan commented Feb 12, 2025

ellio167 commented Feb 17, 2025

ipcamit commented Feb 18, 2025

nav-mohan commented Feb 18, 2025

-`base64-encode` is a simple xxd replacement to convert files
-into embeddable C++ source code. Instead of using binary array
-it used base64 strings, which makes it much more performant
-for large files. Currently only -i option is supported for
-compatibility. More options to be released in future. C++ 98 compliant.
+`base64-encode` is a straightforward replacement for xxd,
+designed to convert files into embeddable C++ source code.
+Unlike binary arrays, it utilizes base64 strings, significantly
+enhancing performance for large files. Currently, only the -i
+option is supported for compatibility, with more options
+planned for future releases. It is compliant with C++98 standards.

ISSUE #85 : Base64-Encoder Executable #93

ISSUE #85 : Base64-Encoder Executable #93

Conversation

nav-mohan commented Jan 21, 2025 • edited Loading

ipcamit commented Jan 21, 2025

codecov bot commented Jan 22, 2025 • edited Loading

Codecov Report

yafshar Jan 22, 2025

Choose a reason for hiding this comment

ipcamit commented Feb 3, 2025 • edited Loading

nav-mohan commented Feb 4, 2025

nav-mohan commented Feb 5, 2025 • edited Loading

ipcamit commented Feb 5, 2025

ilia-nikiforov-umn commented Feb 6, 2025

ellio167 commented Feb 6, 2025

ilia-nikiforov-umn commented Feb 6, 2025

nav-mohan commented Feb 6, 2025 • edited Loading

ipcamit commented Feb 11, 2025

ilia-nikiforov-umn commented Feb 11, 2025

nav-mohan commented Feb 11, 2025

ellio167 commented Feb 12, 2025

nav-mohan commented Feb 12, 2025

ellio167 commented Feb 17, 2025

ipcamit commented Feb 18, 2025

nav-mohan commented Feb 18, 2025

nav-mohan commented Jan 21, 2025 •

edited

Loading

codecov bot commented Jan 22, 2025 •

edited

Loading

ipcamit commented Feb 3, 2025 •

edited

Loading

nav-mohan commented Feb 5, 2025 •

edited

Loading

nav-mohan commented Feb 6, 2025 •

edited

Loading