RDTK / generator

A tool for creating Jenkins jobs and other things from recipes describing software projects
GNU General Public License v3.0
21 stars 3 forks source link

Freeze distribution #37

Open LeroyR opened 4 years ago

LeroyR commented 4 years ago

Is your feature request related to a problem? Please describe. We often want to freeze a distribution for e.g. submissions or experiments. It would be useful to have a command that automates the process.

Describe the solution you'd like A new freeze command that recreates a given distribution file, setting the commit for all projects to the hash of the last successful build from jenkins

scymtym commented 4 years ago

Is the prototypical implementation in the wip-robocub branch (binaries here) close to what you need?

scymtym commented 4 years ago

Is the prototypical implementation in the wip-robocub branch (binaries here) close to what you need?

Sorry, I think I meant the feature-auto-tagging branch (binaries here).

LeroyR commented 4 years ago

The is no usage information in the feature-auto-tagging changelog. The job also does not show the changes. So i need some help how to use this branch.

scymtym commented 4 years ago

The is no usage information in the feature-auto-tagging changelog. The job also does not show the changes. So i need some help how to use this branch.

The changelog entry ended up in the section for an (by now) old release. I updated the branch so that build-generator version -c 1 should now provide the expected usage information.

LeroyR commented 4 years ago

It works fine for the one project in my distribution file but it does not expand stacks which makes it pretty useless for me atm.

I also first forgot to give the mode so i had some fun debugging. As i got no build (missing) for every job. --debug does not increase the logging in the retrieve step

Forgot token/user first which gave the same messages, but was able to look at the api endpoints from an incognito tab. Does missing access gives some different error?

 0.00 % RETRIEVE-BUILD-DATA
 0.00 % RETRIEVE-BUILD-DATA: tiago-kinetic-robocup:cmu-openpose:v1.3.0:main
 0.69 % RETRIEVE-BUILD-DATA: tiago-kinetic-robocup:maven-repo-cleanup:master:main
...
100.00 % RETRIEVE-BUILD-DATA
...
maven-repo-cleanup               master                           no build «missing» 

There also some warning spam

 <WARN> [15:01:28] [main thread] build-generator.commands command-release-distribution.lisp (compute-changes) -
  Skipping #<LOCATION /home/lruegeme/projects/citk/distributions/stacks/gazebo8.distribution: 734-747 {100886B8D3}>
 <WARN> [15:01:28] [main thread] build-generator.commands command-release-distribution.lisp (compute-changes) -
  Skipping #<LOCATION /home/lruegeme/projects/citk/distributions/stacks/gazebo8.distribution: 750-779 {100886B8E3}>
... 200 lines

I guess this will be gone by supporting includes

Proposed changes:

scymtym commented 4 years ago

Forgot token/user first which gave the same messages, but was able to look at the api endpoints from an incognito tab. Does missing access gives some different error?

I just tried it. When a job is inaccessible due to insufficient permissions, Jenkins returns 404 (which kind of makes sense because attackers could otherwise probe for existing jobs).

There also some warning spam.

The warnings were for projects that were not lexically in the specified distribution file. They are no longer produced.

expand inclusions, adding a comment around the projects from the stack stating the original path

Implemented.

(debug) print the tried urls/api endpoints

Not really implemented but Jenkins-related errors are now reported properly.

add a hint if no job was found, along the lines of:

Implemented.

This build includes the improvements.

LeroyR commented 4 years ago

expand inclusions, adding a comment around the projects from the stack stating the original path

Implemented.

The include is still in the output file, should be removed after expanding.

I would also like if all "error" projects could be grouped to the end of the file or marked differently. Different errors are currently encoded in the commit of failed projects e.g. NIL, UNSUPPORTED-SCM which makes it hard to manually fix these (multiple searches, easy to miss)

scymtym commented 4 years ago

The include is still in the output file, should be removed after expanding.

Should be fixed in this build.

I would also like if all "error" projects could be grouped to the end of the file or marked differently. Different errors are currently encoded in the commit of failed projects e.g. NIL, UNSUPPORTED-SCM which makes it hard to manually fix these (multiple searches, easy to miss)

How about a non-zero exit code and not writing the output file? Seems much simpler.

LeroyR commented 4 years ago

I would also like if all "error" projects could be grouped to the end of the file or marked differently. Different errors are currently encoded in the commit of failed projects e.g. NIL, UNSUPPORTED-SCM which makes it hard to manually fix these (multiple searches, easy to miss)

