search

HaikuArchives / Calendar

:calendar: A native Calendar application for Haiku.
MIT License
29 stars 20 forks source link

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

Open OscarL opened 1 year ago

OscarL commented 1 year ago

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 https://github.com/HaikuArchives/Calendar/commit/de85b861924b63d473d387ce3047b1773405b0a0.

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 commented 1 year ago

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

OscarL commented 1 year ago

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 commented 1 year ago

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:

pulkomandy commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

pulkomandy commented 1 year ago

You have to use SetBits. ImportBits is deprecated now

OscarL commented 1 year ago

@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 commented 1 year ago

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 https://github.com/HaikuArchives/Calendar/commit/2ae7fb499655d6a1482ef107fa9d37eeb5229c2c as a patch for the recipe would be acceptable, if that's enough to compile it and make it work again.

OscarL commented 1 year ago

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 commented 1 year ago
  • 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 commented 5 months ago

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.