crowdin / github-action

A GitHub action to manage and synchronize localization resources with your Crowdin project
https://crowdin.com
MIT License
163 stars 55 forks source link

Action attempts to push to wrong branch #252

Closed KTrain5169 closed 1 month ago

KTrain5169 commented 1 month ago

Describe the bug A clear and concise description of what the bug is.

The action appears to be attempting to commit changes to ${{ github.ref_name }} instead of the specified localization_branch_name.

To Reproduce Steps to reproduce the behavior:

  1. Set up workflow, with actions/checkout@v4 and the ref checked out being "main". The crowdin action also requires a value set for localization_branch_name.
  2. Create a new branch
  3. It fails because of pathspec ${{ github.ref_name }} didn't match any files known to git

crowdin@github-action@v2.1.1 No-crowdin.yml setup.

Expected behavior A clear and concise description of what you expected to happen.

It force-pushes downloaded localizations to the branch specified in localization_branch_name

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

The cron schedule method still works as expected.

andrii-bodnar commented 1 month ago

Hi @KTrain5169, I assume you are looking for the skip_ref_checkout option (Checking out multiple branches in a single workflow)

KTrain5169 commented 1 month ago

That could work, though I don't know why it only broke today

KTrain5169 commented 1 month ago

Can I also ask how the action checks for environmental variables? I have another workflow elsewhere which has the following setup:

- name: Grab strings from Crowdin
        uses: crowdin/github-action@v2.1.1
        with:
          upload_sources: true
          download_translations: true
          localization_branch_name: auto-i18n-branch
          create_pull_request: true
          pull_request_title: (Crowdin Action) New translation strings
          pull_request_body: New translated strings are now available!
          pull_request_base_branch_name: "main"
          github_user_name: Crowdin Bot (Actions)
          github_user_email: support+bot@crowdin.com
        env:
          CROWDIN_PROJECT_ID: ${{ secrets.CROWIN_PROJECT_ID }}
          CROWDIN_API_TOKEN: ${{ secrets.CROWDIN_TOKEN_THING }}

(the misspelling in the first secret name is intentional)

And the following crowdin.yml:

"project_id_env": "CROWDIN_PROJECT_ID"
"api_token_env": "CROWDIN_API_TOKEN"
"base_path": "."
"base_url": "https://api.crowdin.com"

"preserve_hierarchy": true

files:
  - source: /assets/minecraft/lang/en_us.json
    translation: /assets/minecraft/lang-crowdin/%locale_with_underscore%.json
    language_mapping:
      locale_with_underscore: # ABSOLULTELY DO NOT CHANGE THIS
        de: de_de
        en-UD: en_ud
        es-ES: es_es
        fr: fr_fr
        lol-US: lol_us
        pt-BR: pt_br
        ru: ru_ru
    upload_language: en
    skip_untranslated_strings: false
    export_only_approved: false

No matter how I do it, the action always detects the Project ID, even if the field is removed in the crowdin.yml. However, api_token always comes up as missing, and I don't know why. I've tried basically every way I can think of to fix it and it still complains that api_token is missing.

andrii-bodnar commented 1 month ago

@KTrain5169 Thank you for the details! I'm still not sure I fully understand the issue. Could you please provide a full GitHub workflow and the error logs from the failed action logs?

KTrain5169 commented 1 month ago

https://github.com/Pridecraft-Studios/pridepack/actions/workflows/auto-i18n.yml

This is the run history for the workflow that we use. Pretty much all of the latest logs always say that API_TOKEN was missing, even though it was entered corretly. I tried multiple different ways to get it but it ended up not working still. Getting rid of the Project ID field in the crowdin.yml still says that only the api_token was missing despite the project ID field clearly gone

andrii-bodnar commented 1 month ago

@KTrain5169 the issue is in the name of the token env variable.

the action always detects the Project ID, even if the field is removed in the crowdin.yml

It's because of the automatic env variables detection.

Please use either the CROWDIN_PERSONAL_TOKEN name for the token secret or pass the Action env variables as shown in the example.

KTrain5169 commented 1 month ago

I think I tried that before but I will try that again.

I assume it is safe to remove both fields from the crowdin.yml?

andrii-bodnar commented 1 month ago

I assume it is safe to remove both fields from the crowdin.yml?

Yes, should be safe.

KTrain5169 commented 1 month ago

Thanks! Sorry if I'm being annoying, but there's one more issue.

The bot now puts all the project stuff into the wrong folder and I changed it back to /lang/ however the bot doesn't change it back. Do I have to close the PR and rerun the Action to get it to place the files in the right directory?

Also, it's not respecting my language mappings, even though I have clearly defined them in the crowdin.yml

andrii-bodnar commented 1 month ago

Yes, try closing the PR and deleting the localization branch.

KTrain5169 commented 1 month ago

Ah, I figured the problem: I was checking out the "main" branch which had an outdated crowdin.yml file.

The language mappings are still not being respected however. Am I doing something wrong with the language mappings? I've ensured it sticks to the Crowdin language code: Code we use format but it doesn't respect it.

andrii-bodnar commented 1 month ago

Based on the latest workflow run from your repo:

⚠️  Downloaded translations don't match the current project configuration. The translations for the following sources will be omitted (use --verbose to get the list of the omitted translations):
    - lol_us.json (9)
    - en_us.json (9)
    - pt_br.json (9)
    - es_es.json (9)
    - de_de.json (9)
    - ru_ru.json (9)
    - en_ud.json (9)
    - fr_fr.json (9)
Visit the https://crowdin.github.io/crowdin-cli/faq for more details

It's probably some misconfiguration. Please read the first section of the FAQ.

KTrain5169 commented 1 month ago

Ah, that's interesting. I suppose I could try changing with CLI, but I already ran sources upload through the action (at least I assume that does the same thing as the CLI), so I'll wait until our Crowdin manager is available and tell him to change it. Thanks :)

Just so I am 100% sure on what to change it to, does the %locale_with_underscore% have an equivalent option but for lowercasing the locale? This is a hard requirements because of the project we are doing.

andrii-bodnar commented 1 month ago

I suppose I could try changing with CLI, but I already ran sources upload through the action (at least I assume that does the same thing as the CLI)

The Action uses CLI under the hood.

Just so I am 100% sure on what to change it to, does the %locale_with_underscore% have an equivalent option but for lowercasing the locale? This is a hard requirements because of the project we are doing.

Language mapping is designed to address these issues.

KTrain5169 commented 1 month ago

Language mapping is designed to address these issues.

I see. So if I change the language_mapping option to change it from, say, es_ES to es_es, would that require mapping es or es_ES?

andrii-bodnar commented 1 month ago

Please refer to the docs - https://crowdin.github.io/crowdin-cli/advanced#languages-mapping-configuration

KTrain5169 commented 1 month ago

The docs are a bit unclear for me. It does tell me to use the language codes found at https://support.crowdin.com/developer/language-codes/ with the format es-ES: es_es (which I've done already), but it doesn't seem to work. It also doesn't provide an example with zh_CN but I assume that the point is to make the mapping like Crowdin lang name from the list: our lang name, which doesn't appear to work as stated before.

andrii-bodnar commented 1 month ago

@KTrain5169 you are using language_mapping in your crowdin.yml configuration file. But, according to the docs, it should be languages_mapping (with s).

The es-ES: es_es mapping is correct.

KTrain5169 commented 1 month ago

Oh I see. Perhaps some new glasses is in order. Thanks for figuring that out and sorry for being a pain in general 😅