Add optional buildscripts #370

ghost · 2024-01-05T09:09:15Z

Implements #367

ghost · 2024-01-05T09:11:45Z

NOTE : THIS IS STILL UNFINISHED

EDIT : THIS IS NOW MOSTLY FINISHED

EDIT2 : THIS IS NOW 99.9% FINISHED

It works perfectly fine when run using chmod +x make-pkgs.sh && ./make-pkgs.sh --ibus --fedora --develop --toolbox
But there are still alot of unfixed issues and edgecases
EDIT : MOST HAVE BEEN IRONED OUT

Notably :

missing debian buildscript
bugs out with error fatal: destination path 'OpenBangla-Keyboard' already exists and is not an empty directory. on subsequent runs.

ghost · 2024-01-05T09:14:56Z

* bugs out with error `fatal: destination path 'OpenBangla-Keyboard' already exists and is not an empty directory.` on subsequent runs.

On that note, the fatal error occurs somewhere near line 11 of make-fedora.sh on subsequent runs
I can't seem to find why it occurs, as the check should result in either entering the directory and doing a git pull or git cloning OBK if the directory doesnt exist, if anyone can help it would be appreciated.

EDIT : I think it has something to do with the git pull command expecting an empty directory or something similar, I am not experienced in dealing with git, so I can't fix the error in this case

EDIT2 : Fixed the bug, it now works fine

mominul · 2024-01-06T09:53:19Z

@ahmubashshir can you take a look?

@larina3315 Have you checked the scripts with shellcheck?

ghost · 2024-01-06T16:30:18Z

@ahmubashshir can you take a look?

@larina3315 Have you checked the scripts with shellcheck?

Just checked, some suggestions showed up, I will update my scripts accordingly.
I wanted to make a prototype/PoC ASAP, there are some things that can be improved, I will continue working on them

ghost · 2024-01-06T19:11:49Z

Script is finished

Added documentation for most variables and what they do
Added documentation on how to add support for other distros
ensured there are no more shellcheck errors/recommendations remaining
Confirmed script works as described in the docs with proper behaviour by running
chmod +x make-pkgs.sh && ./make-pkgs.sh --ibus --fcitx --debian 11 --fedora 37 --toolbox --develop
chmod +x make-pkgs.sh && ./make-pkgs.sh --ibus --fcitx --debian 11 --fedora 37 --develop
chmod +x make-pkgs.sh && ./make-pkgs.sh --clean
chmod +x make-pkgs.sh && ./make-pkgs.sh --help
chmod +x make-pkgs.sh && ./make-pkgs.sh --hello

ahmubashshir · 2024-01-07T15:13:29Z

@larina3315 try amending the commits with meaningful messages.

https://www.conventionalcommits.org/en/v1.0.0/
https://github.com/joelparkerhenderson/git-commit-message/

Initial commit of the script.

Refactor and Revise the script

Add Debian support Misc. revisions to Fedora script for toolbox --help switch for accessing help information --clean switch for cleaning toolbox containers and ~/.obk-build/

Last commit before major changes

Add comments explaining what different variables do, how to add support for new distros. Some misc revisions.

directory, besides the shell scripts

ghost · 2024-01-07T18:06:21Z

@larina3315 try amending the commits with meaningful messages.

https://www.conventionalcommits.org/en/v1.0.0/ https://github.com/joelparkerhenderson/git-commit-message/

I am extremely sorry, I am still inexperienced in working with git.
I have tried my best to squash related commits and give them better commit messages.
I would like to edit the commit messages of some more commits, but so far neither Github nor Github Desktop seems to have the option to edit the commit message of a commit that was already pushed.

As for reviewing the PR, I would suggest focusing on/after Some cleanup and restructuring
As it was a hefty restructure and reorganization of the entire thing, with the scripts being moved into their own folder

mominul · 2024-01-07T19:00:12Z

@larina3315 Okay the commits are good enough and I can squash them when merging this PR.

