I was playing around with this today, and I discovered something a bit annoying.
Per this bug report, the tx client uses file timestamps to decide whether a resource is changed and needs to be downloaded, so if you’ve just done a git clone it will always think your local files are newer. You need to use the -f (force) option to work around this.
(This just means that we’re going to end up downloading all the translations on every build.)
@cintiadr, when I run the command that you helped me with in this PR, it shows there being changes even if there are none. Can you advise on how I should change this command?
For debugging, here is some output (after doing tx pull -f --mode=reviewed): (perhaps this has something to do with the force option I mentioned a couple messages back in this thread)
$ git diff-index HEAD
:100644 100644 0ce9adada3a71b4ca4b46df2ff7098e32a15385e 0000000000000000000000000000000000000000 M api/src/main/resources/messages_de.properties
:100644 100644 108e8ca49c75ec7c7a63a1d68a3feca40b863db3 0000000000000000000000000000000000000000 M api/src/main/resources/messages_fr.properties
:100644 100644 837b7527fe45bcef4080a88b1350d3620e98df4c 0000000000000000000000000000000000000000 M api/src/main/resources/messages_ht.properties
:100644 100644 366275184c0d2c44d2588aac886d73a2d6242797 0000000000000000000000000000000000000000 M api/src/main/resources/messages_pt.properties
The IF on the bash should not use the output of the command, but rather the exit code (0 is success, others are failure). To debug it, can you please run this?
#!/bin/bash
# Script to pull transifex translation files, and commit them to git if there are any changes.
tx pull -f --mode=reviewed
git diff-index --quiet HEAD --; RETURN_CODE=$?
echo "diff-index returned ${RETURN_CODE}"
if [[ "$RETURN_CODE" == "1" ]]; then
echo 'There are changes to be committed'
git commit -am "committing translations from transifex"
git push
fi
Hmm, I should note that (1) this is on my own mac (git version 2.7.4 (Apple Git-66)), not running on the agent, because I was just testing things out, and (2) I created the script in the checked-out folder.
Perhaps the behavior I’m seeing is related to this, but it appears that running git status changes the output of git diff-index. Like this:
nadjazayer:openmrs-module-uicommons djazayer$ tx pull -f --mode=reviewed
(blah blah, lots of output)
nadjazayer:openmrs-module-uicommons djazayer$ git diff-index --quiet HEAD --; echo $?
1
nadjazayer:openmrs-module-uicommons djazayer$ git status
On branch master
Your branch is up-to-date with ‘origin/master’.
Untracked files:
(use “git add …” to include in what will be committed)
test.sh
nothing added to commit but untracked files present (use “git add” to track)
nadjazayer:openmrs-module-uicommons djazayer$ git diff-index --quiet HEAD --; echo $?
0
So maybe this won’t be a problem when I actually do this on CI, e.g. should I be checking out this shell script into a different folder?
Apparently we need to run git update-index -q --refresh every single time we want to get an updated index, which is supposedly what a git status or git diff does.
Funny, uh?
Also, I discovered that the without the --quiet option, the command will always return 0 (usually the quiet mode only suppress the output, right? But this one changes exit behaviour too).
@cintiadr, thanks for investigating, you rock! (Because there was no way I was going to be able to figure this out in the background of my weeklong meetings.)
I tested both the allergyui (with changes) and uicommons (without changes),
and both appear to work fine with this version of the script (I just
prevented it from pushing the new commit).
So I think we can merge and I can create the build for any module you want,
just let me know how you want it to be (a manual stage? A separate build?)
So far I have only reviewed all translations for the uicommons module, so
for now only that one.
I’m concerned about the fact that this will add additional time to each
build (and chain of builds), but at the same time we want it to be
automatic.
Can you try adding it as a separate stage for only the uicommons module and
we can review the real world performance hit on the next few builds? (E.g.
reply on this thread saying what was the last build before you make the
change.)
I made it a separate stage, because I wanted it to not happen in parallel (maybe it can? Not sure).
There’s a side effect that if a commit happens, a new build will be triggered. Which makes me think if this shouldn’t be another build, that runs (let’s say) nightly.
In order to test commit/push, a change in translation should happen, as well as a new build in ui-commons.
Well, if you want a build that is triggered on schedule (instead of on commits) it should be trivial to do one single build for all the modules. But the question would be if that’s what it would be best!
@cintiadr, I just got a chance to look at the before and after timings on the UI Commons module whose build we updated, and…the data is a mess, I can’t interpret anything from it. (The 10 passing builds prior to the change took from 1-12 minutes. The passing builds since the change took from 2-5 minutes.)
Personally I don’t see any reason not to do this as a scheduled nightly job, that can loop over all the modules, if that’s easier to implement.
@cintiadr, having thought this through a bit more, I do think it makes sense to have a single job that does this nightly, rather than add it into every build plan.
I was also able to write a script that marked all translations up to this point as reviewed, so we’re ready to apply this to all the modules that are translated via transifex.
Do you have some time to write the bamboo task and/or script for this? (I can make a list that maps our github repos to their associated transifex projects, if that helps.)
Can you please check if the build is actually correct? Repositories, expected output, and everything. If that’s all correct, please go ahead and revert the commit, so it will actually push changes to github
I created https://wiki.openmrs.org/x/Y4ouBg with some steps on how to add a module to that build, but I’d assume the documentation is probably not good enough and it’s missing the first steps (adding the .tx folder to the module, do something on the transifex server, etc).
I didn’t include openmrs core, because I thought the user might not have commit access? Or maybe we don’t want it committing to the core, not sure.
Thanks @cintiadr, I peeked at a few examples, and they seem good. And thanks for creating the documentation wiki page. You get 10 bonus points for that. (Not sure what you can do with them…)
I used “nightly” as a euphemism for “about every 24 hours”, since the sun never sets for a geographically distributed open-source project. In other words, 1am UTC is fine. And we do not want to include openmrs-core, so we’re good there.
I reverted the safety commit and ran the plan, but there are errors. @cintiadr, I assume you’ll know in 2 minutes whether this something we should set on the agents, or as part of the update translations shell script:
https://ci.openmrs.org/browse/TRAN-TRAN-ALP-8
(I disabled the job just so someone looking at our build dashboard doesn’t see a red build.)
So, the official problem was, when doing the git push:
‘fatal: No path specified. See ‘man git-pull’ for valid url syntax’
The reason is that when Bamboo clones a repository, it doesn’t set the git ‘origin’, so there’s no upstream to be pushed to. When the plan has only one repository, it’s quite simple to get the URL of the repository (variable ${bamboo_planRepository_repositoryUrl} ).
(I also re-ran, and verified that it doesn’t commit anything if there are no changes. That worked for some of the jobs, but others timed out. Hopefully this doesn’t get too flaky.)
One thing: we’re going to eventually be translating things that are not modules (e.g. something like openmrs-owa-conceptdictionary) so we’ll eventually need to find a better hack for specifying the repo URL.