OSGeo / grass

GRASS GIS - free and open-source geospatial processing engine
https://grass.osgeo.org
Other
823 stars 301 forks source link

[Bug] r.colors/v.colors module doesn't contains all colors options thumbnails #3038

Open tmszi opened 1 year ago

tmszi commented 1 year ago

Describe the bug r.colors/v.colors module doesn't contains all colors options thumbnails.

To Reproduce Steps to reproduce the behavior:

  1. Launch r.colors/v.colors modules with --ui flag
  2. Switch to Define tab (page) and visually check Name of color table ComboBox widget color options
  3. Not all colors options have thumbnail (in the case when GRASS GIS was compiled with make -j option value > 1

Expected behavior All color options should has thumbnails by default.

System description (please complete the following information):

Additional context During compilation v.colors and r.colors module colors thumbnails are generated.

test@test~/src/grass$ grep -Rn "/utils/thumbnails.py"
raster/r.colors/Makefile:17:    -$(call run_grass, $(GRASS_HOME)/utils/thumbnails.py)
vector/v.colors/Makefile:15:    -$(call run_grass, $(GRASS_HOME)/utils/thumbnails.py)

Script for generation colors thumbnails https://github.com/OSGeo/grass/blob/main/utils/thumbnails.py used OS ENV vars which are mutually overwritten at compile time especially if make -j option value is > 1 and as a result, not all color options have a thumbnail.

Simple test:

  1. Compilation with make -j4
GRASS nc_basic_spm_grass7/PERMANENT:~ > ls $GISBASE/docs/html/colortables | wc -l
38

Plus visually checking r.colors --ui/v.colors --ui modules Name of color table ComboBox widget color options thumbnails. Not all colors options have thumbnails.

  1. Compilation with make -j1
GRASS nc_basic_spm_grass7/PERMANENT:~ > ls $GISBASE/docs/html/colortables | wc -l
56

Plus visually checking r.colors --ui/v.colors --ui modules Name of color table ComboBox widget color options thumbnails. All colors options have thumbnails.

Bug was originally reported here.

petrasovaa commented 11 months ago

Would it be enough to remove the thumbnail call from v.colors Makefile? I don't think there is any reason why v.colors should be compiling it again.

wenzeslaus commented 11 months ago

It should be probably a separate target or whatever these units are called in Makefiles which both v.colors and r.colors would depend on (or not), but compiling them only for one seems like a reasonable fix.

Good catch BTW, I observed some strange behavior, hopefully it will be fixed by that.

tmszi commented 8 months ago

During my testing, I found this line of code problematic during compilation (if the value of the make -j option > 1), but no error is printed.

https://github.com/OSGeo/grass/blob/5e86482fd9ad7f1bdb874e23740e1ca6b0b9b896/utils/thumbnails.py#L117-L149

If I increase the time between calls grass.write_command("d.graph", ... with time.sleep(0.5) in the for loop, bug disappear during compilation. I tested the compilation 10 times in a row with 100% success in generating all color palettes.

         time.sleep(0.5)
         grass.write_command( 
             "d.graph", 
             quiet=True, 
             flags="m", 
             stdin=""" 
         width 1 
         color {outcolor} 
         polyline 
         {x1} {y1} 
         {x2} {y1} 
         {x2} {y2} 
         {x1} {y2} 
         {x1} {y1} 
         color {incolor} 
         polyline 
         {x3} {y3} 
         {x4} {y3} 
         {x4} {y4} 
         {x3} {y4} 
         {x3} {y3} 
         """.format( 
                 x1=1, 
                 x2=width, 
                 y1=0, 
                 y2=height - 1, 
                 x3=2, 
                 x4=width - 1, 
                 y3=1, 
                 y4=height - 2, 
                 outcolor="white", 
                 incolor="black", 
             ), 
         ) 
petrasovaa commented 8 months ago

I don't see anything particularly wrong about this code, adding sleep might help, but I don't think it's solving the source of the problem, which I believe is in the makefiles.

nilason commented 8 months ago

I cannot reproduce this on Mac with make -j8, all thumbnails are nailed.

echoix commented 8 months ago

I think I hit a concurrency issue in the makesfiles last week on windows ci testing. With make -j, two times straight the was a compilation error (a file not found or something, but not this module), and didn't fail with make -j2 nor make -j4. After that I retried a third time with make -j, changing only some comments on a workflow (after the build step, so no changes) it didn't fail. At that point there wasn't any changes to the compilation that could've impacted that.

So I dismissed it, thinking it was maybe a interdependency that isn't correctly defined that allows a job to run before it should.

tmszi commented 8 months ago

I cannot reproduce this on Mac with make -j8, all thumbnails are nailed.

The bug does not appear on every compilation, but only on non-specific conditions, even if -j > 1 is used.

tmszi commented 8 months ago

I don't see anything particularly wrong about this code, adding sleep might help, but I don't think it's solving the source of the problem, which I believe is in the makefiles.

Yes, of course, the code looks normal, but the loop is not executed 56 times, but only 38 times, if I comment it out, the loop is executed 56 times each time (during compilation).

echoix commented 8 months ago

If there isn't any chance of env vars getting used for the bad iteration, and that the files are correctly made in the python code, then I could suggest to try to find out the issues with missing g dependencies in the makefile.

To try this, using make version 4.4 or later, you can add do make -j --shuffle/make -j --shuffle=random or any -j you want. It will keep the target/prerequisite relationships, but will purposefully reorder the goals and prerequisites. The random seed used is printed in the error messages and can be reused with --shuffle=seed. If we can find at least a seed that breaks, we could be able to investigate closely. Then use --debug=p to completely show the recipes to be executed, even with @.

https://www.gnu.org/software/make/manual/html_node/Options-Summary.html

https://trofi.github.io/posts/238-new-make-shuffle-mode.html

Another linked post, older, and shows some of the fixes that were needed in parallel builds problems (in makefiles) https://trofi.github.io/posts/230-when-make-j-nproc-fails.html

Is there a way to know easily (perhaps without user intervention) when that error is triggered? Something that can be called in bash after the make to check it. If so, we could do a loop for a couple (fixed) iterations, check if it worked, clean, and start over until we find it.

echoix commented 8 months ago

However, GNU make 4.4 (or the latest 4.4.1) isn't available for Ubuntu or Debian, it is available for mysys2 (for the windows builds), Fedora 38+, and Homebrew.

https://repology.org/project/make/versions

I've already got a shortlist of possible problematic folders, even on first try when adding it to the CI of my current work.

I've also added it my Ubuntu-based CI for pytest (that runs 3 times) through homebrew, and it also failed quite early, thus found some problems.

This caused at least one failure https://github.com/OSGeo/grass/blob/e6a37711d2d61957d2d9c83acfe2754ed313adc1/include/Make/Html.make#L13-L14

/usr/bin/install -c -m 644 tplot/g.gui.tplot.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/tplot/g.gui.tplot.py
make[3]: *** [Makefile:41: /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/vdigit/g.gui.vdigit.pyc] Error 1 shuffle=277380116
make[3]: *** Waiting for unfinished jobs....
/usr/bin/install: cannot stat 'tplot/__init__.pyc': No such file or directory
make[3]: *** [Makefile:41: /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/tplot/__init__.pyc] Error 1 shuffle=277380116

Started compilation: Tue Jan  9 00:07:22 UTC 2024
--
Errors in:
/home/runner/work/grass/grass/python/grass/app
/home/runner/work/grass/grass/python/grass/benchmark
/home/runner/work/grass/grass/python/grass/grassdb
/home/runner/work/grass/grass/python/grass/gunittest
/home/runner/work/grass/grass/python/grass/imaging
/home/runner/work/grass/grass/python/grass/jupyter
/home/runner/work/grass/grass/python/grass/pydispatch
/home/runner/work/grass/grass/python/grass/pygrass
/home/runner/work/grass/grass/python/grass/script
/home/runner/work/grass/grass/python/grass/semantic_label
/home/runner/work/grass/grass/python/grass/temporal
/home/runner/work/grass/grass/python/grass/utils
/home/runner/work/grass/grass/scripts/g.download.location
/home/runner/work/grass/grass/scripts/r.in.wms
/home/runner/work/grass/grass/gui/wxpython
--

/usr/bin/install -c -m 644 __init__.pyc /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/benchmark/__init__.pyc
/usr/bin/install -c -m 644 __main__.pyc /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/benchmark/__main__.pyc
/usr/bin/install: cannot stat '__init__.pyc': No such file or directory
make[5]: *** [Makefile:19: /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/benchmark/__init__.pyc] Error 1 shuffle=280083930
make[5]: *** Waiting for unfinished jobs....
/usr/bin/install: cannot stat '__main__.pyc': No such file or directory
make[5]: *** [Makefile:19: /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/benchmark/__main__.pyc] Error 1 shuffle=280083930
make[5]: Leaving directory '/home/runner/work/grass/grass/python/grass/benchmark'
make[5]: Entering directory '/home/runner/work/grass/grass/python/grass/exceptions'
mkdir -p /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/exceptions
python3 -t -m py_compile __init__.py
python3 -t -m py_compile /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/__init__.py
/usr/bin/install -c -m 644 __init__.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/exceptions/__init__.py
/usr/bin/install -c -m 644 __init__.pyc /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/exceptions/__init__.pyc
/usr/bin/install: cannot stat '__init__.pyc': No such file or directory
make[5]: *** [Makefile:27: /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/exceptions/__init__.pyc] Error 1 shuffle=280083930
make[5]: Leaving directory '/home/runner/work/grass/grass/python/grass/exceptions'
make[5]: Entering directory '/home/runner/work/grass/grass/python/grass/grassdb'
mkdir -p /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb
python3 -t -m py_compile create.py
python3 -t -m py_compile checks.py
/usr/bin/install -c -m 644 checks.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/checks.py
/usr/bin/install -c -m 644 data.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/data.py
/usr/bin/install -c -m 644 config.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/config.py
/usr/bin/install -c -m 644 manage.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/manage.py
/usr/bin/install -c -m 644 create.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/create.py
/usr/bin/install -c -m 644 history.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/history.py
/usr/bin/install -c -m 644 __init__.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/__init__.py
python3 -t -m py_compile /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/manage.py
python3 -t -m py_compile /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/data.py
python3 -t -m py_compile /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/config.py
python3 -t -m py_compile /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/history.py
python3 -t -m py_compile /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/__init__.py
/usr/bin/install -c -m 644 create.pyc /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/create.pyc
/usr/bin/install: cannot stat 'create.pyc': No such file or directory
make[5]: *** [Makefile:19: /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/grassdb/create.pyc] Error 1 shuffle=280083930
make[5]: *** Waiting for unfinished jobs....
make[5]: Leaving directory '/home/runner/work/grass/grass/python/grass/grassdb'

On windows, I resaw my first failure that I was referring to, I remember it was in gui/scripts

--------------------------------------------------
Started compilation: Mon, Jan  8, 2024 11:10:05 PM
--
Errors in:
/d/a/grass/grass/python/grass/app
/d/a/grass/grass/python/grass/benchmark
/d/a/grass/grass/python/grass/exceptions
/d/a/grass/grass/python/grass/grassdb
/d/a/grass/grass/python/grass/gunittest
/d/a/grass/grass/python/grass/imaging
/d/a/grass/grass/python/grass/jupyter
/d/a/grass/grass/python/grass/pydispatch
/d/a/grass/grass/python/grass/pygrass/messages
/d/a/grass/grass/python/grass/pygrass/modules/interface
/d/a/grass/grass/python/grass/pygrass/modules/grid
/d/a/grass/grass/python/grass/pygrass/raster
/d/a/grass/grass/python/grass/pygrass/vector
/d/a/grass/grass/python/grass/script
/d/a/grass/grass/python/grass/temporal
/d/a/grass/grass/python/grass/utils
/d/a/grass/grass/scripts/r.in.wms
/d/a/grass/grass/gui/scripts
/d/a/grass/grass/gui/wxpython
--

On windows, another shuffle run failed at more places this time:


--------------------------------------------------
Started compilation: Tue, Jan  9, 2024 12:10:05 AM
--
Errors in:
/d/a/grass/grass/python/grass/app
/d/a/grass/grass/python/grass/benchmark
/d/a/grass/grass/python/grass/grassdb
/d/a/grass/grass/python/grass/gunittest
/d/a/grass/grass/python/grass/imaging
/d/a/grass/grass/python/grass/jupyter
/d/a/grass/grass/python/grass/pydispatch
/d/a/grass/grass/python/grass/pygrass
/d/a/grass/grass/python/grass/script
/d/a/grass/grass/python/grass/semantic_label
/d/a/grass/grass/python/grass/temporal
/d/a/grass/grass/python/grass/utils
/d/a/grass/grass/scripts/v.db.join
/d/a/grass/grass/scripts/db.droptable
/d/a/grass/grass/scripts/i.in.spotvgt
/d/a/grass/grass/scripts/r.reclass.area
/d/a/grass/grass/scripts/r.rgb
/d/a/grass/grass/scripts/v.db.addtable
/d/a/grass/grass/scripts/v.db.update
/d/a/grass/grass/scripts/r.in.aster
/d/a/grass/grass/scripts/db.dropcolumn
/d/a/grass/grass/scripts/g.extension.all
/d/a/grass/grass/scripts/v.db.droprow
/d/a/grass/grass/scripts/r.blend
/d/a/grass/grass/scripts/v.rast.stats
/d/a/grass/grass/scripts/wxpyimgview
/d/a/grass/grass/scripts/r.in.wms
/d/a/grass/grass/scripts/i.pansharpen
/d/a/grass/grass/scripts/d.rast.leg
/d/a/grass/grass/scripts/i.tasscap
/d/a/grass/grass/scripts/r.plane
/d/a/grass/grass/scripts/r.mask
/d/a/grass/grass/scripts/v.clip
/d/a/grass/grass/scripts/i.colors.enhance
/d/a/grass/grass/scripts/v.db.droptable
/d/a/grass/grass/scripts/v.to.lines
/d/a/grass/grass/scripts/r.semantic.label
/d/a/grass/grass/scripts/i.band.library
/d/a/grass/grass/scripts/v.db.renamecolumn
/d/a/grass/grass/scripts/v.db.dropcolumn
/d/a/grass/grass/scripts/i.image.mosaic
/d/a/grass/grass/scripts/db.in.ogr
/d/a/grass/grass/scripts/v.db.addcolumn
/d/a/grass/grass/scripts/g.extension
/d/a/grass/grass/scripts/r.in.srtm
/d/a/grass/grass/scripts/v.report
/d/a/grass/grass/scripts/r.grow
/d/a/grass/grass/scripts/db.univar
/d/a/grass/grass/scripts/r.fillnulls
/d/a/grass/grass/scripts/v.db.reconnect.all
/d/a/grass/grass/scripts/v.in.geonames
/d/a/grass/grass/scripts/v.centroids
/d/a/grass/grass/scripts/d.background
/d/a/grass/grass/scripts/d.rast.edit
/d/a/grass/grass/scripts/g.download.location
/d/a/grass/grass/scripts/v.db.univar
/d/a/grass/grass/scripts/r.buffer.lowmem
/d/a/grass/grass/scripts/r.mapcalc.simple
/d/a/grass/grass/scripts/r.colors.stddev
/d/a/grass/grass/scripts/v.dissolve
/d/a/grass/grass/scripts/r.import
/d/a/grass/grass/scripts/v.import
/d/a/grass/grass/scripts/v.what.strds
/d/a/grass/grass/scripts/r.drain
/d/a/grass/grass/temporal/t.info
/d/a/grass/grass/temporal/t.rast.neighbors
/d/a/grass/grass/temporal/t.remove
/d/a/grass/grass/temporal/t.rast3d.algebra
/d/a/grass/grass/temporal/t.topology
/d/a/grass/grass/temporal/t.merge
/d/a/grass/grass/temporal/t.rast.list
/d/a/grass/grass/temporal/t.rast.import
/d/a/grass/grass/temporal/t.vect.observe.strds
/d/a/grass/grass/temporal/t.rast.univar
/d/a/grass/grass/temporal/t.vect.extract
/d/a/grass/grass/temporal/t.create
/d/a/grass/grass/temporal/t.list
/d/a/grass/grass/temporal/t.rast3d.list
/d/a/grass/grass/temporal/t.rast.extract
/d/a/grass/grass/temporal/t.rast.algebra
/d/a/grass/grass/temporal/t.rast3d.mapcalc
/d/a/grass/grass/temporal/t.vect.export
/d/a/grass/grass/temporal/t.copy
/d/a/grass/grass/temporal/t.rast3d.univar
/d/a/grass/grass/temporal/t.rast.to.vect
/d/a/grass/grass/temporal/t.rast.what
/d/a/grass/grass/temporal/t.rast.aggregate.ds
/d/a/grass/grass/temporal/t.sample
/d/a/grass/grass/temporal/t.rast.out.vtk
/d/a/grass/grass/temporal/t.rename
/d/a/grass/grass/temporal/t.shift
/d/a/grass/grass/temporal/t.rast.export
/d/a/grass/grass/temporal/t.rast.accdetect
/d/a/grass/grass/temporal/t.rast.mapcalc
/d/a/grass/grass/temporal/t.rast.accumulate
/d/a/grass/grass/temporal/t.vect.db.select
/d/a/grass/grass/temporal/t.select
/d/a/grass/grass/temporal/t.unregister
/d/a/grass/grass/temporal/t.rast.contour
/d/a/grass/grass/temporal/t.rast.gapfill
/d/a/grass/grass/temporal/t.vect.list
/d/a/grass/grass/temporal/t.rast.to.rast3
/d/a/grass/grass/temporal/t.vect.algebra
/d/a/grass/grass/temporal/t.snap
/d/a/grass/grass/temporal/t.rast.colors
/d/a/grass/grass/temporal/t.register
/d/a/grass/grass/temporal/t.vect.univar
/d/a/grass/grass/temporal/t.upgrade
/d/a/grass/grass/temporal/t.rast.series
/d/a/grass/grass/temporal/t.support
/d/a/grass/grass/temporal/t.vect.import
/d/a/grass/grass/temporal/t.rast3d.extract
/d/a/grass/grass/temporal/t.rast.aggregate
/d/a/grass/grass/temporal/t.vect.what.strds
/d/a/grass/grass/gui/scripts
/d/a/grass/grass/gui/wxpython
--

So, it's starting to show a trend, if it was makefile-only problem, it would be in a Python-related makefile. It doesn't seem limited to one specific subdirectory.

tmszi commented 8 months ago

Compilation always happens without errors under GNU/Linux Gentoo distribution, with gcc (Gentoo 13.2.1_p20230826 p7) 13.2.1 20230826).

During r.colors module compilation all dependencies are always complete, r.mapcalc module is compiled and lib/gis is also compiled including copying all colors file definition to the correct directory (thumbnails.py file script: color_dir = os.path.join(os.environ["GISBASE"], "etc", "colors")) before running the utils/thumbnails.py script (line 17).

Makefile of r.colors module:

https://github.com/OSGeo/grass/blob/e6a37711d2d61957d2d9c83acfe2754ed313adc1/raster/r.colors/Makefile#L1-L26