gnuplot-beta — Developers and beta testers of new gnuplot code

Re: [PATCH] Support building development version without git

[Henri M. <hen...@gm...>]

On 01/07/20, 17:15, Dima Kogan wrote:
> Henri Menke <hen...@gm...> writes:
> 
> > The original motivation for this change was that I wanted to build the
> > development version on NixOS.  In the Nix packaging system you can build
> > stuff directly from a git checkout, but because the system emphasizes
> > reproducible builds, the build cannot depend on the contents of the .git
> > directory because these are actually non-deterministic (due to git gc),
> > so two checkouts of the same commit may actually have a different .git
> > directory.  This will lead to spurious build failures because the output
> > hashes differ.
> >
> >     https://github.com/NixOS/nixpkgs/issues/8567
> >     https://github.com/NixOS/nixpkgs/issues/20521
> >
> > I thought it would be a good idea to upstream the support for this but
> > if this is too much of a pain here, I can also patch it locally.
> 
> Thanks for explaining. What would you do if this project was hosted
> somewhere that doesn't make these snapshots available? Like github for
> instance?
> 

I'm not actually using the snapshot that is generated by SourceForge (on
GitHub you can also download snapshots, btw).  In my NixOS overlay I'm
cloning from the git repo but normally the .git directory is removed for
the aforementioned reasons.  Below you can see my Nix derivation to
build gnuplot from git which currently randomly fails due to git
repacking its .git directory which changes the output hash of the
derivation (that's how Nix verifies the integrity of packages).  Sorry,
this might all sound very obscure if you've never used Nix.

Kind regards,
Henri

    self: super:
    
    {
      gnuplot_qt = super.gnuplot_qt.overrideAttrs (oldAttrs: {
        version = "5.5_8685df8";
    
        src = self.fetchgit {
          url = "https://git.code.sf.net/p/gnuplot/gnuplot-main";
          rev = "8685df8e070211f99337bdf69162ab0ca6b4b1e6";
          sha256 = "17yv88xxfwgy9r07irbpwf30np8vvgkwy3nd3ngrrqw58ibjlrm6"; # <--- This changes if .git changes
          leaveDotGit = true; # <--- Fix timestamp.h generation
        };
    
        nativeBuildInputs = oldAttrs.nativeBuildInputs ++ [
          self.autoconf
          self.automake
          self.git
        ];
    
        postPatch = ''
          git checkout -b master # <--- Fix timestamp.h generation
          ./prepare
          ${oldAttrs.postPatch}
        '';
      });
    }

> _______________________________________________
> gnuplot-beta mailing list
> gnu...@li...
> Membership management via: https://lists.sourceforge.net/lists/listinfo/gnuplot-beta

[PATCH v3] Support building development version without git

[Henri M. <hen...@gm...>]

Currently building the development version requires git to be installed
to fetch the `last modified' date from the commit information.  This is
okay when building from a git clone, but fails when downloading a
snapshot that does not contain the .git directory.

To this end, I implemented a fallback that uses the last modification
time of the Makefile.am, which corresponds to the last commit date when
using a fresh checkout.  The old version had the advantage that
`timestamp.h' would only be rebuilt when changing the git branch.  If
git is not available, this logic fails of course.  Instead I made
`timestamp.h' a phony target, i.e. it will be rebuilt unconditionally
every time.  The disadvantage here is that this will also trigger a
rebuild of every file that depends on `timestamp.h' but as far as I can
see this is only `version.c'.
---
I've changed the commit format from %cs to %cd which is supported in
older git versions as well.  I'd like to avoid using %ci because this
requires an extra pipe to strip off the unnecessary time of day and
timezone information which makes the code even more ugly and unreadable.

 src/Makefile.am | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 1b0d8136e..0b261ea47 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -107,12 +107,12 @@ endif
 
 DISTCLEANFILES = timestamp.h
 BUILT_SOURCES = timestamp.h
-git_current := $(shell cut -c6- $(top_srcdir)/.git/HEAD)
-timestamp.h: $(top_srcdir)/.git/${git_current} Makefile
+.PHONY: timestamp.h
+timestamp.h: Makefile
 	@echo Making $@
 	@echo "#ifndef GNUPLOT_TIMEBASE_H_INCLUDED" >$@t
 	@echo "#define GNUPLOT_TIMEBASE_H_INCLUDED" >>$@t
-	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%ci | cut -b-11`\";" >>$@t
+	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --date=short --format=%cd || stat -c '%.10y' Makefile.am`\";" >>$@t
 	@echo "#endif /* GNUPLOT_TIMEBASE_H_INCLUDED */" >> $@t
 	@if cmp -s $@ $@t; then rm -f $@t; else mv $@t $@; fi
 
-- 
2.27.0

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Dima K. <gn...@di...>]

Ethan A Merritt <me...@uw...> writes:

> In the end the fix was trivial, but I wouldn't call it "obvious".
>
> The clipping in proces_image() at step 5 correctly accounts for the pixel
> extent on x and y regardless of what flag is in pixel->type.  So nothing is
> gained by  setting it to OUTRANGE in step 2.  If instead we force all
> pixels to be flagged INRANGE at that stage, the incorrect zrange
> determination at step 3 doesn't happen and all pixels are kept.
> It wasn't obvious to me that marking all pixels INRANGE would preserve
> both autoscaling and clipping, but it does.
>
> fix commited to tip, queued for 5.4

I tested it out, and it appears to work. Thanks a lot! I would've had a
hard time not breaking stuff if I tried to fix this myself.

Re: [PATCH] Support building development version without git

[Dima K. <gn...@di...>]

Henri Menke <hen...@gm...> writes:

> The original motivation for this change was that I wanted to build the
> development version on NixOS.  In the Nix packaging system you can build
> stuff directly from a git checkout, but because the system emphasizes
> reproducible builds, the build cannot depend on the contents of the .git
> directory because these are actually non-deterministic (due to git gc),
> so two checkouts of the same commit may actually have a different .git
> directory.  This will lead to spurious build failures because the output
> hashes differ.
>
>     https://github.com/NixOS/nixpkgs/issues/8567
>     https://github.com/NixOS/nixpkgs/issues/20521
>
> I thought it would be a good idea to upstream the support for this but
> if this is too much of a pain here, I can also patch it locally.

Thanks for explaining. What would you do if this project was hosted
somewhere that doesn't make these snapshots available? Like github for
instance?

Re: [PATCH] Support building development version without git

[Ethan A M. <me...@uw...>]

On Wednesday, 1 July 2020 16:25:59 PDT Ethan A Merritt wrote:
 
> Henri:
> 
> Unfortunately your patch doesn't work for me. 
> 
> When I apply the patch and run
>   ./prepare
>   ./configure
>   make
> 
> I end up with an executable that reports it's version as 
> 
>         G N U P L O T
>         Version 5.5 patchlevel 0    last modified %cs
> 
> Was your change from "--format=%ci" to "--format=%cs" intentional,
> or was it just a typo? 
>   git --version
>   git version 2.21.3
>   git log -1 --format=%cs
>   %cs
>   git log -1 --format="%cs"
>   %cs
> 

Poking around online git docs, I find that %cs was introduced in 
version 2.25 (Jan 2020).  Too new for most distros!
I think we should stick to %ci for now.

	Ethan

Re: [PATCH] Support building development version without git

[Ethan A M. <me...@uw...>]

On Wednesday, 1 July 2020 15:35:03 PDT Henri Menke wrote:
> On 01/07/20, 03:30, Dima Kogan wrote:
> > Henri Menke <hen...@gm...> writes:
> > 
> > > If I go to the SourceForge project page [1] and just hit the `Download
> > > Snapshot' button in the upper right corner, I get a zip file with all
> > > the source but no .git folder and also no timestamp.h. I suspect there
> > > is no or at least no easy way to make SourceForge preprocess the
> > > source when generating a snapshot.
> > >
> > > Within the Makefile determining the latest change would be tricky. The
> > > only way I can think of right now would be to run stat on all the
> > > files in the source tree and pick the newest one, but that is slow and
> > > ugly. Maybe instead of using the current date if git is unavailable
> > > just do echo '0000-00-00'. It should be pretty obvious that this is
> > > not the true `Last modified' then.
> > 
> > Oh. Wow. OK. My feeling is that people should be either
> > 
> > - using release tarballs
> > - using git
> > 
> > It's unfortunate that sourceforge makes another option available without
> > asking, but I don't think it's worth the effort to support it, and a
> > dummy string is just fine. Is a date string of "DATE UNKNOWN; PLEASE USE
> > A RELEASE TARBALL OR A GIT CHECKOUT" valid for the purposes of
> > timestamp.h? Ethan: do you want to support this use case in a better
> > way?
> 
> The original motivation for this change was that I wanted to build the
> development version on NixOS.  In the Nix packaging system you can build
> stuff directly from a git checkout, but because the system emphasizes
> reproducible builds, the build cannot depend on the contents of the .git
> directory because these are actually non-deterministic (due to git gc),
> so two checkouts of the same commit may actually have a different .git
> directory.  This will lead to spurious build failures because the output
> hashes differ.
> 
>     https://github.com/NixOS/nixpkgs/issues/8567
>     https://github.com/NixOS/nixpkgs/issues/20521
> 
> I thought it would be a good idea to upstream the support for this but
> if this is too much of a pain here, I can also patch it locally.
> 
> Kind regards,
> Henri

Let me add a 3rd reason why it would be a good idea to re-think the
current timestamp dependency:   
If you use "git bisect"  for debugging, it is made more cumbersome by the
fact the builds fail because the timestamp dependency in the Makefile is
not satisfied by the content of the git repository as seen during the
bisection.   

Henri:

Unfortunately your patch doesn't work for me. 

When I apply the patch and run
  ./prepare
  ./configure
  make

I end up with an executable that reports it's version as 

        G N U P L O T
        Version 5.5 patchlevel 0    last modified %cs

Was your change from "--format=%ci" to "--format=%cs" intentional,
or was it just a typo? 
  git --version
  git version 2.21.3
  git log -1 --format=%cs
  %cs
  git log -1 --format="%cs"
  %cs


If I change it back to %ci I get 

        G N U P L O T
        Version 5.5 patchlevel 0    last modified 2020-06-30 20:23:26 -0700

which is correct but obviously longer than just the date.

	Ethan

[PATCH v2] Support building development version without git

[Henri M. <hen...@gm...>]

Currently building the development version requires git to be installed
to fetch the `last modified' date from the commit information.  This is
okay when building from a git clone, but fails when downloading a
snapshot that does not contain the .git directory.

To this end, I implemented a fallback that uses the last modification
time of the Makefile.am, which corresponds to the last commit date when
using a fresh checkout.  The old version had the advantage that
`timestamp.h' would only be rebuilt when changing the git branch.  If
git is not available, this logic fails of course.  Instead I made
`timestamp.h' a phony target, i.e. it will be rebuilt unconditionally
every time.  The disadvantage here is that this will also trigger a
rebuild of every file that depends on `timestamp.h' but as far as I can
see this is only `version.c'.
---
This implements the suggestion by Miquel Garriga to use the mtime of
Makefile.am which is set to the last commit time on a fresh checkout.

 src/Makefile.am | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 1b0d8136e..bb9461c52 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -107,12 +107,12 @@ endif
 
 DISTCLEANFILES = timestamp.h
 BUILT_SOURCES = timestamp.h
-git_current := $(shell cut -c6- $(top_srcdir)/.git/HEAD)
-timestamp.h: $(top_srcdir)/.git/${git_current} Makefile
+.PHONY: timestamp.h
+timestamp.h: Makefile
 	@echo Making $@
 	@echo "#ifndef GNUPLOT_TIMEBASE_H_INCLUDED" >$@t
 	@echo "#define GNUPLOT_TIMEBASE_H_INCLUDED" >>$@t
-	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%ci | cut -b-11`\";" >>$@t
+	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%cs || stat -c '%.10y' Makefile.am`\";" >>$@t
 	@echo "#endif /* GNUPLOT_TIMEBASE_H_INCLUDED */" >> $@t
 	@if cmp -s $@ $@t; then rm -f $@t; else mv $@t $@; fi
 
-- 
2.27.0

Re: [PATCH] Support building development version without git

[Henri M. <hen...@gm...>]

On 01/07/20, 03:30, Dima Kogan wrote:
> Henri Menke <hen...@gm...> writes:
> 
> > If I go to the SourceForge project page [1] and just hit the `Download
> > Snapshot' button in the upper right corner, I get a zip file with all
> > the source but no .git folder and also no timestamp.h. I suspect there
> > is no or at least no easy way to make SourceForge preprocess the
> > source when generating a snapshot.
> >
> > Within the Makefile determining the latest change would be tricky. The
> > only way I can think of right now would be to run stat on all the
> > files in the source tree and pick the newest one, but that is slow and
> > ugly. Maybe instead of using the current date if git is unavailable
> > just do echo '0000-00-00'. It should be pretty obvious that this is
> > not the true `Last modified' then.
> 
> Oh. Wow. OK. My feeling is that people should be either
> 
> - using release tarballs
> - using git
> 
> It's unfortunate that sourceforge makes another option available without
> asking, but I don't think it's worth the effort to support it, and a
> dummy string is just fine. Is a date string of "DATE UNKNOWN; PLEASE USE
> A RELEASE TARBALL OR A GIT CHECKOUT" valid for the purposes of
> timestamp.h? Ethan: do you want to support this use case in a better
> way?

The original motivation for this change was that I wanted to build the
development version on NixOS.  In the Nix packaging system you can build
stuff directly from a git checkout, but because the system emphasizes
reproducible builds, the build cannot depend on the contents of the .git
directory because these are actually non-deterministic (due to git gc),
so two checkouts of the same commit may actually have a different .git
directory.  This will lead to spurious build failures because the output
hashes differ.

    https://github.com/NixOS/nixpkgs/issues/8567
    https://github.com/NixOS/nixpkgs/issues/20521

I thought it would be a good idea to upstream the support for this but
if this is too much of a pain here, I can also patch it locally.

Kind regards,
Henri

> 
> 
> _______________________________________________
> gnuplot-beta mailing list
> gnu...@li...
> Membership management via: https://lists.sourceforge.net/lists/listinfo/gnuplot-beta

Re: [PATCH] Support building development version without git

[Miquel G. <mi...@ic...>]

On Wed, 1 Jul 2020 12:18:16 +1200
Henri Menke <hen...@gm...> wrote:

> If I go to the SourceForge project page [1] and just hit the `Download
> Snapshot' button in the upper right corner, I get a zip file with all
> the source but no .git folder and also no timestamp.h.  I suspect there
> is no or at least no easy way to make SourceForge preprocess the source
> when generating a snapshot.
> 

You can use the mtime of any file inside the zip file. 
All them are set to the commit time of the git-hash you hit 'Download Snapshot' for.
Thus "stat -c '%y ' src/Makefile.am" will give the commit date.

For tarballs obtained via "git archive -o snapshot.tar githash" 
all the files in the tarball have the commit-time as mtime.

Miquel

Re: [PATCH] Support building development version without git

[Dima K. <gn...@di...>]

Henri Menke <hen...@gm...> writes:

> If I go to the SourceForge project page [1] and just hit the `Download
> Snapshot' button in the upper right corner, I get a zip file with all
> the source but no .git folder and also no timestamp.h. I suspect there
> is no or at least no easy way to make SourceForge preprocess the
> source when generating a snapshot.
>
> Within the Makefile determining the latest change would be tricky. The
> only way I can think of right now would be to run stat on all the
> files in the source tree and pick the newest one, but that is slow and
> ugly. Maybe instead of using the current date if git is unavailable
> just do echo '0000-00-00'. It should be pretty obvious that this is
> not the true `Last modified' then.

Oh. Wow. OK. My feeling is that people should be either

- using release tarballs
- using git

It's unfortunate that sourceforge makes another option available without
asking, but I don't think it's worth the effort to support it, and a
dummy string is just fine. Is a date string of "DATE UNKNOWN; PLEASE USE
A RELEASE TARBALL OR A GIT CHECKOUT" valid for the purposes of
timestamp.h? Ethan: do you want to support this use case in a better
way?

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Ethan A M. <me...@uw...>]

On Monday, 29 June 2020 19:51:04 PDT Dima Kogan wrote:
> Alright, I debugged this (rr was invaluable), and I know mostly what's
> going on. The flow is roughly this:
> 
> 1. The data is read here:
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/plot3d.c#l1037
> 
>    At the end of the first line, x is 3999.0000000000005, which is just outside the
>    xrange [0:3999]. This isn't unexpected due to the "using" expression
> 
> 2. Later we evaluate the x axis for that point (first row, last col):
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/axis.c#l2782
> 
>    3999.0000000000005>3999, so we set this to OUTRANGE
> 
>    In a perfect world we should be looking at the "with image" pixel
>    edges instead of just the center; like we do in step 5 below.
> 
> 3. Later we evaluate the z axis for that same point, but this is already
>    OUTRANGE, so we don't bother to use that point to set axis->min or
>    axis->max
> 
> 4. As a result the view_port_z[] range we set later is narrower than it
>    should be:
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/graphics.c#l4822
> 
> 5. And then, further on in in that function, that corner pixel is deemed
>    invisible. So the first row then has only 15 pixels, but the next row
>    has all 16, so we complain
> 
> The logic in step 5 (what we do for !visible pixels) looks suspicious,
> but I want to say the fundamental problem is that step 2 and step 5 have
> different logic for inrange/outrange. The top-right corner of the
> failing plot is OUT in step 2, but IN in step 5.
> 
> Any obvious fixes?

In the end the fix was trivial, but I wouldn't call it "obvious".

The clipping in proces_image() at step 5 correctly accounts for the pixel
extent on x and y regardless of what flag is in pixel->type.  So nothing is
gained by  setting it to OUTRANGE in step 2.  If instead we force all
pixels to be flagged INRANGE at that stage, the incorrect zrange
determination at step 3 doesn't happen and all pixels are kept. 
It wasn't obvious to me that marking all pixels INRANGE would preserve
both autoscaling and clipping, but it does.

fix commited to tip, queued for 5.4

	Ethan

Re: [PATCH] Support building development version without git

[Henri M. <hen...@gm...>]

On 30/06/20, 16:19, Ethan Merritt wrote:
> On Tuesday, 30 June 2020 16:03:22 PDT Henri Menke wrote:
> > Currently building the development version requires git to be installed
> > to fetch the `last modified' date from the commit information.  This is
> > okay when building from a git clone, but fails when downloading a
> > snapshot that does not contain the .git directory.
> 
> Yeah, that has been a problem.
> Thanks for looking at this.
> 
> > To this end, I implemented a fallback which just uses the current date
> > if git is not available.  
> 
> If you are building from a snapshot, the correct date should be the
> date of the snapshot.  I don't know how to do that.  Do you?

If I go to the SourceForge project page [1] and just hit the `Download
Snapshot' button in the upper right corner, I get a zip file with all
the source but no .git folder and also no timestamp.h.  I suspect there
is no or at least no easy way to make SourceForge preprocess the source
when generating a snapshot.

Within the Makefile determining the latest change would be tricky.  The
only way I can think of right now would be to run stat on all the files
in the source tree and pick the newest one, but that is slow and ugly.
Maybe instead of using the current date if git is unavailable just do
echo '0000-00-00'.  It should be pretty obvious that this is not the
true `Last modified' then.

Kind regards,
Henri

> 
> 	Ethan
> 
> > The old version had the advantage that
> > `timestamp.h' would only be rebuilt when changing the git branch.  If
> > git is not available, this logic fails of course.  Instead I made
> > `timestamp.h' a phony target, i.e. it will be rebuilt unconditionally
> > every time.  The disadvantage here is that this will also trigger a
> > rebuild of every file that depends on `timestamp.h' but as far as I can
> > see this is only `version.c'.
> > ---
> >  src/Makefile.am | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 1b0d8136e..7eca23cfa 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -107,12 +107,12 @@ endif
> >  
> >  DISTCLEANFILES = timestamp.h
> >  BUILT_SOURCES = timestamp.h
> > -git_current := $(shell cut -c6- $(top_srcdir)/.git/HEAD)
> > -timestamp.h: $(top_srcdir)/.git/${git_current} Makefile
> > +.PHONY: timestamp.h
> > +timestamp.h: Makefile
> >  	@echo Making $@
> >  	@echo "#ifndef GNUPLOT_TIMEBASE_H_INCLUDED" >$@t
> >  	@echo "#define GNUPLOT_TIMEBASE_H_INCLUDED" >>$@t
> > -	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%ci | cut -b-11`\";" >>$@t
> > +	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%cs || date +'%Y-%m-%d'`\";" >>$@t
> >  	@echo "#endif /* GNUPLOT_TIMEBASE_H_INCLUDED */" >> $@t
> >  	@if cmp -s $@ $@t; then rm -f $@t; else mv $@t $@; fi
> >  
> > 
> 
> 
> 
> 

Re: [PATCH] Support building development version without git

[Ethan A M. <me...@uw...>]

On Tuesday, 30 June 2020 17:08:17 PDT Dima Kogan wrote:
> Ethan A Merritt <me...@uw...> writes:
> 
> > It sounds reasonable to me.
> >
> > This seems to be surprisingly tricky. Previous attempts did not work out.
> > See commits:
> > 	9d0f148d  	d543154a
> 
> OK, I'll look in a bit. In the meantime, can you tell me how we build
> snapshots? Do we just 'tar cz --exclude .git ...' or is there a script
> of some sort?

That's a large part of the problem.

We clearly can build a stand-alone tarball if we have control over it.
That's what I do to make a release package.

Unfortunately so far as I know we don't have control over the snapshot
building process on the SourceForge page.
It looks like they just dump the current content of the repository into a tarball.

I could try sending a support request to the SourceForge ops, but I'm
not all that hopeful about getting a useful response.

	Ethan  

Re: [PATCH] Support building development version without git

[Dima K. <gn...@di...>]

Ethan A Merritt <me...@uw...> writes:

> It sounds reasonable to me.
>
> This seems to be surprisingly tricky. Previous attempts did not work out.
> See commits:
> 	9d0f148d  	d543154a

OK, I'll look in a bit. In the meantime, can you tell me how we build
snapshots? Do we just 'tar cz --exclude .git ...' or is there a script
of some sort?

Re: [PATCH] Support building development version without git

[Ethan A M. <me...@uw...>]

On Tuesday, 30 June 2020 16:31:06 PDT Dima Kogan wrote:
> Dima Kogan <gn...@di...> writes:
> 
> > Ethan Merritt <eam...@gm...> writes:
> >
> >> If you are building from a snapshot, the correct date should be the
> >> date of the snapshot. I don't know how to do that. Do you?
> >
> > This should be encoded in the snapshot in some way.

> Looking at the patch, I think that we should generate the timestamp.h at
> snapshot-building time, and the Makefile in the snapshot shouldn't ever
> generate it or delete it. Reasonable?

It sounds reasonable to me.

This seems to be surprisingly tricky. Previous attempts did not work out.
See commits:
	9d0f148d  	d543154a

	Ethan
	

Re: [PATCH] Support building development version without git

[Dima K. <gn...@di...>]

Dima Kogan <gn...@di...> writes:

> Ethan Merritt <eam...@gm...> writes:
>
>> If you are building from a snapshot, the correct date should be the
>> date of the snapshot. I don't know how to do that. Do you?
>
> This should be encoded in the snapshot in some way.

Looking at the patch, I think that we should generate the timestamp.h at
snapshot-building time, and the Makefile in the snapshot shouldn't ever
generate it or delete it. Reasonable?

Re: [PATCH] Support building development version without git

[Dima K. <gn...@di...>]

Ethan Merritt <eam...@gm...> writes:

> If you are building from a snapshot, the correct date should be the
> date of the snapshot. I don't know how to do that. Do you?

This should be encoded in the snapshot in some way. How are we building
the snapshot? The Makefile in the snapshot shouldn't have any git
commands, or at least should know to not run them.

Re: [PATCH] Support building development version without git

[Ethan M. <eam...@gm...>]

On Tuesday, 30 June 2020 16:03:22 PDT Henri Menke wrote:
> Currently building the development version requires git to be installed
> to fetch the `last modified' date from the commit information.  This is
> okay when building from a git clone, but fails when downloading a
> snapshot that does not contain the .git directory.

Yeah, that has been a problem.
Thanks for looking at this.

> To this end, I implemented a fallback which just uses the current date
> if git is not available.  

If you are building from a snapshot, the correct date should be the
date of the snapshot.  I don't know how to do that.  Do you?

	Ethan

> The old version had the advantage that
> `timestamp.h' would only be rebuilt when changing the git branch.  If
> git is not available, this logic fails of course.  Instead I made
> `timestamp.h' a phony target, i.e. it will be rebuilt unconditionally
> every time.  The disadvantage here is that this will also trigger a
> rebuild of every file that depends on `timestamp.h' but as far as I can
> see this is only `version.c'.
> ---
>  src/Makefile.am | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 1b0d8136e..7eca23cfa 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -107,12 +107,12 @@ endif
>  
>  DISTCLEANFILES = timestamp.h
>  BUILT_SOURCES = timestamp.h
> -git_current := $(shell cut -c6- $(top_srcdir)/.git/HEAD)
> -timestamp.h: $(top_srcdir)/.git/${git_current} Makefile
> +.PHONY: timestamp.h
> +timestamp.h: Makefile
>  	@echo Making $@
>  	@echo "#ifndef GNUPLOT_TIMEBASE_H_INCLUDED" >$@t
>  	@echo "#define GNUPLOT_TIMEBASE_H_INCLUDED" >>$@t
> -	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%ci | cut -b-11`\";" >>$@t
> +	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%cs || date +'%Y-%m-%d'`\";" >>$@t
>  	@echo "#endif /* GNUPLOT_TIMEBASE_H_INCLUDED */" >> $@t
>  	@if cmp -s $@ $@t; then rm -f $@t; else mv $@t $@; fi
>  
> 

[PATCH] Support building development version without git

[Henri M. <hen...@gm...>]

Currently building the development version requires git to be installed
to fetch the `last modified' date from the commit information.  This is
okay when building from a git clone, but fails when downloading a
snapshot that does not contain the .git directory.

To this end, I implemented a fallback which just uses the current date
if git is not available.  The old version had the advantage that
`timestamp.h' would only be rebuilt when changing the git branch.  If
git is not available, this logic fails of course.  Instead I made
`timestamp.h' a phony target, i.e. it will be rebuilt unconditionally
every time.  The disadvantage here is that this will also trigger a
rebuild of every file that depends on `timestamp.h' but as far as I can
see this is only `version.c'.
---
 src/Makefile.am | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 1b0d8136e..7eca23cfa 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -107,12 +107,12 @@ endif
 
 DISTCLEANFILES = timestamp.h
 BUILT_SOURCES = timestamp.h
-git_current := $(shell cut -c6- $(top_srcdir)/.git/HEAD)
-timestamp.h: $(top_srcdir)/.git/${git_current} Makefile
+.PHONY: timestamp.h
+timestamp.h: Makefile
 	@echo Making $@
 	@echo "#ifndef GNUPLOT_TIMEBASE_H_INCLUDED" >$@t
 	@echo "#define GNUPLOT_TIMEBASE_H_INCLUDED" >>$@t
-	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%ci | cut -b-11`\";" >>$@t
+	@echo "const char gnuplot_date[] = \"`git --git-dir '$(top_srcdir)/.git' log -1 --format=%cs || date +'%Y-%m-%d'`\";" >>$@t
 	@echo "#endif /* GNUPLOT_TIMEBASE_H_INCLUDED */" >> $@t
 	@if cmp -s $@ $@t; then rm -f $@t; else mv $@t $@; fi
 
-- 
2.27.0

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Hans-Bernhard B. <HBB...@t-...>]

Am 30.06.2020 um 20:49 schrieb Ethan A Merritt:
> 
> I have some reservations about whether it ever makes sense to clip
> the contents of a plot `with image`, since in the general case of
> rotation+translation, clipping will not leave a rectangle and thus the
> optimized image-data protocols that pass the image as a single chunk
> of data won't work.

OTOH, the alternative would be that the boundaries set up for clipping 
willl be disrespected completely.  That cannot really be considered correct.

> The current gnuplot code seems to assume that clipping will cleanly
> remove a constant width slice from each edge.  Perhaps one fix is to first
> detect that clipping exists, and if so fall back to 'with image pixels' so
> that each pixel is considered individually.

That does make more sense, IMHO.

> B) For any given axis (z in this case) the program should never try to
> apply both autoscaling and clipping!   

Not so fast.  It's still possible for one end of an axis to be clipped, 
while the other is autoscaled.

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Ethan A M. <me...@uw...>]

On Monday, 29 June 2020 19:51:04 PDT Dima Kogan wrote:
> Alright, I debugged this (rr was invaluable), and I know mostly what's
> going on. The flow is roughly this:
> 
> 1. The data is read here:
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/plot3d.c#l1037
> 
>    At the end of the first line, x is 3999.0000000000005, which is just outside the
>    xrange [0:3999]. This isn't unexpected due to the "using" expression
> 
> 2. Later we evaluate the x axis for that point (first row, last col):
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/axis.c#l2782
> 
>    3999.0000000000005>3999, so we set this to OUTRANGE
> 
>    In a perfect world we should be looking at the "with image" pixel
>    edges instead of just the center; like we do in step 5 below.
> 
> 3. Later we evaluate the z axis for that same point, but this is already
>    OUTRANGE, so we don't bother to use that point to set axis->min or
>    axis->max
> 
> 4. As a result the view_port_z[] range we set later is narrower than it
>    should be:
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/graphics.c#l4822
> 
> 5. And then, further on in in that function, that corner pixel is deemed
>    invisible. So the first row then has only 15 pixels, but the next row
>    has all 16, so we complain
> 
> The logic in step 5 (what we do for !visible pixels) looks suspicious,
> but I want to say the fundamental problem is that step 2 and step 5 have
> different logic for inrange/outrange. The top-right corner of the
> failing plot is OUT in step 2, but IN in step 5.
> 
> Any obvious fixes?

I agree with your analysis.

I have some reservations about whether it ever makes sense to clip
the contents of a plot `with image`, since in the general case of
rotation+translation, clipping will not leave a rectangle and thus the
optimized image-data protocols that pass the image as a single chunk
of data won't work.
The current gnuplot code seems to assume that clipping will cleanly
remove a constant width slice from each edge.  Perhaps one fix is to first
detect that clipping exists, and if so fall back to 'with image pixels' so
that each pixel is considered individually.

Your case, however, triggers uneven clipping not because of rotation
but simply due to round-off error.  Two fixes occur to me.

A) The one implied by your point 2 above: auto-scaling and clipping
should take into account the full width of the pixel.
That can probably be done but I will put that on hold for now.

B) For any given axis (z in this case) the program should never try to
apply both autoscaling and clipping!   I don't know whether this glitch
can happen for other plot styles, but I will look for the most generic
possible place to prevent it.

	Ethan

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Ethan A M. <me...@uw...>]

On Monday, 29 June 2020 20:55:26 PDT Dima Kogan wrote:
> Ethan A Merritt <me...@uw...> writes:
> 
> > On Monday, 29 June 2020 19:51:04 PDT Dima Kogan wrote:
> >> Alright, I debugged this (rr was invaluable), and I know mostly what's
> >> going on. The flow is roughly this:
> >>
> >> 1. The data is read here:
> >>
> >>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/plot3d.c#l1037
> >>
> >>    At the end of the first line, x is 3999.0000000000005, which is just outside the
> >>    xrange [0:3999]. This isn't unexpected due to the "using" expression
> >
> > But the x coordinates should come out identically the same in the "good" and "bad"
> > cases, right?  So how does this explain a different between them?
> 
> In the "good" case we're getting lucky in step 5. The corners (which
> were out-of-bounds in step 2, and were thus ignored for computing
> view_port_z[]) still end up in-bounds in step 5. There's an extra step
> that I didn't mention in the previous email:

Very strange.  The z range should be totally irrelevant to "set view map".
z as a coordinate doesn't matter because it's in projection.
z as an image pixel value doesn't matter because the image grid doesn't
care about the pixel value, only the number of pixels.

But you're right, it does seem to make a difference in this case
For instance:
    gnuplot> splot $MATRIX matrix  using ($1*266.6):($2*199.9090909090909):3 title "" with image
         warning: Visible pixel grid has a scan line longer than previous scan lines.
    gnuplot> splot $MATRIX matrix  using ($1*266.6):($2*199.9090909090909):(0):3 title "" with image
         works with no problem

But opening up the z range explicitly does not fix the problem.
So I still don't understand exactly what's going on here. 

	I'll look harder tomorrow,

		Ethan


> 3.5. We expand the autoranged axis extents to nice, round numbers:
> 
>      https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/axis.c#l960
> 
> This provides some margin so that getting lucky in this way is possible.

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Dima K. <gn...@di...>]

Ethan A Merritt <me...@uw...> writes:

> On Monday, 29 June 2020 19:51:04 PDT Dima Kogan wrote:
>> Alright, I debugged this (rr was invaluable), and I know mostly what's
>> going on. The flow is roughly this:
>>
>> 1. The data is read here:
>>
>>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/plot3d.c#l1037
>>
>>    At the end of the first line, x is 3999.0000000000005, which is just outside the
>>    xrange [0:3999]. This isn't unexpected due to the "using" expression
>
> But the x coordinates should come out identically the same in the "good" and "bad"
> cases, right?  So how does this explain a different between them?

In the "good" case we're getting lucky in step 5. The corners (which
were out-of-bounds in step 2, and were thus ignored for computing
view_port_z[]) still end up in-bounds in step 5. There's an extra step
that I didn't mention in the previous email:

3.5. We expand the autoranged axis extents to nice, round numbers:

     https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/axis.c#l960

This provides some margin so that getting lucky in this way is possible.

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Ethan A M. <me...@uw...>]

On Monday, 29 June 2020 19:51:04 PDT Dima Kogan wrote:
> Alright, I debugged this (rr was invaluable), and I know mostly what's
> going on. The flow is roughly this:
> 
> 1. The data is read here:
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/plot3d.c#l1037
> 
>    At the end of the first line, x is 3999.0000000000005, which is just outside the
>    xrange [0:3999]. This isn't unexpected due to the "using" expression

But the x coordinates should come out identically the same in the "good" and "bad"
cases, right?  So how does this explain a different between them?

	Ethan

> 
> 2. Later we evaluate the x axis for that point (first row, last col):
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/axis.c#l2782
> 
>    3999.0000000000005>3999, so we set this to OUTRANGE
> 
>    In a perfect world we should be looking at the "with image" pixel
>    edges instead of just the center; like we do in step 5 below.
> 
> 3. Later we evaluate the z axis for that same point, but this is already
>    OUTRANGE, so we don't bother to use that point to set axis->min or
>    axis->max
> 
> 4. As a result the view_port_z[] range we set later is narrower than it
>    should be:
> 
>    https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/graphics.c#l4822
> 
> 5. And then, further on in in that function, that corner pixel is deemed
>    invisible. So the first row then has only 15 pixels, but the next row
>    has all 16, so we complain
> 
> The logic in step 5 (what we do for !visible pixels) looks suspicious,
> but I want to say the fundamental problem is that step 2 and step 5 have
> different logic for inrange/outrange. The top-right corner of the
> failing plot is OUT in step 2, but IN in step 5.
> 
> Any obvious fixes?
> 
> Thanks
> 
> 
> _______________________________________________
> gnuplot-beta mailing list
> gnu...@li...
> Membership management via: https://lists.sourceforge.net/lists/listinfo/gnuplot-beta
> 


-- 
Ethan A Merritt
Biomolecular Structure Center,  K-428 Health Sciences Bldg
MS 357742,   University of Washington, Seattle 98195-7742

Re: Plot failure: "Visible pixel grid has a scan line longer than previous scan lines"

[Dima K. <gn...@di...>]

Alright, I debugged this (rr was invaluable), and I know mostly what's
going on. The flow is roughly this:

1. The data is read here:

   https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/plot3d.c#l1037

   At the end of the first line, x is 3999.0000000000005, which is just outside the
   xrange [0:3999]. This isn't unexpected due to the "using" expression

2. Later we evaluate the x axis for that point (first row, last col):

   https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/axis.c#l2782

   3999.0000000000005>3999, so we set this to OUTRANGE

   In a perfect world we should be looking at the "with image" pixel
   edges instead of just the center; like we do in step 5 below.

3. Later we evaluate the z axis for that same point, but this is already
   OUTRANGE, so we don't bother to use that point to set axis->min or
   axis->max

4. As a result the view_port_z[] range we set later is narrower than it
   should be:

   https://sourceforge.net/p/gnuplot/gnuplot-main/ci/branch-5-4-stable/tree/src/graphics.c#l4822

5. And then, further on in in that function, that corner pixel is deemed
   invisible. So the first row then has only 15 pixels, but the next row
   has all 16, so we complain

The logic in step 5 (what we do for !visible pixels) looks suspicious,
but I want to say the fundamental problem is that step 2 and step 5 have
different logic for inrange/outrange. The top-right corner of the
failing plot is OUT in step 2, but IN in step 5.

Any obvious fixes?

Thanks