diff mbox series

[next,v2,3/5] support/scripts/pkg-stats-new: add current version information

Message ID 20180221221342.15683-4-thomas.petazzoni@bootlin.com
State Superseded
Headers show
Series New pkg-stats script, with version information | expand

Commit Message

Thomas Petazzoni Feb. 21, 2018, 10:13 p.m. UTC
This commit adds a new column in the HTML output containing the
current version of a package in Buildroot. As such, it isn't terribly
useful, but combined with the latest upstream version added in a
follow-up commit, it will become very useful.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Changes since v1:
- Fix flake8 warnings
- Pass BR2_HAVE_DOT_CONFIG=y when calling make, in order to fake
  having a .config. This allows "printvars" to dump all variables even
  without a .config.
- Add missing newline in HTML code
---
 support/scripts/pkg-stats-new | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Ricardo Martincoski Feb. 26, 2018, 12:47 a.m. UTC | #1
Hello,

On Wed, Feb 21, 2018 at 07:13 PM, Thomas Petazzoni wrote:

[snip]
> @@ -187,12 +189,33 @@ def add_pkg_make_info(packages):
>  
>          license_files.append(pkgvar)
>  
> +    # Version
> +    o = subprocess.check_output(["make", "BR2_HAVE_DOT_CONFIG=y",
> +                                 "-s", "printvars", "VARS=%_VERSION"])
> +    for l in o.splitlines():
> +        # Get variable name and value
> +        pkgvar, value = l.split("=")
> +
> +        # If present, strip HOST_ from variable name
> +        if pkgvar.startswith("HOST_"):
> +            pkgvar = pkgvar[5:]

The output of printvars is sorted, so this makes the 'versions' dict to
contain at the very end:
 1) the target version when there is no host version
 2) the host version for packages alphabetically before hostapd
 3) the target version for packages alphabetically after hplip
 4) the host version when there is no target version

The inconsistency between 2 and 3 is not a problem right now because all
packages that have host and target versions use the same versions for both.
Maybe it will never be a problem?

I hope Yann can help us here (I added to CC).

The case 4 seems to serve a single package: lpc3250loader
We have other packages that are host-only, i.e. vboot-utils, they set _VERSION,
not HOST.*_VERSION.
Maybe it is something to fix in the package?
If yes, you could just ignore the entries starting with HOST_.

> +
> +        if pkgvar.endswith("_DL_VERSION"):
> +            continue
> +        if pkgvar.startswith("HOST_"):
> +            pkgvar = pkgvar[5:]
> +
> +        # Strip _VERSION
> +        pkgvar = pkgvar[:-8]
> +
> +        versions[pkgvar] = value
[snip]
> @@ -387,6 +410,9 @@ def dump_html_pkg(f, pkg):
>      f.write("  <td class=\"%s\">%s</td>\n" %
>              (" ".join(td_class), boolean_str(pkg.has_hash)))
>  
> +    # Current version
> +    f.write("  <td class=\"centered\">%s</td>\n" % pkg.current_version)

This column shows raw sha1 for git packages.
Wouldn't be visually better to show only the first, say 20, characters?
At the end, it's a matter of option.

If more people have the same opinion you could cut the string after checking
with a regexp for 40 or 39 (see sunxi-mali) hexa digits.

Anyway it can be done later.

Regards,
Ricardo
Thomas Petazzoni March 7, 2018, 10:25 p.m. UTC | #2
Hello,

Thanks for your review!

On Sun, 25 Feb 2018 21:47:48 -0300, Ricardo Martincoski wrote:

> The output of printvars is sorted, so this makes the 'versions' dict to
> contain at the very end:
>  1) the target version when there is no host version
>  2) the host version for packages alphabetically before hostapd
>  3) the target version for packages alphabetically after hplip
>  4) the host version when there is no target version
> 
> The inconsistency between 2 and 3 is not a problem right now because all
> packages that have host and target versions use the same versions for both.
> Maybe it will never be a problem?
> 
> I hope Yann can help us here (I added to CC).
> 
> The case 4 seems to serve a single package: lpc3250loader
> We have other packages that are host-only, i.e. vboot-utils, they set _VERSION,
> not HOST.*_VERSION.
> Maybe it is something to fix in the package?
> If yes, you could just ignore the entries starting with HOST_.

