fixes from @marcows repo
Closed this issue · 42 comments
I was happy to learn that you are back and active maintaining srecord - great news.
Are you keeping track of the changes Markus has in his repo, to eventually merge them back into upstream ?
For example,
-
fix-end-of-address-spacebranch - 4 commits, could probably be rebased + merged ? -
reference-manual-pdf-featuresbranch - probably difficult to port now due to different doc generation ? - new OS65A format by @glitchwrks ? https://github.com/glitchwrks/SRecord/tree/os65a_work
Thanks fenugrec. I thought I had pulled across all of Markus's branches. Looks like I have a few more to go!
The reference manual stuff might be tricky but I'll take a look. Definitely keen to pull in the new OS65A format too.
Thanks for the pointers. I'll log progress here.
I plan to have a look at my changes and rebase them - in the next days if possible.
That would be great Markus. Thank you.
Does that include the OS65A format? If not, I'll take a look at pulling that myself. Either is good for me.
The "fix-end-of-address-space" part is now ready, PR created. One of the four commits (marcows/srecord@9aa2a7b) had already been applied upstream in 3a7c13a - not the original patch, though, so there is another new bugfix included in the PR.
Will soon have a look at my reference manual fixes/improvements.
The OS65A stuff I don't know about, that's for someone else :)
Maybe @chapmajs himself wants to push that forward?
I separated out the documentation fixes from "reference-manual-pdf-features" branch to get them out of the way for the real PDF improvements. Thereby found some more things to fix. See PR #11
I might not respond until after weekend.
Yes, porting the PDF features is a bit tricky, but it seems doable.
After rebase I have a basic state now, only two problems currently.
-
"sed '/^[.]XX ./d'" had been removed from soelim invocation during switch to cmake, but this sed invocation was extended for eliminating .pdf* macros from troff to avoid bad influence in HTML+man. Currently the URLs are missing (removed) there when generating HTML and man pages. Might I have to reactivate the sed invocation or had it been removed for a good reason, @sierrafoxtrot ?
-
"ref-parts.so" is generated automatically now, so I have to find another way to get PDF outline hierarchy.
Will continue next week I guess.
Regarding item 1, wasn't removed for a good reason. I was looking to get 1.65 "good enough" to publish and so still had a backlog of minor issues including this one to work on later.
Regarding ref-parts.so, it was originally automatically generated so I just kept that. If there is a good reason to go with hand-crafted I'd be keen to understand.
Happy to help, great project that I really couldn't live without! Is this now the official repo for SRecord? If so, I can rebase and send a PR for my OS65A stuff here.
I've been using the OS65A output generator extensively for my own projects, I feel that it's pretty solidly tested at this point.
Is this now the official repo for SRecord?
@sierrafoxtrot has been the maintainer of srecord on SF for years, and just recently became (quite) active again. Context here in his announcement https://sourceforge.net/p/srecord/mailman/message/37724629/
Just read the announcement, great to hear! I'll rebase against this repo and send a PR, if that's easier than integrating from @marcows repo.
That would be sensational @chapmajs! I was literally sitting in a cafe wondering how best to get in touch with you.
Regarding ref-parts.so, it was originally automatically generated so I just kept that.
Oh, indeed, it was already generated before.
My SRecord repo was based on the v1.64 zip file which included ref-parts.so, so I thought it wasn't generated.
Regarding ref-parts.so, it was originally automatically generated so I just kept that.
Oh, indeed, it was already generated before. My SRecord repo was based on the v1.64 zip file which included ref-parts.so, so I thought it wasn't generated.
Ah, of course. There were several files that were generated by the old cook build system and just included in distribution tarball. This included ref-parts but also the makefile itself. I build the cmake based build from cook rather than the makefile. Apologies for the unexpected surprise on that one :-)
Hey @chapmajs, just following up to see if you'd had time to progress the rebase? I have a next release coming soon and would love to include OS65A. Completely understand that we have limited hours in the day and I'm more than happy to fill in any gaps remaining.
Thanks Jonathan!
Tried to get the current source building on Mac OS X but am having issues. I can handle it at work (Linux) but it's probably something to look at.
Looks like there's a requirement for CMake 3.22, but Slackware 15 is on CMake 3.21. I'm building a local CMake 3.26 but it's probably a good idea to push the requirement back to 3.21.x if possible.
If you edit the CMakeLists.txt cmake_minimum_required(VERSION 3.21) , does it build ?
It builds as well as with 3.26, which is to say, it doesn't :P
Am I doing something wrong? I'm running cmake CMakeLists.txt in the project root, followed by make.
Not quite. You should be doing something like
mkdir build
cd build
cmake-gui ..
(or cmake ..)
make
e.g. you want to compile in a subdir which exists separately from the source dir.
I can't make in the build/ directory as there's no Makefile in there. In any case, I'm getting errors like this:
/home/glitch/projects/srecord/srecord/arglex.cc:32:1: note: ‘memcmp’ is defined in header ‘<cstring>’; did you forget to ‘#include <cstring>’?
31 | #include <srecord/versn_stamp.h>
+++ |+#include <cstring>
32 |
/home/glitch/projects/srecord/srecord/arglex.cc: In member function ‘int srecord::arglex::token_next()’:
/home/glitch/projects/srecord/srecord/arglex.cc:451:31: error: ‘strchr’ was not declared in this scope; did you mean ‘std::strchr’?
451 | const char *eqp = strchr(arg.c_str(), '=');
| ^~~~~~
| std::strchr
In file included from /home/glitch/projects/srecord/srecord/arglex.cc:24:
/usr/include/c++/11.2.0/cstring:106:3: note: ‘std::strchr’ declared here
106 | strchr(char* __s, int __n)
Looks like there's some difference in the standard lib configs between my environments (this is on both Slackware 15 and Mac OS X) and stuff out of the std namespace aren't being pulled in as expected. Is development by chance mainly going on in Windows, by chance?
I can't make in the build/ directory as there's no Makefile in there.
What ?
cmake creates makefiles wherever you want. What happens when you run cmake (or preferably cmake-gui .. as I wrote before) from the build-dir ?
e.g.
[21Gi] srecord $ mkdir build
[21Gi] srecord $ cd build
[21Gi] build $ cmake ..
-- The C compiler identification is GNU 12.2.1
-- The CXX compiler identification is GNU 12.2.1
-- Detecting C compiler ABI info
....
....
-- Configuring done (1.5s)
-- Generating done (0.1s)
-- Build files have been written to: /home/q/d/srec/srecord/build
[21Gi] build $ make
[ 1%] Building CXX object srecord/CMakeFiles/lib_srecord.dir/adler16.cc.o
....
Main dev environment is Linux (I'm running xubuntu but I've also seen it built recently on Fedora). Windows support has only recently become a standard build. Keen to get OS X and slackware working though.
For building do check out the BUILDING.pdf which can be fetched from the website. Summary typical instructions will be (from the toplevel directory):
mkdir build && cd build && cmake .. && cmake --build . && ctest running make directly at this stage might mask a whole multitude of problems.
Suggest we sort out this cmake building issue first. (compiler is complaining for example about including files that are indeed included)
OK, I reset to HEAD and followed the instructions:
[glitch@mbp srecord] (master)$ pwd
/Users/glitch/projects/srecord
[glitch@mbp srecord] (master)$ git reset --hard HEAD
HEAD is now at 6bca708a Add git hash including to documentation(#55)
[glitch@mbp srecord] (master)$ mkdir build
[glitch@mbp srecord] (master)$ cd build
[glitch@mbp build] (master)$ cmake ..
-- GIT_SHA1 6bca708ae4
-- COPYRIGHT_YEARS 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2018, 2019, 2020, 2022, 2023
-- PS2PDF /usr/local/bin/ps2pdf
-- CYGPATH CYGPATH-NOTFOUND
-- Packaging for Macintosh
-- gcrypt location
-- CMAKE_INSTALL_SYSTEM_RUNTIME_DESTINATION lib
-- CMAKE_INSTALL_PREFIX /usr
-- CMAKE_INSTALL_SYSTEM_RUNTIME_DESTINATION lib
-- CMAKE_INSTALL_PREFIX /usr
-- CMAKE_INSTALL_SYSTEM_RUNTIME_DESTINATION lib
-- CMAKE_INSTALL_PREFIX /usr
-- Configuring done (0.5s)
-- Generating done (0.3s)
-- Build files have been written to: /Users/glitch/projects/srecord
[glitch@mbp build] (master)$ ls -lah
total 0
drwxr-xr-x 2 glitch staff 64B Apr 1 19:13 .
drwxr-xr-x 34 glitch staff 1.1K Apr 1 19:13 ..
No Makefile in build/ as you can see. There's a Makefile generated in the parent directory, but that doesn't help me in the build/ subdirectory, and I figure if it's deviating from the usual development process then something's wrong somewhere.
Doing a make in the parent directory produces stuff like the following (just the current visible output in tmux):
In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/string_view:175:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/__string:393:62: error: cannot initialize a parameter of type 'wchar_t *' with an lvalue of type 'std::__1::char_traits<char>::char_type *' (aka 'char *')
: __n == 0 ? __s : (char_type*)memset(__s, to_int_type(__a), __n);
^~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/wchar.h:154:27: note: passing argument to parameter here
wchar_t *wmemset(wchar_t *, wchar_t, size_t);
^
In file included from /Users/glitch/projects/srecord/srecord/arglex.cc:24:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/cstring:69:9: error: no member named 'memcpy' in the global namespace; did you mean 'wmemcpy'?
using ::memcpy;
~~^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/wchar.h:152:10: note: 'wmemcpy' declared here
wchar_t *wmemcpy(wchar_t * __restrict, const wchar_t * __restrict, size_t);
^
In file included from /Users/glitch/projects/srecord/srecord/arglex.cc:24:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/cstring:70:9: error: no member named 'memmove' in the global namespace; did you mean 'wmemmove'?
using ::memmove;
~~^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/wchar.h:153:10: note: 'wmemmove' declared here
wchar_t *wmemmove(wchar_t *, const wchar_t *, size_t);
^
In file included from /Users/glitch/projects/srecord/srecord/arglex.cc:24:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/cstring:71:9: error: no member named 'strcpy' in the global namespace
using ::strcpy;
~~^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/cstring:72:9: error: no member named 'strncpy' in the global namespace
using ::strncpy;
~~^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/cstring:73:9: error: no member named 'strcat' in the global namespace
using ::strcat;
~~^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
Are the results any different if try again and from the build directory run cmake --build . instead of running make itself?
ie starting from scratch at HEAD do something like
mkdir build && cd build && cmake .. && cmake --build . && ctest
That Makefile being generated in the parent directory is definitely an issue.
I think we might have a hiccup along these lines:
https://stackoverflow.com/questions/24876930/cmake-produces-output-in-parent-directory
That Makefile being generated in the parent directory is definitely an issue.
I think we might have a hiccup along these lines: https://stackoverflow.com/questions/24876930/cmake-produces-output-in-parent-directory
That seems to be the problem! Still building but it looks like the cache file in the parent directory was the issue.
Sorry if this is a n00b error, but I don't use CMake in any of my personal or $day_job projects!
Don't feel bad. CMake is wonderful but it broke my spirit several times :-). It still happens on occasion!
One tip that I never really appreciated was it can be helpful to call cmake --build . instead of make. Also, ctest instead of make test. On some platforms the default build system might be ninja or something else and doing this will push that complexity into the background. You can pass it args like -j for example so cmake -j 8 --build . kick off 8 parallel compile jobs and it will translate to the underlying build tool.
Thanks for sticking with it!
One tip that I never really appreciated was it can be helpful to call
cmake --build .
Good point. Also note that git reset --hard doesn't delete untracked files - try git status to see for yourself.
Agreed, forgetting to delete CMakeCache files after certain types of changes, is a subtle trap that probably caught everyone at least once !
That's another advantage of building "out of source", such as inside a subdir : easier to purge and start over, without affecting the repo you're working from.
No problem -- thanks for keeping the project alive and working with me to get what I'm sure amounts to a rounding-error platform integrated :P We...actually generate this output for what amounts to billable work.
I have the initial work ported over:
https://github.com/glitchwrks/SRecord/tree/os65a_work
I would like to get a test written for it, but couldn't figure out the old build system (I think I was missing some critical part having to do with Ageis or whatever the old one was...something basically unsearchable due to some namespace collission IIRC). It looks like tests are much easier for the new system, so let me see if I can get that knocked out tonight.
The tests are such a valuable part of the project so that would be greatly appreciated! I'd also recommend checking out the "New Format" section of the reference manual (which should now be building for you). It has a handy list of files that need to be updated and are easily overlooked especially for new input formats to enable --guess to work and for inclusion in the libsrecord public interface.
Alright, tests run no problem now, and I have a working test! I have added to the suggested documentation. Should I add to AUTHORS too?
Currently this only generates output, as the dump format from the OSI 65A monitor is completely different. That's OK, right?
Also it looks like it builds on Slackware 15 with CMake 3.21, but the tests all fail as it can't find test_prelude.sh. I'll see if I can run that to ground and do a separate PR on it, as it'd be nice to build with the CMake included with Slackware 15. I can update the Slackbuild for it once the new release is out, either way.
Probably something to do with ENVIRONMENT_MODIFICATION which the tests are using, and appears to have been added in CMake 3.22:
https://cmake.org/cmake/help/latest/release/3.22.html
I'm going to say that fixing this in a way that handles path separation differences the way path_list_prepend does automatically probably greatly exceeds my experience with CMake and ctest. If this is something that someone else can fix without too much headache, great, I can test it on Slackware 15 for you. If not, I'll do something disgusting in the Slackbuild and sed the version back in the CMakeList.txt or something :P
Definitely agree we peel the cmake changes off into a separate PR to investigate. But sounds like great progress! and I'm keen to take a look when you are happy.
OK I'll go ahead and open a PR for it!
I have a next release coming soon
I know it has been a while without activity, but I really hoped to get PDF metadata and outline of the reference-manual-pdf-features branch integrated before a new release.
Do you have a schedule/deadline?
Amongst others, slight differences in destination position of the PDF outline links between Linux and Windows build was one reason holding me back.
Do you have a schedule/deadline?
No hard and fast deadline. The normal trigger for a new release used to be either a new format or a serious bug fix and we have three new formats including OS65A so I figured it was about time. But there's no hard and fast deadline so please don't panic or feel rushed.
I'd really like to understand what the changes are if that is at all possible. Is there a branch I could check out just to understand what kind of changes you'd like to make?
Added a file input feature for OSI 65A to the current PR. Friend suggested that maybe someone will have some SRecord output in OSI 65A format and need to read it back in (using SRecord, of course!) and turn it into something else.
I'd really like to understand what the changes are if that is at all possible. Is there a branch I could check out just to understand what kind of changes you'd like to make?
I created #58 so discussion can continue there.
This issue could be closed now.
Task "OS65A format" has been finished by #57.
Task "PDF features" has been finished by #58, though not all of the original adaptions have been adopted:
- No hierarchical/tree structure for the man page sections in the PDF outline (grouping into man 1/3/5).
Since the TOC is also flat, it is consistent at least. I'm not sure if I will resume work on this, have just played around with a bit now in branch "pdf-outline-tree". - No hypertext links (via .pdfhref) in the text.
In the meantime (since 43bafe1) several links have been modified to use .UR/.UE macros, but unfortunately they won't be transformed to active links in the PDF automatically.