Allow multiple repositories

[dmonllao]

After switching to single to multiple and to multiple to single again we finally allow multiple repositories :) which means that every branch can use a different repo. With this we can run daily comparisons between moodle.git and integration.git

[stronk7]

LOL, I always thought that was done using remotes and branches (in the same clone).

so you have integration_master pointitng to integration.git/master and production_master to moodle.git/master

NVM. I suppose that will continue working (aka, is not mandatory to use separate clones).

Am I correct?

[dmonllao]

Yes yes, is will be in the same clone as it was; sorry for the naming confusion, I meant remote

[dmonllao]

This patch can also be cherry picked to MOODLE_25_STABLE

[dmonllao]

After integrating this the CI jobs should forward $beforebranchrepository and $afterbranchrepository to the setup script, and it should get them to write webserver_config.properties. I'm thinking of also adding those 2 values to defaults.properties, what do you think of it? Should we add them along with the other ones to make it consistent?

[stronk7]

Oki, I get the idea. We have 3 remotes defined (origin = base, before = 1st run, and after = 2nd run).

Some points / thoughts:

1) Semantical: Up to some point I really think that we are mixing nomenclature (before, after..) because this is very oriented to the compare.sh execution. Being honest each day I think on it I find it worse. I mean, we just have 1 base and 1 run to be defined. The fact of doing comparisons between 2 runs is something that comes later and it "artificially" makes us to run the "before" script only once. And then, the "after" script N times (along weeks or along issues..). Some day we should change this to, simply: a) base setup. b) make a run. NOT NOW, I only want to share the idea. Still the compare.sh script would continue having sense, but surely running a) and b) twice... but abandoning the before and after duplicate variables and scripts. Just a) and b).

2) While reviewing this... looking at your checkout_branch() code... wow, do we really need to drop the remote and re-add it on each execution? Perhaps the answer is yes, but it's so slow to be fetching everything for each run (now I understand some "pauses" in the execution where the computer seemed to be doing nothing, lol). Also, in the other side, ok, remotes are always rebuild. But what happens with local branches? I mean, imagine I'm a developer. And I've my base stable and also my "before" run, and then, I'm performing "after" runs for different issues and their branches in my repository (MDL-11111, MDL-22222... next week MDL-33333, MDL-44444)... who is in charge of cleaning those "after" branches once the run has finished? I can imagine the local clone storing all them forever, never being cleaned.

3) About having defaults for them, not sure. My idea is that we need defaults for all those things that don't need to be defined to get a specific run performed (commands, thresholds...) but are used to execute them. But all the variables, related with a run, (like the dbuser/pass... or the url, or the backup dir... or the repositories and the branches) really need to be defined/are related with the runs. And in fact the script should stop if they are undefined. So I'm not sure if they need a "default value" in the defaults file. Having those default values in the webserver_config dist file should be enough.

4) Finally it seems you've an error and have defined $afterbranchrepository twice!

Ciao :-)

PS: Ask me if some point is not clear, specially the 1st one is sort of "changing the mind/nomenclature". Still being able to execute 2 runs and compare them, but in a more clear separation of processes.

[dmonllao]

Thanks Eloy

1) I agree that a before and an after is not ideal, but I was thinking the same but from the opposite point of view, change the script to run a base + N branches, this would include your scenario.

It was done like it is to simplify life to developers, that are usually the more reticent ones to changes like having to run a tool before sending to integration; IMO if the tool is not simple enough they will just not do it and and we will detect the regressions once in integration.

2) The user can change the remote value ($repository, $beforebranchrepository and $afterbranchrepository vars) and we must update to the new ones, I've added a check to skip the rm if the repo address is the same, but for what I have seen the cost of remove and add it again is similar as there is some kind of cache (I'm a git internals ignorant). We need to fetch because we can fetch directly to hashes AFAIK.

About the cleaning, what do you propose? I can only think of clean_data.sh, but seems to radical to me, as the first fetch is when we really spend time.

3) We are not setting the defaults we are adding them to the .dist. I've added a pretty message if a provided branch/hash does not exist or it is wrong, the same for the remote. I've added few more checking in case wrong values are set or they are not set at all.

4) Thanks, I didn't spotted it because it was properly set in my webserver_config.properties... Corrected and I've also fixed another bug in the remote existence conditional that was preventing remotes to be removed.

Attaching a new patch with the mentioned differences.

[dmonllao]

https://github.com/dmonllao/moodle-performance-comparison/commit/ce83e83eaf45558b9887916cf260e7c358aef637

[stronk7]

it all looks ok for me +1!

[dmonllao]

Changes merged in master and 25. Closing