Mirroring scripts by vladak · Pull Request #1975 · oracle/opengrok

vladak · 2018-01-11T15:08:48Z

This set of changes adds a bunch of Python scripts to synchronize SCM repositories (git, Mercurial, CVS, SVN, Teamware are supported) with their parents/upstream. Plus pre-requisite changes to Messages.

The main mirror.py script is supposed to be run per-project via the already existing sync.py script.

fixes oracle#1970

fixes oracle#1969

tulinkry · 2018-01-11T15:57:22Z

tools/Messages

Update the main README as well

fixes oracle#1973

vladak · 2018-01-16T10:15:10Z

I found a Pythonista in the company to do additional (sic!) review for the python scripts, still need someone to look at the Messages changes. Also, I will probably switch our production instance to these scripts to give it additional testing.

tulinkry · 2018-01-16T10:18:20Z

src/org/opensolaris/opengrok/configuration/messages/ConfigMessage.java

+     * @return string representation of the field value
+     * @throws java.io.IOException 
+     */
+    protected String invokeGetter(Configuration config, String field) throws IOException {


this may be a candidate for a util function along with invokeSetter

you mean moving this to org.opensolaris.opengrok.util as a utility class ? or possibly to Configuration.java ?

t thought more about a new utility class but you decide both is better than have it here

or maybe if you extract this to the ClassUtils and make it more generic (for any object and not just for configuration) and then you can delegate these calls from here. How about that?

yep, that's what I did.

tulinkry · 2018-01-16T10:19:22Z

src/org/opensolaris/opengrok/configuration/messages/ConfigMessage.java


- /*
+/*
 * Copyright (c) 2017, 2017, Oracle and/or its affiliates. All rights reserved.


fixed, hopefully we won't have to deal with this anymore soon.

tulinkry · 2018-01-16T10:21:05Z

src/org/opensolaris/opengrok/configuration/messages/ConfigMessage.java

                + toInteger(hasTag("auth"))
+                + toInteger(hasTag("get"))
                + toInteger(hasTag("set")) > 1) {
            throw new Exception("The message tag must be either setconf, getconf, auth or set");


fixed by rewriting this test

tulinkry · 2018-01-16T10:21:58Z

src/org/opensolaris/opengrok/configuration/messages/ProjectMessage.java


- /*
+/*
 * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.


tulinkry · 2018-01-16T10:25:42Z

src/org/opensolaris/opengrok/configuration/messages/RepositoryMessage.java

+
+        switch (msgtext) {
+            case "get-repo-type":
+                List<String> types = new ArrayList<>();


maybe a linked list? or array list with known capacity? to reduce the unnecessary overhead

used specified initial capacity

tulinkry · 2018-01-16T10:29:06Z

src/org/opensolaris/opengrok/configuration/messages/RepositoryMessage.java

+                        types.add("N/A");
+                    }
+                }
+                ret = types.stream().collect(Collectors.joining("\n")).toString();


just some thoughts..

this gives for a message with tags
-t opengrok -t opengrok-master -t nothing
the result

git git N/A

isn't something like

opengrok: git opengrok-master: git nothing: N/A

better?
or some json?

okay, will use the colon separated output.

I will save JSON output for #1801.

tulinkry · 2018-01-16T10:31:40Z

src/org/opensolaris/opengrok/configuration/messages/RepositoryMessage.java

+    @Override
+    public void validate() throws Exception {
+        String command = getText();
+        Set<String> allowedText = new TreeSet<>(Arrays.asList("get-repo-type"));


allowedTexts

tulinkry · 2018-01-16T10:32:42Z

test/org/opensolaris/opengrok/configuration/messages/ProjectMessageTest.java

- /*
+/*
 * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
 * Portions Copyright (c) 2017, Chris Fraire <cfraire@me.com>.


tulinkry · 2018-01-16T10:34:02Z

test/org/opensolaris/opengrok/configuration/messages/RepositoryMessageTest.java

+    public void setUp() throws IOException {
+        Assume.assumeTrue(new MercurialRepository().isWorking());
+        Assume.assumeTrue(new SubversionRepository().isWorking());
+        Assume.assumeTrue(new GitRepository().isWorking());


I think I included the repeateable conditional rule for this, can you try it?

these are not needed, removed.

tulinkry · 2018-01-16T10:35:15Z

test/org/opensolaris/opengrok/configuration/messages/RepositoryMessageTest.java

+        Assume.assumeTrue(new MercurialRepository().isWorking());
+        Assume.assumeTrue(new SubversionRepository().isWorking());
+        Assume.assumeTrue(new GitRepository().isWorking());
+


do you need git and subversion in this test?

technically only the mercurial is needed. I have extended the test to check git repo as well and used the repeated annotation.

vladak · 2018-01-16T11:07:15Z

For the record, here are the python comments:

(11:25:01) timf: vladimir.kotal: platform/solaris/ips/create.sh looks pretty antiquated. You really should replace that with a single p5m manifest and pkgsend publish
(11:25:15) timf: rather than publishing via repeated pkgsend add actions
(11:37:09) timf: tools/sync/hook.py looks a bit weird too. There's a 'cwd' parameter for popen call, so you're kinda going the long way around here.
(11:37:46) jan.parcel left the room.
(11:41:13) timf: tools/sync/mirror.py line 140, no need for the '+' at the end of the line
(11:41:28) timf: string literals continued across lines are automatically concatenated.
(11:42:17) timf: line 151, and elsewhere I'd use double quotes rather than single quotes - generally, just pick a style and stick with it.
(11:43:25) timf: line 184, no spaces between the name, the equals sign and the value. eg. good: parm_name=param_value, bad: param_name = param_value
(11:45:18) timf: line 237 should probably be log.error rather than log.debug
(11:49:38) timf: tools/sync/teamware.py a bit of extra safety might be to verify that os.path.isdir(command) before proceeding.
(11:50:19) timf: line 43, copy/paste coding s/hg_command/bringover_command/
(11:53:48) timf: tools/sync/utils.py def which(..) can be replaced by shutil.which(..) if you're guaranteed to have python3.
(11:53:48) timf: >>> import shutil

shutil.which("cc")
'/opt/solarisstudio12.4/bin/cc'

(11:58:28) timf: line 28, os.mkdir should perhaps be os.makedirs, so as to make any intermediate dirs along the way
(12:00:42) timf: the get_dict_val function seems a bit weird, not wrong, just.. odd. (you know you can do dictionary.get(key) and it'll return None if the key doesn't exist, or dictionary.get(key, default) and it'll return default if key doesn't exist)
(12:00:55) timf: ok, I'm done. Generally, nothing too weird there.

tarzanek · 2018-01-16T14:05:53Z

looking forward to try it out once merged, thank you Vlad :)

vladak · 2018-01-16T15:46:03Z

@tarzanek I am a bit worried about the Python scripts having hardcoded Python 3.4 so might not run on anything else (3.5) even if it could.

tarzanek · 2018-01-16T17:03:54Z

d'oh ... so once again the history repeats and the same language is not compatible with its own old versions?

vladak · 2018-01-17T08:42:59Z

Well, python 2 and python 3 are different languages. This is not C. That said, I can probably make the script headers use Python 3 or even generic Python (since the script have embedded version checks) and hope it will run on all 3.x. Dne 16. 1. 2018 6:04 odp. napsal uživatel "Lubos Kosco" < notifications@github.com>:

…

d'oh ... so once again the history repeats and the same language is not compatible with its own old versions? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#1975 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACzGDDGGPYwRXiNzNQ_aJYKWfwDEFX_yks5tLNZ7gaJpZM4Ra_Lv> .

tulinkry · 2018-01-17T09:29:04Z

how about /usr/bin/env python3 ? this should lead to the recent version of python3 on the system, or?

vladak · 2018-01-17T12:30:34Z

I was thinking about python3 however that would not work on Solaris. And the most recent version guarantee does not universally hold true.

tulinkry · 2018-01-17T12:55:35Z

you can add a check in the code that the version is atleast 3 and throw an exception?

vladak · 2018-01-17T13:16:27Z

The version check is already present however this is still not going to work on a system which has both Python 2 and 3 but /usr/bin/python points to Python 2.

tulinkry · 2018-01-17T13:20:44Z

Ok I didn't see it

vladak · 2018-01-17T13:54:33Z

I will just bite the bullet and use python3 at the expense of having to create python3 symlink on systems that do not have it.

tarzanek · 2018-01-17T13:57:39Z

I agree, since it seems most OSes have python3 symlink, no ?

thnx
L

vladak · 2018-01-17T14:02:58Z

Ubuntu has the symlink
macOS has the symlink if Python 3.x was installed from Homebrew
Solaris does not have it

tulinkry · 2018-01-17T14:07:17Z

yes I am for python3

tarzanek · 2018-01-17T14:23:37Z

windows also doesn't have it, but I am OK with Ubuntu(Debian) and macOS :)
(and I think RHEL/Centos have it too)

Vladimir Kotal added 3 commits January 10, 2018 16:32

add config get Message

4221a32

fixes oracle#1970

2018

11def00

add repository Message

7931f28

fixes oracle#1969

tulinkry reviewed Jan 11, 2018

View reviewed changes

add repository mirroring scripts

8f71c1b

vladak force-pushed the mirroring_scripts branch from bce4b60 to 8f71c1b Compare January 12, 2018 14:19

Vladimir Kotal added 2 commits January 12, 2018 15:54

nicer logs

a49f1f7

check setconf tag properly

2e55a70

fixes oracle#1973

tarzanek added this to the 1.1 milestone Jan 12, 2018

tulinkry reviewed Jan 16, 2018

View reviewed changes

tarzanek assigned vladak Jan 16, 2018

vladak force-pushed the mirroring_scripts branch from 6fe3889 to 1146e34 Compare January 16, 2018 16:45

Vladimir Kotal added 2 commits January 16, 2018 17:54

fix comments from Krystof

9d1714f

fix javadoc

f70fbdf

vladak force-pushed the mirroring_scripts branch from 1146e34 to f70fbdf Compare January 16, 2018 16:55

vladak mentioned this pull request Jan 16, 2018

platform/solaris/ips/create.sh looks pretty antiquated #1978

Closed

fix python code comments from timf

1df674a

vladak force-pushed the mirroring_scripts branch from 1bc5256 to 1df674a Compare January 17, 2018 13:28

use python3 instead of specific version

da6ee65

document python3

d598ace

test env

9256e88

vladak merged commit 198a733 into oracle:master Jan 17, 2018

vladak mentioned this pull request Apr 16, 2018

platform/solaris/ips/create.sh looks pretty antiquated OpenGrok/platform#1

Open

Conversation

vladak commented Jan 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladak commented Jan 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tulinkry Jan 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladak commented Jan 16, 2018

Uh oh!

tarzanek commented Jan 16, 2018

Uh oh!

vladak commented Jan 16, 2018

Uh oh!

tarzanek commented Jan 16, 2018

Uh oh!

vladak commented Jan 17, 2018 via email

Uh oh!

tulinkry commented Jan 17, 2018

Uh oh!

vladak commented Jan 17, 2018

Uh oh!

tulinkry commented Jan 17, 2018

Uh oh!

vladak commented Jan 17, 2018

Uh oh!

tulinkry commented Jan 17, 2018

Uh oh!

vladak commented Jan 17, 2018

Uh oh!

tarzanek commented Jan 17, 2018

tulinkry Jan 16, 2018 •

edited

Loading

tarzanek commented Jan 17, 2018 •

edited

Loading