As you intend it to be a user-facing tool, there should be documentation about it. Can you add basic information about how should a user run it in the Readme.md file?

ghost · 2024-01-07T19:06:46Z

Can you add basic information about how should a user run it in the Readme.md file?

yes, I will add a readme.md file for it asap.

Create a readme for the tool

ahmubashshir

Good work btw 👍🏽 I'll try to add other distro and architecture support on this...

tools/make-pkgs/make-build.sh

ahmubashshir · 2024-01-09T17:21:35Z

tools/make-pkgs/make-pkgs.sh

+    echo "Usage: $0 [OPTIONS]"
+    echo "Options:"
+    echo "  --ibus          Compile IBus version"
+    echo "  --fcitx         Compile Fcitx version (Doesn't work for OBK2 and earlier)"
+    echo "  --develop       Compile from the develop branch"
+    echo "  --fedora        Compile for Fedora"
+    echo "  --debian        Compile for Debian"
+    echo "  --toolbox       Compile inside of a toolbox"
+    echo "  --clean         Clean \"$FILE_DIR_OBK\" directory and remove obk-toolbox-* containers"
+    echo "  --help          Display this help message"
+    echo " "
+    echo "  -> NOTE: User needs to specify the --toolbox flag to compile for multiple distros at the same time."
+    echo "           Otherwise, only one flag can be used at a time (e.g. --fedora and --debian can't be used together without --toolbox)"
+    echo "  -> NOTE: When compiling inside of a toolbox, an additional <version> can be specified for the distro flags."
+    echo "           (e.g. --fedora 37) This will generate packages for a specific version of a distro."
+    echo "Example: "
+    echo "  $0 --ibus --fcitx --develop --fedora 37 --debian --toolbox"
+    echo "  $0 --ibus --develop --debian"


Instead of using multiple echos, try using

cat <<EOF ... EOF

Unless absolutely mandatory, I would personally like to avoid the change, as using the echos make the script more readable and understandable compared to using cat <<EOF

mominul · 2024-01-11T18:44:50Z

tools/make-pkgs/readme.md

+For example, If you are on Debian and you want to generate `.rpm`s specifically for Fedora 37, you can do so.
+
+## How to use
+Copy this folder, make the make-pkgs.sh script executable by doing `chmod +x make-pkgs.sh`


@larina3315 why do we need to copy this folder?

I was writing this from the POV that if a user clones the OBK git repo with this tool in it, then the make-pkgs.sh file will reside in ./OpenBangla-Keyboard/tools/make-pkgs/ , so the user might want to copy the folder to some other conveinent location and run it from there.

It could be worded in a better way as a suggestion.

Add toolbox-related changes. Cleaned up the markdown. Restructured Howto and Examples sections.

Add bash brackets Fixed behaviour of --clean flag

ghost · 2024-01-12T07:50:03Z

I would like to ask for suggestion on whether stdout/stderr output of things like toolbox/dnf/apt etc should be

suppressed
silently logged to a file
be shown to the user (current behaviour)
be shown ONLY on faliure

ahmubashshir · 2024-01-12T16:28:22Z

Let's keep current behavior as default and put others behind optiol… like --log-output=<file|always(default)|hide|on-error>

ghost · 2024-01-12T17:41:29Z

Let's keep current behavior as default and put others behind optiol… like --log-output=<file|always(default)|hide|on-error>

The user would be bombarded with random things happening on screen, as long as things are going fine why bother the user ?
I was thinking about making show on error the default

Although adding logging options can surely be done

mominul · 2024-01-12T18:01:36Z

I would like to ask for suggestion on whether stdout/stderr output of things like toolbox/dnf/apt etc should be
* suppressed

* silently logged to a file

* be shown to the user (current behaviour)

* be shown ONLY on faliure

verbose output should be behind a -v flag. But we can inform the user about the successes of the stages by outputting like Finished setting up dependencies, Successfully built package...etc.

ghost · 2024-01-12T18:06:18Z

