Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure

From: Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure
Date: 2021-06-30 07:28:11
Message-ID: CAKtn9dMcVSjEwjeOfZC5Kt7CD-YUmQ=W9fFN4XXGSCLsT-oyzw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi All,

Please find the attached patch for resolving this issue wrt above
suggestion.

Let me know if anyone has any queries.

On Tue, Jun 29, 2021 at 9:20 PM Rahul Shirsat <
rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:

> Hi Dave / Aditya,
>
> For a time being, Let's make a call to gettext conditional instead of
> passing dynamic parameters for this scenario at least. With this, we won't
> be touching the *.po files and translations will do its task smoothly.
> I have already checked for the string with weird unprintable characters,
> and there were none.
>
> e.g.
> Instead of - *title = gettext('%s objects?', obj.label);*
>
> *if (drop) {*
> * title = gettext('Drop objects?');*
> *}*
> *else {*
> *title = gettext('Reassign objects?');*
> *}*
>
> I have tried the above code and it looks good to me.
>
> Please suggest.
>
> On Tue, Jun 29, 2021 at 7:31 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> On Tue, Jun 29, 2021 at 2:55 PM Aditya Toshniwal <
>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>
>>> Hi,
>>>
>>> On Tue, Jun 29, 2021 at 7:14 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jun 29, 2021 at 2:41 PM Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Dave,
>>>>>
>>>>> Somehow, the new text strings are added to PO with incorrect
>>>>> translations. That is causing the issue.
>>>>> Either they should be empty or fixed.
>>>>>
>>>>
>>>> Then the source problem should be fixed. There's no point at all in
>>>> putting fixes directly in the PO files as they'll be overwritten prior to
>>>> release anyway.
>>>>
>>> The translations submitted by translators are updated in the PO file
>>> right ? And then they're compiled to MO ?
>>>
>>
>> Right.
>>
>>
>>> It's the same like Rahul will be submitting the translations. Please
>>> correct me if I'm wrong.
>>>
>>
>> That depends if he's changed the original message or the translated
>> message. It's impossible to see without reading megabytes of text.
>>
>> And in any case, he's updated translations that haven't been touched in
>> ages - why would they suddenly have broken? (hint: if the upstream message
>> has changed, it'll be marked as fuzzy or untranslated - in other words,
>> gettext knows how to handle that).
>>
>>
>>>
>>>>
>>>>>
>>>>> On Tue, Jun 29, 2021 at 7:01 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> Please send the patch without updates to the po files. Those get
>>>>>> updated as part of the release process.
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> On Tue, Jun 29, 2021 at 2:00 PM Rahul Shirsat <
>>>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Hackers,
>>>>>>>
>>>>>>> Thanks Aditya for pointing out the issue. Please find the attached
>>>>>>> patch which contains all the .po files corrected with %s.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Rahul Shirsat.
>>>>>>>
>>>>>>> On Tue, Jun 29, 2021 at 4:31 PM Aditya Toshniwal <
>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Rahul,
>>>>>>>>
>>>>>>>> I did "make msg-extract" and "make msg-update" and looking at the
>>>>>>>> PO files I think it is not updated correctly.
>>>>>>>> For instance, the below message has msgstr without %s. I corrected
>>>>>>>> it and the error was gone.
>>>>>>>>
>>>>>>>> #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:766
>>>>>>>> #, fuzzy, python-format
>>>>>>>> msgid "%s Objects"
>>>>>>>> msgstr "Obiekty"
>>>>>>>>
>>>>>>>> The one below had 2 %s in msgstr and I corrected it to fix the
>>>>>>>> error.
>>>>>>>>
>>>>>>>> #: pgadmin/browser/server_groups/servers/roles/static/js/role.js:767
>>>>>>>> #, fuzzy, python-format
>>>>>>>> msgid "Are you sure you wish to %s all the objects owned by the
>>>>>>>> selected role?"
>>>>>>>> msgstr "Czy na pewno skasować %s \"%s\" i wszystkie obiekty zależne
>>>>>>>> od niego?"
>>>>>>>>
>>>>>>>>
>>>>>>>> You have to update the .po files to match the total %s and send the
>>>>>>>> patch.
>>>>>>>>
>>>>>>>> On Tue, Jun 29, 2021 at 1:56 PM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Tue, Jun 29, 2021 at 4:38 AM Rahul Shirsat <
>>>>>>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> I feel gettext sometimes won't escape the characters as it should
>>>>>>>>>> be.
>>>>>>>>>>
>>>>>>>>>> I now tried to escape those using some utils.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> That won't work either. The string being passed to gettext()
>>>>>>>>> *must* be in the gettext call.
>>>>>>>>>
>>>>>>>>> When gettext extracts strings to create/update the catalogs, it
>>>>>>>>> will search the code for all gettext calls, and then extract a string
>>>>>>>>> constant from the first argument. You cannot have variables, function calls
>>>>>>>>> or expressions in there. It *must* be a string constant.
>>>>>>>>>
>>>>>>>>> Keep in mind that msgextract is scanning the source code; it's not
>>>>>>>>> executing it. There are many examples in the code, e.g. (from node.js):
>>>>>>>>>
>>>>>>>>> title = gettext('Drop %s?', obj.label);
>>>>>>>>>
>>>>>>>>> I don't see anything obviously wrong with the existing code. Are
>>>>>>>>> you sure there are no weird unprintable characters in there?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Please find the updated patch.
>>>>>>>>>>
>>>>>>>>>> On Mon, Jun 28, 2021 at 9:33 PM Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi
>>>>>>>>>>>
>>>>>>>>>>> On Mon, Jun 28, 2021 at 4:57 PM Rahul Shirsat <
>>>>>>>>>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>
>>>>>>>>>>>> Please find the attached patch for fixation of jenkins failure.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> That won't work - you can't include variables (or string
>>>>>>>>>>> building operations) in the first argument to gettext calls, as there won't
>>>>>>>>>>> be any way to extract a complete message into the catalogs. The way it's
>>>>>>>>>>> being done at the moment is correct (I don't know why it's failing, but
>>>>>>>>>>> it's the correct way to structure the gettext calls).
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Dave Page
>>>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>
>>>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> *Rahul Shirsat*
>>>>>>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Dave Page
>>>>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>
>>>>>>>>> EDB: https://www.enterprisedb.com
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Thanks,
>>>>>>>> Aditya Toshniwal
>>>>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>>>>> <http://edbpostgres.com>
>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> *Rahul Shirsat*
>>>>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: https://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EDB: https://www.enterprisedb.com
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Aditya Toshniwal
>>>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>>>> <http://edbpostgres.com>
>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: https://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EDB: https://www.enterprisedb.com
>>>>
>>>>
>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
>>> <http://edbpostgres.com>
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> Dave Page
>> Blog: https://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: https://www.enterprisedb.com
>>
>>
>
> --
> *Rahul Shirsat*
> Senior Software Engineer | EnterpriseDB Corporation.
>

--
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.

Attachment Content-Type Size
jenkins_failure_fix_v4.patch application/octet-stream 3.0 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2021-06-30 08:22:53 Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure
Previous Message Rahul Shirsat 2021-06-29 15:50:10 Re: [patch][pgAdmin] Fix for pgadmin4-linux-qa #1651 failure