How about a non-zero exit code and not writing the output file? Seems much simpler.

Not generating without a --force switch (and accompanying prompt) sounds better.

I would still like to use this in distributions that have some projects with e.g. UNSUPPORTED SCM where one can then manually add the commit or maybe create a version. Another thing may be: looking at the generated file can remind me to create a permalink or pub entry for needed training data.

I personally would be fine with a simple #TODO <error> after the commit: so i can grep the file for that but that does not sound like the cleanest solution.

scymtym commented 4 years ago

I personally would be fine with a simple #TODO after the commit: so i can grep the file for that but that does not sound like the cleanest solution.

In this build.

Not generating without a --force switch (and accompanying prompt) sounds better.

An (ignorable) error is now reported before writing the output file.

Another thing may be: looking at the generated file can remind me to create a permalink or pub entry for needed training data.

Not sure what you mean in terms of the written distribution file.

scymtym commented 4 years ago

I made a couple of additional changes. Please use the last successful build.

scymtym commented 4 years ago

Did the most recent changes work OK for you?

semeyerz commented 4 years ago

Thanks for working on this feature. I found the following problems with the current implementation:

  1. When the source distribution has multiple versions of a project the output contains only one entry.

example:

- name: ignition-math
  versions:
  - version: ign-math2
  - version: ign-math3
  1. the header might contain sensible information like the key

  2. Some entries are completely missing. The output seems to be merged with some results. example:

commit: # TODO build not found8055413ad594fd486ac7a34ebefcd55802

here the linux-headers-rtai project is missing completely and its hash has become part of the comment of the sync entry.

  1. commit entry for unsupported scms. Shouldn't the "commit:" entry be left out if no commit could be found?
scymtym commented 4 years ago

the header might contain sensible information like the key

Can you provide more details? Passwords and API tokens should already be removed from the printed commandline.

commit entry for unsupported scms. Shouldn't the "commit:" entry be left out if no commit could be found?

The commit: # TODO … entries are generated because an action is required in order to get an at least somewhat reproducible released distribution. See https://github.com/RDTK/generator/issues/37#issuecomment-525685611.

scymtym commented 4 years ago

the header might contain sensible information like the key

I tried to reproduce this, but I got Commandline 'release-distribution' … '-u' 'test' '-p' '********' … and Commandline 'release-distribution' … '-u' 'test' '-a' '********' …`.

semeyerz commented 4 years ago

the header might contain sensible information like the key

I tried to reproduce this, but I got Commandline 'release-distribution' … '-u' 'test' '-p' '********' … and Commandline 'release-distribution' … '-u' 'test' '-a' '********' …`.

I called it with '-p=APIKEY'. Maybe you filter only for -pWHITESPACE?

LeroyR commented 4 years ago

last build only works with user/password. Using user/token gives:

Could not connect to Jenkins instance at https://localhost:8080/. Caused by:
> Object not found at https://localhost:8080/ (code 401):
>
>   <html>
>   <head>
>   <meta http-equiv="Content-Type" content="text/html;charset=utf-8"/>
>   <title>Error 401 Invalid password/token for user: </title>
>   </head>
>   <body><h2>HTTP ERROR 401</h2>
>   <p>Problem accessing /. Reason:
>   <pre>    Invalid password/token for user: </pre></p><hr><a href="http://eclipse.org/jetty">Powered by Jetty:// 9.4.z-SNAPSHOT</a><hr/>
>   
>   </body>
>   </html>
>   

Also: could the command reuse command.generate.XX variables?

Currently it uses command.release-distribution.XX variables which means calling both commands with the same cmdline can still result in incompatible settings.

scymtym commented 4 years ago

last build only works with user/password. Using user/token gives

Should be fixed in most recent build.

the header might contain sensible information like the key

Same.

could the command reuse command.generate.XX variables?

I would rather not do that. In fact, I'm not sure the release-distribution command should use the configuration file at all since information from that source will not appear in the generated header. Doing everything explicitly when releasing a distribution doesn't seem like a bad idea to me.

I'm still working on the problems related to including multiple versions of a given project.

LeroyR commented 4 years ago

could the command reuse command.generate.XX variables?

I would rather not do that. In fact, I'm not sure the release-distribution command should use the configuration file at all since information from that source will not appear in the generated header. Doing everything explicitly when releasing a distribution doesn't seem like a bad idea to me.

Well, it could dump all vars into the header. It should at least reuse api-key/pass/user. Maybe we could move that to a global scope?