I would like to ask for suggestion on whether stdout/stderr output of things like toolbox/dnf/apt etc should be
* suppressed

* silently logged to a file

* be shown to the user (current behaviour)

* be shown ONLY on faliure
verbose output should be behind a -v flag. But we can inform the user about the successes of the stages by outputting like Finished setting up dependencies, Successfully built package...etc.

I am currently looking into ways to introduce logging to bash scripts, although this change will take time as I will have to take a temporary hiatus due to academic reasons soon.

EDIT : Using set -x will log every command the shell tries to run

move initial functions and vars into function.bash

-v must be first arg ./make-pkgs.sh -v --xyz WARNING : CURRENTLY BROKEN

-v flag works correctly

ghost · 2024-01-12T22:25:45Z

There can be some more QoL enchancements, code cleanups and documentation effort, but I feel like make-pkgs has reached it's intended state, adding more features will take it out of it's original scope of being an enduser focused tool

distrobox/docker support can also be easily added should anyone want to do such a thing, toolbox is a wrapper around podman so it already has podman support

ghost · 2024-01-13T08:17:48Z

Due to limitations of bash, I sadly cannot suppress log output for toolbox/dnf/cpack/git etc
But the -v flag, when used as the first argument, will print and log every single line bash executes alongside stdout and stderr, this way if something does fail, we can just ask users to upload the *.log files

This is the best that could be done given the current circumstances, I had to put ALOT of ugly hacks to make it work.
Ideally, I would've just written a rust program but the point of this is to be as portable as possible ("one-click") without platform/distro-specific issues, thus I had to use bash.

mominul · 2024-01-13T11:42:16Z

@larina3315 I appreciate your effort on this, thanks! ❤️

@ahmubashshir Do you have any review comments to add? Otherwise, I'd like to merge it.

ghost · 2024-01-13T13:55:15Z

@larina3315 I appreciate your effort on this, thanks! ❤️

@ahmubashshir Do you have any review comments to add? Otherwise, I'd like to merge it.

I have to write some misc. documentation regarding adding distro support and variables, some slight behaviour has changed which need to be mentioned in the readme

EDIT : I have finished doing appropriate changes

ahmubashshir · 2024-01-14T07:30:48Z

On Sat, Jan 13 2024 at 03:42:27 AM -08:00:00, Muhammad Mominul Huque ***@***.***> wrote: @ahmubashshir Do you have any review comments to add? Otherwise, I'd like to merge it.

LGTM, you can merge it. - Mubashshir

mominul

Thanks!

larina3315 and others added 6 commits January 7, 2024 23:19

Add initial commit

7802064

Initial commit of the script.

Refactor and Revise the script

35fb471

Refactor and Revise the script

Add Debian support, --help and --clean

705f56d

Add Debian support Misc. revisions to Fedora script for toolbox --help switch for accessing help information --clean switch for cleaning toolbox containers and ~/.obk-build/

Some cleanup and restructuring

131c869

Last commit before major changes

Add doumentation

d50209d

Add comments explaining what different variables do, how to add support for new distros. Some misc revisions.

script now copies packages to a ./builds/

e53cdd9

directory, besides the shell scripts

larina3315 added 2 commits January 8, 2024 02:18

Create readme.md

4fab8a8

Create a readme for the tool

Reorganize the NOTE section

3587c70

ahmubashshir approved these changes Jan 9, 2024

View reviewed changes

mominul reviewed Jan 11, 2024

View reviewed changes

larina3315 and others added 2 commits January 12, 2024 12:49

Minor readme edit

f7e95ac

Add toolbox-related changes. Cleaned up the markdown. Restructured Howto and Examples sections.

Add bash brackets

2063dbf

Add bash brackets Fixed behaviour of --clean flag

larina3315 added 2 commits January 12, 2024 14:02

Revise readme

9d87d9c

Revise readme

acae229

larina3315 added 2 commits January 13, 2024 03:27

Refactor code

5bbd5df

move initial functions and vars into function.bash