I've addressed your concern by doing this:

+    # We process first the host package VERSION, and then the target
+    # package VERSION. This means that if a package exists in both
+    # target and host variants, with different version numbers
+    # (unlikely), we'll report the target version number.
+    version_list = o.splitlines()
+    version_list = [ x for x in version_list if x.startswith("HOST_") ] + \
+                   [ x for x in version_list if not x.startswith("HOST_") ]
+    for l in version_list:

This means that we will first process all the HOST_*_VERSION variables,
filling up the dict with the version numbers for the host packages, and
then we will process the *_VERSION variables.

With this:

 - If the package specifies only a target version, the target version
   is saved.

 - If the package specifies only a host version, the host version is
   saved.

 - If the package specifies both a target version and a host version,
   the target version always wins. So the scripts doesn't handle
   differentiating host and target versions for the same package, but
   at least we are consistent in the fact that we will always report
   the target version.

How does that sound ?

> This column shows raw sha1 for git packages.
> Wouldn't be visually better to show only the first, say 20, characters?
> At the end, it's a matter of option.
> 
> If more people have the same opinion you could cut the string after checking
> with a regexp for 40 or 39 (see sunxi-mali) hexa digits.

I'm just cutting after 20 characters, and adding "..." at the end, and
it seems good enough to me (compared to doing a regexp to try to guess
if it's a git hash or not).

Best regards,

Thomas
Ricardo Martincoski March 8, 2018, 3:14 a.m. UTC | #3
Hello,

On Wed, Mar 07, 2018 at 07:25 PM, Thomas Petazzoni wrote:

> On Sun, 25 Feb 2018 21:47:48 -0300, Ricardo Martincoski wrote:
> 
>> The output of printvars is sorted, so this makes the 'versions' dict to
>> contain at the very end:
>>  1) the target version when there is no host version
>>  2) the host version for packages alphabetically before hostapd
>>  3) the target version for packages alphabetically after hplip
>>  4) the host version when there is no target version
>> 
>> The inconsistency between 2 and 3 is not a problem right now because all
>> packages that have host and target versions use the same versions for both.
>> Maybe it will never be a problem?
>> 
>> I hope Yann can help us here (I added to CC).
>> 
>> The case 4 seems to serve a single package: lpc3250loader
>> We have other packages that are host-only, i.e. vboot-utils, they set _VERSION,
>> not HOST.*_VERSION.
>> Maybe it is something to fix in the package?
>> If yes, you could just ignore the entries starting with HOST_.
> 
> I've addressed your concern by doing this:
> 
> +    # We process first the host package VERSION, and then the target
> +    # package VERSION. This means that if a package exists in both
> +    # target and host variants, with different version numbers
> +    # (unlikely), we'll report the target version number.
> +    version_list = o.splitlines()
> +    version_list = [ x for x in version_list if x.startswith("HOST_") ] + \
> +                   [ x for x in version_list if not x.startswith("HOST_") ]
> +    for l in version_list:
> 
> This means that we will first process all the HOST_*_VERSION variables,
> filling up the dict with the version numbers for the host packages, and
> then we will process the *_VERSION variables.
> 
> With this:
> 
>  - If the package specifies only a target version, the target version
>    is saved.
> 
>  - If the package specifies only a host version, the host version is
>    saved.
> 
>  - If the package specifies both a target version and a host version,
>    the target version always wins. So the scripts doesn't handle
>    differentiating host and target versions for the same package, but
>    at least we are consistent in the fact that we will always report
>    the target version.
> 
> How does that sound ?

Sounds good.
Nit: please recheck with flake8.

