gettext icon indicating copy to clipboard operation
gettext copied to clipboard

Duplicate references in POT files and warnings about redefining modules

Open barttenbrinke opened this issue 2 years ago • 7 comments

When I run mix gettext.extract --merge

on my umrella project, I get a LOT of warnings saying:

warning: redefining module Core.Reports.ReportThree.Report (current version loaded from /_build/dev/lib/core/ebin/Elixir.Core.Reports.ReportThree.Report.beam)
  lib/core/reports/report_three.ex:24

Even if I delete _build - I think this is also causing this issue, on all of my POT & PO files, I get

#: lib/core/reports/report_three.ex:170
#: lib/core/reports/report_three.ex:170
#, elixir-autogen, elixir-format
msgid "Error"
msgstr "Error"

Which I assume is caused by the module being in memory twice or something, duplicating the reference. I now just uniq the generated PO and POT files as a workaround, but I'd rather understand what the core issue is here.

barttenbrinke avatar Aug 15 '23 10:08 barttenbrinke

Same issue here, now using a script like the following:

#! /usr/bin/env elixir
#
# fix-gettext <dir>
#
# Will dedup each file in the given dir, recursively.

System.argv()
|> Stream.flat_map(fn path ->
  if File.dir?(path) do
    Path.wildcard(Path.join(path, "**"))
  else
    path
  end
end)
|> Stream.reject(&File.dir?/1)
|> Stream.map(fn path ->
  data =
    path
    |> File.stream!()
    |> Enum.dedup()

  File.write!(path, data)

  IO.puts("Wrote to: #{path}")
end)
|> Stream.run()

arjan avatar Oct 23 '23 09:10 arjan

I believe this is caused by the same as #178

maennchen avatar Oct 23 '23 10:10 maennchen

@josevalim / @whatyouhide Would you potentially be open to a new extract methodology that should resolve the umbrella issue?

  • Add config value extraction_environments, which defaults to [:dev]
  • Add config value extraction_apps, which defaults to the current application
  • The gettext macros will register a module attribute containing all translations and register an @before_compile, which exposes the translations with an __translations__/0 function if the current env is contained in extraction_environments.
  • The Gettext.Extractor will read all modules translations from __translations__/0

Benefits of this approach:

  • mix gettext.extract does not have to recompile since all translations are already available from the normal compilation
  • This approach should easily work for umbrella applications or potentially even dependencies with a few tweaks

If you're open to this approach, I do have some time to try it and see how it performs.

maennchen avatar Oct 23 '23 10:10 maennchen

@maennchen that's a bigger change than I'd like. I’m ok to do it if it's the only way, but I wanna try to figure out if there is a simpler way instead. Have you tried playing a bit with the recompilation that is triggered by mix gettext.merge? Because if we recompile from scratch correctly we shouldn't be seeing these issues. 😕

whatyouhide avatar Oct 25 '23 12:10 whatyouhide

@whatyouhide I played around with it for a while in this project: https://github.com/jshmrtn/hygeia/tree/main

But even with a complete recompilation, I would run into multiple outcomes at random:

  • New Translations not added
  • Old Translations not removed
  • Or all good

I used this mix tak, which made it better, but didn't resolve it completely.

https://github.com/jshmrtn/hygeia/blob/e3ac39437c19041609162d70baf4ff6e96f846ea/apps/hygeia_gettext/lib/mix/tasks/gettext/extract/umbrella.ex

I haven't really been able to pinpoint the source of the issue.

Everything was resolved when I merged all apps.

maennchen avatar Oct 25 '23 13:10 maennchen