haikuports icon indicating copy to clipboard operation
haikuports copied to clipboard

haiku-format: update to latest commit

Open humdingerb opened this issue 2 years ago • 14 comments

Use the provided build.sh

humdingerb avatar Oct 11 '22 05:10 humdingerb

Instead of having build.sh download the llvm and clang sources via wget, this recipe downloads them, see SOURCE_URL_2/3. build.sh never gets to use wget. Is cmd:wget still needed as a build requirement?

humdingerb avatar Oct 11 '22 05:10 humdingerb

It would be a lot simpler to make the recipe build directly from https://github.com/owenca/llvm-project/tree/haiku-format which is a version of clang and llvm already patched with haiku-format patches. That is how the recipe used to work, why change it again?

In any case, wget should not be needed.

pulkomandy avatar Oct 11 '22 06:10 pulkomandy

Oh, I thought it'd be easier for @owenca (or anyone) to maintain those patches and build script than provide the whole patched clang/llvm code. Should I revert that?

humdingerb avatar Oct 11 '22 11:10 humdingerb

For me it's easier to work with the full repository at least in the context of haikuports packaging. I will let owenca say if he has any preference.

pulkomandy avatar Oct 11 '22 12:10 pulkomandy

Instead of having build.sh download the llvm and clang sources via wget, this recipe downloads them, see SOURCE_URL_2/3. build.sh never gets to use wget.

I can simplify build.sh or add a command-line option if you want to use it in the recipe.

Should I revert that?

Yeah, or edit the recipe after I add the feature you wanted and fix the tabs in _haiku-format.

owenca avatar Oct 12 '22 05:10 owenca

I will let owenca say if he has any preference.

Thanks, though I don't know which will be easier until after updating haiku-format to the latest LLVM that Haiku supports. I will keep both repositories in sync meanwhile.

BTW the copywrite is including Saloni Goyal, Adrien Destugues, but none of their work has been merged. I asked @saloniig to create pull requests but haven't heard back.

owenca avatar Oct 12 '22 05:10 owenca

I will keep both repositories in sync meanwhile.

Alright. Then I'll revert to how it was done before and we'll see if anything needs to change when you made an update.

BTW the copywrite is including Saloni Goyal, Adrien Destugues, but none of their work has been merged. I asked @saloniig to create pull requests but haven't heard back.

OK, we'll remove them then for now. I'll wait with the recipe update for your ~/config/setting/.haiku-format update. No hurry.

humdingerb avatar Oct 12 '22 13:10 humdingerb

@owenca, is there a chance to put the _haiku-format config file into the [llvm]https://github.com/owenca/llvm-projectl) repo? Then we'd just have to pull in one repo in the recipe.

ETA: I see that https://github.com/owenca/llvm-project/commit/bcc5a44f23e6663ade208840fe5509c6da5f2459 includes a _haiku-format. Is it save to use he latest commitin that repo? (There's no _haiku-format any more..)

humdingerb avatar Oct 12 '22 15:10 humdingerb

@humdingerb The _haiku-format file has always been there and is the same as the one in https://github.com/owenca/haiku-format. :) Did you switch branch to haiku-format in the https://github.com/owenca/llvm-project repo?

Anyway, I need to fix the tabs in _haiku-format as noted above before you can use it.

owenca avatar Oct 13 '22 07:10 owenca

Did you switch branch to haiku-format in the https://github.com/owenca/llvm-project repo? Of course not... I was wondering why I couldn't find the commit hash of the old recipe in master. :) Give us a shout when you've finished your changes.

humdingerb avatar Oct 13 '22 11:10 humdingerb

@humdingerb I'm done with updating https://github.com/owenca/llvm-project/tree/haiku-format. Now that the default config file goes into ~/config/settings, I recommend that you use _haiku-format, not .haiku-format, as the file name.

I have noticed that the package name of haiku-format is haiku_format. Not sure it was on purpose or an oversight.

owenca avatar Oct 14 '22 08:10 owenca

Now that the default config file goes into ~/config/settings, I recommend that you use _haiku-format, not .haiku-format, as the file name.

Does anything speak against naming it "haiku-format.settings" or "haiku-format.config". That would seem to fit better with toher setting files.

Also, wouldn't it be better to use find_directory(B_USER_SETTINGS_DIRECTORY) instead of hard-coding the settings folder? Granted, it seems unlikely to change, but who knows...

I have noticed that the package name of haiku-format is haiku_format. Not sure it was on purpose or an oversight.

Package names aren't allowed dashes, they are replaced with underscores. I will, however, see to it's visible name in HaikuDepot showing "haiku-format".

humdingerb avatar Oct 14 '22 10:10 humdingerb

Does anything speak against naming it "haiku-format.settings" or "haiku-format.config". That would seem to fit better with toher setting files.

Also, wouldn't it be better to use find_directory(B_USER_SETTINGS_DIRECTORY) instead of hard-coding the settings folder? Granted, it seems unlikely to change, but who knows...

I had actually thought of both but settled on doing neither. I didn't use B_USER_SETTINGS_DIRECTORY because I usually develop haiku-format on a non-Haiku OS. As to the naming of the config file, I don't want to change the current clang-format behavior of looking for .clang-format (and if not found, then _clang-format) up the directory hierarchy. (The only reason that I renamed the executable and config files to haiku-format and _haiku-format etc. is to allow clang-format and haiku-format to coexist on Haiku.) It just feels weird to me to have config files named e.g. haiku-format.config in various source directories if one needs different formatting configurations than the settings in the default config file in ~/config/settings.

On second thought, you can keep the name as .haiku-format as there is at least one precedence: .lesshst. Alternatively, I can go with either haiku_format (similar to bash_history, time_dststatus, etc) or haiku-format (similar to wget-hsts).

Anyway, I want to minimize the diff between clang-format and haiku-format to ease maintainability.

owenca avatar Oct 15 '22 03:10 owenca

Alright. Let's keep it named .haiku-format to make it distinguishablefrom the executable.

Anyone, feel free to merge, if it looks OK.

humdingerb avatar Oct 15 '22 05:10 humdingerb