> 
>> This column shows raw sha1 for git packages.
>> Wouldn't be visually better to show only the first, say 20, characters?
>> At the end, it's a matter of option.
>> 
>> If more people have the same opinion you could cut the string after checking
>> with a regexp for 40 or 39 (see sunxi-mali) hexa digits.
> 
> I'm just cutting after 20 characters, and adding "..." at the end, and
> it seems good enough to me (compared to doing a regexp to try to guess
> if it's a git hash or not).

OK since you are only changing the display of the version, not the string that
will be compared to the new column.

The few version strings in the tree that are not sha1 larger than 20 are:
2.0.0.alpha20140727b
kvm-unit-tests-20171020
rel_imx_4.9.x_1.0.0_ga

For me these are OK:
2.0.0.alpha20140727...
kvm-unit-tests-2017...
rel_imx_4.9.x_1.0.0...


Regards,
Ricardo
Thomas Petazzoni March 8, 2018, 7:48 a.m. UTC | #4
Hello,

On Thu, 08 Mar 2018 00:14:51 -0300, Ricardo Martincoski wrote:

> > With this:
> > 
> >  - If the package specifies only a target version, the target version
> >    is saved.
> > 
> >  - If the package specifies only a host version, the host version is
> >    saved.
> > 
> >  - If the package specifies both a target version and a host version,
> >    the target version always wins. So the scripts doesn't handle
> >    differentiating host and target versions for the same package, but
> >    at least we are consistent in the fact that we will always report
> >    the target version.
> > 
> > How does that sound ?  
> 
> Sounds good.
> Nit: please recheck with flake8.

Will do.


> > I'm just cutting after 20 characters, and adding "..." at the end, and
> > it seems good enough to me (compared to doing a regexp to try to guess
> > if it's a git hash or not).  
> 
> OK since you are only changing the display of the version, not the string that
> will be compared to the new column.
> 
> The few version strings in the tree that are not sha1 larger than 20 are:
> 2.0.0.alpha20140727b
> kvm-unit-tests-20171020
> rel_imx_4.9.x_1.0.0_ga
> 
> For me these are OK:
> 2.0.0.alpha20140727...
> kvm-unit-tests-2017...
> rel_imx_4.9.x_1.0.0...

Yes, I'm only changing the display of the version, and I'm also adding
"..." which helps the reader understand that the version string has
been cut.

Best regards,

Thomas
diff mbox series

Patch

diff --git a/support/scripts/pkg-stats-new b/support/scripts/pkg-stats-new
index 85a6caeeb9..c4174877aa 100755
--- a/support/scripts/pkg-stats-new
+++ b/support/scripts/pkg-stats-new
@@ -36,6 +36,7 @@  class Package:
         self.has_hash = False
         self.patch_count = 0
         self.warnings = 0
+        self.current_version = None
 
     def __eq__(self, other):
         return self.path == other.path
@@ -148,6 +149,7 @@  def add_pkg_make_info(packages):
     """
     licenses = list()
     license_files = list()
+    versions = dict()
 
     # Licenses
     o = subprocess.check_output(["make", "BR2_HAVE_DOT_CONFIG=y",
@@ -187,12 +189,33 @@  def add_pkg_make_info(packages):
 
         license_files.append(pkgvar)
 
+    # Version
+    o = subprocess.check_output(["make", "BR2_HAVE_DOT_CONFIG=y",
+                                 "-s", "printvars", "VARS=%_VERSION"])
+    for l in o.splitlines():
+        # Get variable name and value
+        pkgvar, value = l.split("=")
+
+        # If present, strip HOST_ from variable name
+        if pkgvar.startswith("HOST_"):
+            pkgvar = pkgvar[5:]
+
+        if pkgvar.endswith("_DL_VERSION"):
+            continue
+
+        # Strip _VERSION
+        pkgvar = pkgvar[:-8]
+
+        versions[pkgvar] = value
+
     for pkg in packages:
         var = pkgname_to_pkgvar(pkg.name)
         if var in licenses:
             pkg.has_license = True
         if var in license_files:
             pkg.has_license_files = True
+        if var in versions:
+            pkg.current_version = versions[var]
 
 
 def add_hash_info(packages):
@@ -387,6 +410,9 @@  def dump_html_pkg(f, pkg):
     f.write("  <td class=\"%s\">%s</td>\n" %
             (" ".join(td_class), boolean_str(pkg.has_hash)))
 
+    # Current version
+    f.write("  <td class=\"centered\">%s</td>\n" % pkg.current_version)
+
     # Warnings
     td_class = ["centered"]
     if pkg.warnings == 0:
@@ -409,6 +435,7 @@  def dump_html_all_pkgs(f, packages):
 <td class=\"centered\">License</td>
 <td class=\"centered\">License files</td>
 <td class=\"centered\">Hash file</td>
+<td class=\"centered\">Current version</td>
 <td class=\"centered\">Warnings</td>
 </tr>
 """)