Added debugging support with -v

a16d6f8

-v must be first arg ./make-pkgs.sh -v --xyz WARNING : CURRENTLY BROKEN

Fix debugging

05b690d

-v flag works correctly

larina3315 added 3 commits January 14, 2024 10:45

Add more docs

e71ab71

Revise --help

d24dbd8

Revise new distro instructions

0bfcada

mominul approved these changes Jan 14, 2024

View reviewed changes

mominul merged commit 7989cc0 into OpenBangla:master Jan 14, 2024
0 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional buildscripts #370

Add optional buildscripts #370

ghost commented Jan 5, 2024

ghost commented Jan 5, 2024 •

edited by ghost

Loading

ghost commented Jan 5, 2024 •

edited by ghost

Loading

mominul commented Jan 6, 2024

ghost commented Jan 6, 2024

ghost commented Jan 6, 2024 •

edited by ghost

Loading

ahmubashshir commented Jan 7, 2024

ghost commented Jan 7, 2024

mominul commented Jan 7, 2024

ghost commented Jan 7, 2024

ahmubashshir left a comment

ahmubashshir Jan 9, 2024

ghost Jan 10, 2024

mominul Jan 11, 2024

ghost Jan 12, 2024

ghost commented Jan 12, 2024

ahmubashshir commented Jan 12, 2024

ghost commented Jan 12, 2024 •

edited by ghost

Loading

mominul commented Jan 12, 2024

ghost commented Jan 12, 2024 •

edited by ghost

Loading

ghost commented Jan 12, 2024 •

edited by ghost

Loading

ghost commented Jan 13, 2024

mominul commented Jan 13, 2024

ghost commented Jan 13, 2024 •

edited by ghost

Loading

ahmubashshir commented Jan 14, 2024 via email

mominul left a comment

Add optional buildscripts #370

Add optional buildscripts #370

Conversation

ghost commented Jan 5, 2024

ghost commented Jan 5, 2024 • edited by ghost Loading

NOTE : THIS IS STILL UNFINISHED

EDIT : THIS IS NOW MOSTLY FINISHED

EDIT2 : THIS IS NOW 99.9% FINISHED

ghost commented Jan 5, 2024 • edited by ghost Loading

mominul commented Jan 6, 2024

ghost commented Jan 6, 2024

ghost commented Jan 6, 2024 • edited by ghost Loading

Script is finished

ahmubashshir commented Jan 7, 2024

ghost commented Jan 7, 2024

mominul commented Jan 7, 2024

ghost commented Jan 7, 2024

ahmubashshir left a comment

Choose a reason for hiding this comment

ahmubashshir Jan 9, 2024

Choose a reason for hiding this comment

ghost Jan 10, 2024

Choose a reason for hiding this comment

mominul Jan 11, 2024

Choose a reason for hiding this comment

ghost Jan 12, 2024

Choose a reason for hiding this comment

ghost commented Jan 12, 2024

ahmubashshir commented Jan 12, 2024

ghost commented Jan 12, 2024 • edited by ghost Loading

mominul commented Jan 12, 2024

ghost commented Jan 12, 2024 • edited by ghost Loading

ghost commented Jan 12, 2024 • edited by ghost Loading

ghost commented Jan 13, 2024

mominul commented Jan 13, 2024

ghost commented Jan 13, 2024 • edited by ghost Loading

ahmubashshir commented Jan 14, 2024 via email

mominul left a comment

Choose a reason for hiding this comment

ghost commented Jan 5, 2024 •

edited by ghost

Loading

ghost commented Jan 5, 2024 •

edited by ghost

Loading

ghost commented Jan 6, 2024 •

edited by ghost

Loading

ghost commented Jan 12, 2024 •

edited by ghost

Loading

ghost commented Jan 12, 2024 •

edited by ghost

Loading

ghost commented Jan 12, 2024 •

edited by ghost

Loading

ghost commented Jan 13, 2024 •

edited by ghost

Loading