[tools] Fix the use of cached command-line variables in build_omim.sh #8710

2024-07-15T16:27:57Z

Tmpod commented

2024-07-15 16:27:57 +00:00

(Migrated from github.com)

Variables declared only through the command-line, such as BUILD_STANDALONE, assume their cached values (visible in CMakeCache.txt files) whenever they are not defined. This caused the build script to run in standalone mode even when you didn't want it to, as long as you passed the flag once before.
BUILD_DESIGNER didn't have the same problem because it is declared in the root CMakeList.txt and defaults to OFF. Should we also define the standalone variable there?

I also used this PR to extend our .gitignore files with more autogenerated stuff, mostly from CMake.

Variables declared only through the command-line, such as `BUILD_STANDALONE`, assume their cached values (visible in CMakeCache.txt files) whenever they are not defined. This caused the build script to run in standalone mode even when you didn't want it to, as long as you passed the flag once before. `BUILD_DESIGNER` didn't have the same problem because it is declared in the root `CMakeList.txt` and defaults to OFF. Should we also define the standalone variable there? I also used this PR to extend our `.gitignore` files with more autogenerated stuff, mostly from CMake.

biodranik (Migrated from github.com) reviewed 2024-07-15 16:50:36 +00:00

.gitignore

biodranik (Migrated from github.com) commented

2024-07-15 16:43:12 +00:00

tools/shaders_compiler is a valid directory. Why do you need this line in .gitignore?

.gitignore

				`@ -146,6 +146,7 @@ iphone/Maps/app.omaps/`

biodranik (Migrated from github.com) commented

2024-07-15 16:45:51 +00:00

Looks like the cmake is not used properly. All temporary files should be created in a different directory, not in the source directory. E.g.:

cd ..
mkdir omim-build-debug
cd omim-build-debug
cmake -DCMAKE_BUILD_TYPE=Debug -GNinja ../organicmaps/
ninja [optional_target]

Looks like the cmake is not used properly. All temporary files should be created in a different directory, not in the source directory. E.g.: ```bash cd .. mkdir omim-build-debug cd omim-build-debug cmake -DCMAKE_BUILD_TYPE=Debug -GNinja ../organicmaps/ ninja [optional_target] ```

biodranik (Migrated from github.com) commented

2024-07-15 16:46:14 +00:00

All these lines should not be needed if cmake is used properly from a different directory.

android/.gitignore

					
				@ -22,3 +22,5 @@ local.properties

				lint.xml

				.gradletasknamecache

				# Generated by Eclipse and JDTLS

biodranik (Migrated from github.com) commented

2024-07-15 16:46:34 +00:00

Which app creates this folder?

tools/unix/build_omim.sh

biodranik (Migrated from github.com) commented

2024-07-15 16:50:33 +00:00

Could it be a one-liner?
4 spaces indent for the same command is clearer.

1. Could it be a one-liner? 2. 4 spaces indent for the same command is clearer.

Tmpod (Migrated from github.com) reviewed 2024-07-15 16:57:29 +00:00

android/.gitignore

					
				@ -22,3 +22,5 @@ local.properties

				lint.xml

				.gradletasknamecache

				# Generated by Eclipse and JDTLS

Tmpod (Migrated from github.com) commented

2024-07-15 16:57:28 +00:00

This appeared back when I was working on my PR with an Android UI fix. Don't know exactly what caused it, but I assume it was some gradle build step?

Tmpod (Migrated from github.com) reviewed 2024-07-15 16:59:09 +00:00

.gitignore

				`@ -146,6 +146,7 @@ iphone/Maps/app.omaps/`

Tmpod (Migrated from github.com) commented

2024-07-15 16:59:09 +00:00

Right, I must have messed it up when I was trying to understand the macdeployqt issue, but shouldn't we ignore those anyway? Or is it used for important files?

Tmpod (Migrated from github.com) reviewed 2024-07-15 17:01:32 +00:00

tools/unix/build_omim.sh

Tmpod (Migrated from github.com) commented

2024-07-15 17:01:31 +00:00

I suppose it could, but it felt quite a long line (>100 chars long). The other comparably long line is one of the help ones.
Noted!

1. I suppose it could, but it felt quite a long line (>100 chars long). The other comparably long line is one of the help ones. 2. Noted!

Tmpod (Migrated from github.com) reviewed 2024-07-15 17:03:29 +00:00

.gitignore

Tmpod (Migrated from github.com) commented

2024-07-15 17:03:29 +00:00

Shoot, I missed that! I most likely got that directory from not building on another directory, like the rest.

biodranik (Migrated from github.com) reviewed 2024-07-15 17:24:45 +00:00

android/.gitignore

					
				@ -22,3 +22,5 @@ local.properties

				lint.xml

				.gradletasknamecache

				# Generated by Eclipse and JDTLS

biodranik (Migrated from github.com) commented

2024-07-15 17:24:45 +00:00

What's inside?

biodranik (Migrated from github.com) reviewed 2024-07-15 17:25:32 +00:00

tools/unix/build_omim.sh

biodranik (Migrated from github.com) commented

2024-07-15 17:25:32 +00:00

We're ok with 120 chars per line. It fits nicely into all modern monitors and the Github review window.

Tmpod (Migrated from github.com) reviewed 2024-07-15 17:28:40 +00:00

android/.gitignore

					
				@ -22,3 +22,5 @@ local.properties

				lint.xml

				.gradletasknamecache

				# Generated by Eclipse and JDTLS

Tmpod (Migrated from github.com) commented

2024-07-15 17:28:39 +00:00

Seems to be some preference files related to Eclipse (which I never used) and JDTLS (Eclipse's Java language server, which I use in my editor). We should probably keep it?

Seems to be some preference files related to Eclipse (which I never used) and [JDTLS](https://github.com/eclipse-jdtls/eclipse.jdt.ls) (Eclipse's Java language server, which I use in my editor). We should probably keep it?

Tmpod (Migrated from github.com) reviewed 2024-07-15 17:29:03 +00:00

tools/unix/build_omim.sh

Tmpod (Migrated from github.com) commented

2024-07-15 17:29:02 +00:00

Ah alright, will collapse into one line then.

biodranik (Migrated from github.com) reviewed 2024-07-15 18:00:15 +00:00

android/.gitignore

					
				@ -22,3 +22,5 @@ local.properties

				lint.xml

				.gradletasknamecache

				# Generated by Eclipse and JDTLS

biodranik (Migrated from github.com) commented

2024-07-15 18:00:15 +00:00

Yes, please add a comment why it's needed.

biodranik (Migrated from github.com) reviewed 2024-07-15 18:00:42 +00:00

.gitignore

				`@ -146,6 +146,7 @@ iphone/Maps/app.omaps/`

biodranik (Migrated from github.com) commented

2024-07-15 18:00:42 +00:00

Please remove them, using cmake in the same dir as the source is an anti-pattern.

biodranik (Migrated from github.com) approved these changes 2024-07-26 14:25:26 +00:00

biodranik (Migrated from github.com) left a comment

Thanks!

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik

No labels

Android Automotive (AAOS)

API

AppGallery

AppStore

Battery and Performance

External Map Datasets

F-Droid

Fonts

Frequently User Reported

No milestone

No project

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#8710

Rows
Columns