Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fails on GCC2 due to undeclared stoi and to_string methods. #134

Open
OscarL opened this issue Nov 8, 2022 · 13 comments
Open

Build fails on GCC2 due to undeclared stoi and to_string methods. #134

OscarL opened this issue Nov 8, 2022 · 13 comments

Comments

@OscarL
Copy link

OscarL commented Nov 8, 2022

I was updating Calendar's recipe for Haikuports (to address the "missing symbols" after libshared changes). Latest git revision builds and works OK on 64 bits, but the builds fails on x86_gcc2, with errors like:

"::stoi undeclared" and "::to_string undeclared" on EventWindow.cpp. I assume this has been like this since de85b86.

I was thinking on moving Calendar to the secondaryArch (x86) on 32 bits, but that might be problematic if Calendar uses (or plans to have) replicants (eg: #89), right?

@pulkomandy
Copy link
Member

Why does Calendar use these instead of BString? If APIs are missing in BString they could be added there.

@OscarL OscarL changed the title Build fais on GCC2 due to undeclared stoi and to_string methods. Build fails on GCC2 due to undeclared stoi and to_string methods. Nov 8, 2022
@OscarL
Copy link
Author

OscarL commented Nov 12, 2022

Why does Calendar use these instead of BString? If APIs are missing in BString they could be added there.

Dunno, let page @harshit-sharma-gits to ask him about it. 8^)

@nielx
Copy link

nielx commented Nov 12, 2022

Why does Calendar use these instead of BString? If APIs are missing in BString they could be added there.

Hard disagree with the rhetoric of this question. std::stoi and std::string are perfectly valid for use as part of the C++ library. I do not share the viewpoint that BString is preferred in all cases. If this was part of the Haiku source code, I may be sympathetic to the argument that for consistency purposes BString should be considered, but not for what is essentially a third party app.

I was updating Calendar's recipe for Haikuports (to address the "missing symbols" after libshared changes). Latest git revision builds and works OK on 64 bits, but the builds fails on x86_gcc2, with errors like:

"::stoi undeclared" and "::to_string undeclared" on EventWindow.cpp. I assume this has been like this since de85b86.

I was thinking on moving Calendar to the secondaryArch (x86) on 32 bits, but that might be problematic if Calendar uses (or plans to have) replicants (eg: #89), right?

Two notes:

  • It is not finished. The app needs more work. The latest Git revision should be considered to be unstable. So don't update the recipe.
  • Could Calendar be made compatible with GCC2 gain? Probably. std::stoi could be replaced with a BString or C-library/POSIX equivalent. Should it be? I don't know... The fact that maybe one day it will use a replicant.... you might as well argue that it can be backported once that has happened...

@pulkomandy
Copy link
Member

I didn't look closely at what the code is doing (for lack of time on my side :( ) and I don't know each and every method available in BString. I expect there should be an easy way to convert to and from integers using BString APIs. If there is not, that seems like something that could be improved in BString.

So my question is not really rethoric, it's an honest one: can we improve the BString API on Haiku side?

I'm fine with dropping gcc2 support when it's not possible or not reasonable to do otherwise, and I work on several apps which go that way. No one wants to be stuck writing C++98 forever, I think? I'm not sure converting between strings and integer is the place where you really need a modern compiler. I think the "compromise" to make here would be very small and the plans to make this app a replicant means it seems worth doing. But yes, this is a 3rd party app and I'm not in control of it, so that's just my opinion.

I am more interested in adding the missing API on Haiku side. My experience in other apps mixing std::string and BString shows that it is not really fun to do, and error-prone in some cases. And with the public API using BString in many place, it's not really possible to use the C++ standard APIs everywhere. So for my own code I prefer using BString and I'm interested in adding any missing APIs there could be there. Wether this specific app then decides to use these APIs or not is not really what worries me here.

@OscarL
Copy link
Author

OscarL commented Nov 12, 2022

Thanks for the input @nielx, much appreciated.

As I'm new to the whole Haikuports thing, and my skills are almost non-existent, but I want to contribute the little I can... in this case, trying to rebuild the apps affected by the libshared symbol things. But if that requires more than some quick fixes... I'm out of my depth already :-/

I tend to second/triple guess everything I attempt. so I seek out as much feedback as I can from people with more knowledge and/or experience. Albeit I have to admit that is not always easy for me to absorb (specially when opinions do not match).

The replicant thing was pointed out to me on IRC by a long time haikuports contributor, as I didn't even have thought about it, that's why I've mentioned it here.

So don't update the recipe.

That's partly why I set that PR at haikuports as a Draft too. To seek out this type of feedback.

I guess I should see if the original Calendar recipe can be rebuilt "as-is" (without updating to newer commits), and call it a day until someone else comes up with better solution.

@OscarL
Copy link
Author

OscarL commented Nov 13, 2022

For what it's worth:

I've just tried building the current Calendar recipe from HaikuPorts (Calendar-0.1), and now I remember what prompted me to try to update the recipe to newer code in the first place.

As mentioned on my draft PR, that version can't be built on Haiku after hrev55770 due to:

error 'status_t BBitmap::ImportBits(const BBitmap*, BPoint, BPoint, int32, int32)' is private in this context

And trying to solve that issue is what brought me here, asking for suggestions/help :-)

@nielx
Copy link

nielx commented Nov 13, 2022

I will do some more work on this after we release R1 Beta 4. I will send you an update when ready.

@pulkomandy
Copy link
Member

You have to use SetBits. ImportBits is deprecated now

@OscarL
Copy link
Author

OscarL commented Nov 13, 2022

@pulkomandy:

You have to use SetBits. ImportBits is deprecated now

Sorry for not being clearer: Latest Calendar code in this repo don't have the ImportBits() related error anymore (thus why I intended to use the newer code, but then the GCC2 issue appeared).

@nielx:

That's fine by me. Thanks.

@lonemadmax
Copy link

I want to contribute the little I can... in this case, trying to rebuild the apps affected by the libshared symbol things.

I've just tried building the current Calendar recipe from HaikuPorts (Calendar-0.1), and now I remember what prompted me to try to update the recipe to newer code in the first place.

As mentioned on my draft PR, that version can't be built on Haiku after hrev55770 due to:

error 'status_t BBitmap::ImportBits(const BBitmap*, BPoint, BPoint, int32, int32)' is private in this context

In that case, I guess adding 2ae7fb4 as a patch for the recipe would be acceptable, if that's enough to compile it and make it work again.

@OscarL
Copy link
Author

OscarL commented Nov 19, 2022

Thanks for the useful suggestion @lonemadmax! (one that I'll try to keep in mind in the future!)

Sadly, the patch resulting from git format-patch -1 2ae7fb4 does not applies cleanly over the old Calendar 0.1 code, and "massaging it" to fit seems a bit too much at this point (at least for me: as a non-user of this app, I can wait :-D).

@scottmc
Copy link
Member

scottmc commented Nov 21, 2022

  • It is not finished. The app needs more work. The latest Git revision should be considered to be unstable
    @nielx you say the app in its current state is unstable, can you elaborate on that, and open new issues for things you feel are holding it back from being able to release a new version? We had a GSoC student work on it this past summer, is any of that ready for prime time yet?
    I keep seeing updated recipes / patches to build the 0.1, are we not ready for a 0.2 yet?

@korli
Copy link
Contributor

korli commented Mar 18, 2024

Just a note, that I set up the CI builder for x86 secondary arch. When/If gcc2 is to be used, it can be adjusted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants