To adopt PR #1264 approach of using Readline replacement for MinGW build (Low Priority) #1271
Assignees
Labels
enhancement
New feature or request
Comments
This was referenced Jan 7, 2023
|
@mariusgreuel |
|
I add this as part of the AVRDUDE 7.2 milestone. But you can remove this if you feel that this is of lower priority. Thanks. |
|
PR #1513 (Add GNU Readline to mingw github action) brings in undesired dependency of libwinpthread-1.dll to the mingw32/mingw64 binaries. I have to revert the PR. It is very strange that there is no such side effect under my own MSYS2 ming64 build. Anyway, I think it is better to use the approach used by @mariusgreuel for MSVC to the MinGW build as well since GNU Readline is not that good under Windows. |
|
Since @mariusgreuel is busy, I will try to see if I can do something about this one.
|
|
Need your help here as I am stuck at this error. I have zero knowledge on flex/bison. I believe my changes are quite similar to PR#1264 (note: error in term.c was fixed in the next commit). |
|
It's a mystery why make/the compiler does not find avrdude.h for lexer.c (that will have been generated from lexer.l by flex). Virtually all other .c sources require avrdude.h too. Maybe sprinkle in a -Isrc or -Isrc/CMakeFiles/libavrdude.dir/all or similar? |
|
The complete verbose build log for reference. verbose build log |
|
Compilation of These |
|
git main verbose build log for comparison. git main verbose build log |
|
Yes, git main has the needed |
Thanks for finding the root cause. Now I need to see how to fix it. Probably the CMake scripts need to be updated. Unfortunately my CMake knowledge is very limited. |
|
I am sorry to disturb you again. It seems that my CMake files changes are causing this issue. I modeled the change from PR #1264. But PR #1264 does not touch MinGW build. |
|
Known Windows specific issues. Bug
Lack of features compared to Linux and macOS
MinGW build |
|
Short description "why" (why it doesn't work): Full description: The target uses Otherwise @mcuee if you grant me write permission to your fork - I'll push the fix directly into your branch, I already have it locally and don't want to create another fork of my own. |
|
Thanks a lot. I have added you as a collabortor for my avrdude fork. You should be able to push to my fork once you accept the invitation. |
|
So the missing includes issue is fixed but now we have another one :) I don't have mingw locally to try/debug this. UPD: and add in |
|
Your change is good on my local MSYS2 mingw installation. local build log |
|
@Youw |
|
Do you even want the possibility to long against system-provided libreadline? |
We still want to use GNU Readline for Linux/FreeBSD/etc and libedit for macOS (readline is a keg for Homebrew under macOS). But GNU Readline is not so good under Windows. So we do not want it for MSVC/MinGW. |
|
Let me push the suggested update to enforce that. |
|
Here is the verbose log. failed build log |
|
I tend to think that we are almost there. I think we may need to change some ifdefs in the file |
|
Just one more verbose log for reference (MSYS2 clang64 build) verbose build log for MSYS2 clang64 |
No, not that. |
Wonderful. Now it is a success. |
|
I'l unsubscribing from this conversation now. |
|
@stefanrueger and @Youw One potential side effect of using tje C++ Just wondering how difficult it is to rewrite |
|
Lets discuss this as part fo #1553 |
|
Unfortunately readline.cpp does not work under MinGW. I have abondoned PR #1553. @mariusgreuel |
mcuee
changed the title
|
After the merging of #1576, this issue becomes less important since GNU Readline will work in most cases for the MinGW build. It is still nice to have but can be of lower priority. |
|
Close this one as not planned until someone is actibely working on this issue. This issue is also linked here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PR #1264 has been merged and it performs much better than using GNU Readline under Windows. It is good to adopt the same approach for MinGW.
The text was updated successfully, but these errors were encountered: