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

Meson uninstalled attempt 2 #1793

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

infirit
Copy link
Contributor

@infirit infirit commented Jul 22, 2022

This appears to work for me. Yes it copies all the files to the build dir but for now I don't care. I just want it to work so we can drop autotools. If someone want to spend the time to figure out other options they can and send a PR.

It needs some testing and I will have to update the release job in main.

edit. I'll also fix the other jobs over the weekend.

@infirit infirit force-pushed the uninstalledv2 branch 19 times, most recently from fa1dbdc to 9b4ecda Compare July 22, 2022 20:28
@infirit infirit requested a review from cschramm July 22, 2022 20:29
@infirit
Copy link
Contributor Author

infirit commented Jul 22, 2022

Fixed the jobs.

Just one concern/question, the release job version check command works when I run it in a terminal. Not sure it'll work in CI 🤔

@sonarcloud
Copy link

sonarcloud bot commented Jul 23, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@infirit
Copy link
Contributor Author

infirit commented Jul 23, 2022

Fixed per comments.

@cschramm
Copy link
Member

To be honest, I'm not super happy with the workflow yet, as a debug cycle now means editing the sources, syncing them into the build directory with ninja and setting breakpoints etc. in the build directory, thus in a different context, but I guess I'll get used to it.

One idea: Shouldn't it be rather easy to reliably detect that an app is running from a build directory instead of using BLUEMAN_SOURCE? It should e.g. find meson-info next to it.

@@ -63,9 +66,12 @@ jobs:
steps:
- uses: actions/checkout@v2
- run: apt-get update
- run: apt-get install -y -qq --no-install-recommends automake autoconf libtool autopoint gettext libglib2.0-dev python-gi-dev libbluetooth-dev iproute2 libgirepository1.0-dev gir1.2-gtk-3.0 gir1.2-nm-1.0 libpulse0 libpulse-mainloop-glib0
- run: apt-get install -y -qq --no-install-recommends meson gettext gettext libglib2.0-dev python-gi-dev libbluetooth-dev iproute2 libgirepository1.0-dev gir1.2-gtk-3.0 gir1.2-nm-1.0 libpulse0 libpulse-mainloop-glib0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gettext got duplicated here.

@@ -1,4 +1,7 @@
cython = find_program('cython', required: true)
cython = find_program('cython', required: false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_program takes additional arguments as fallbacks, so find_program('cython', 'cython3', required: false) should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, didn't know that. I'll fix that.

ICON_PATH = os.path.join(_dirname, 'data', 'icons')
PIXMAP_PATH = os.path.join(_dirname, 'data', 'icons', 'pixmaps')
UI_PATH = os.path.join(_dirname, 'data', 'ui')
_builddir = os.path.abspath(os.path.dirname(__file__))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A .. is missing here, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least launch is looking at the wrong BIN_DIR for me, so that it does not find the apps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No .. is intentional but I'll check why your bindir is incorrect. What is your build dir named?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name was build and BIN_DIR was pointing to build/blueman which seems very plausible to me as that's where Constants.py is and what I thus expect from os.path.dirname(__file__).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was wrong, will fix. I confused this with the apps.

@infirit
Copy link
Contributor Author

infirit commented Jul 25, 2022

We could drop Constants for another way to configure paths. I could move these things to a JSON file and allow the apps to load a custom one on the command line.

@cschramm
Copy link
Member

cschramm commented Jul 25, 2022

Totally forgot to add that README.md and Dependencies.md need updates.

@eli-schwartz
Copy link

We could drop Constants for another way to configure paths. I could move these things to a JSON file and allow the apps to load a custom one on the command line.

I commented here about some possible solutions: #1207 (comment)

Loading a json file from the command line doesn't seem like the best solution to me as meson devenv is supposed to make things work without command line options, usually via environment variables.

Copy link

sonarcloud bot commented Mar 1, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

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

Successfully merging this pull request may close these issues.

3 participants