Userspace: add support for adding environment variables during build

[tzarc]

Description

A few complaints on Discord and the like for not being able to specify things like CONVERT_TO and FORCE_LAYOUT when using userspace.

This adds support for the usual -e/--env parameters to qmk userspace-add and qmk userspace-remove, and ensures such extra environment variables are propagated across to each build.

Userspace repo version is bumped to 1.1 as a result of the addition to the file format.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

FIxes

• Fixes [Feature Request] Allow forcing individual community layouts in qmk.json for userspace-compile and GitHub Actions #22850
• Fixes [Feature Request] Add output file argument to qmk compile #22590

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[bcat]

Thanks for this change! I tested it out with my userspace, and here are my findings:

1. The userspace-add and userspace-remove changes seem to work as expected. Here's the qmk.json file I ended up with:

   {
       "userspace_version": "1.1",
       "build_targets": [
           ["9key", "bcat"],
           [
               "ai03/polaris",
               "bcat",
               [
                   ["FORCE_LAYOUT", "60_tsangan_hhkb"]
               ]
           ],
           [
               "cannonkeys/an_c",
               "bcat",
               [
                   ["FORCE_LAYOUT", "60_tsangan_hhkb"]
               ]
           ],
           [
               "cannonkeys/instant60",
               "bcat",
               [
                   ["FORCE_LAYOUT", "60_tsangan_hhkb"]
               ]
           ],
           [
               "crkbd/rev1",
               "bcat",
               [
                   ["FORCE_LAYOUT", "split_3x6_3"]
               ]
           ],
           [
               "dz60",
               "bcat",
               [
                   ["FORCE_LAYOUT", "60_ansi_split_bs_rshift"]
               ]
           ],
           [
               "dz60",
               "bcat",
               [
                   ["FORCE_LAYOUT", "60_tsangan_hhkb"]
               ]
           ],
           ["eco/rev2", "bcat"],
           [
               "kbdfans/kbd67/hotswap",
               "bcat",
               [
                   ["FORCE_LAYOUT", "65_ansi_blocker_split_bs"]
               ]
           ],
           ["keebio/bdn9/rev1", "bcat"],
           ["keebio/quefrency/rev1", "bcat"],
           ["lily58/rev1", "bcat"],
           ["yanghu/unicorne/f411", "bcat"]
       ]
   }
   

2. The qmk userspace-compile command succeeds. However, despite running a separate build for each (keyboard, keymap, layout) combo, it doesn't seem to actually use the specified layouts:

   $ qmk userspace-compile -c
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   QMK Firmware 0.23.5
   Deleting .build/ ... done.
   Build 9key:bcat                                                        [OK]
   Build ai03/polaris:bcat                                                [OK]
   Build cannonkeys/an_c:bcat                                             [OK]
   Build cannonkeys/instant60:bcat                                        [OK]
   Build crkbd/rev1:bcat                                                  [OK]
   Build dz60:bcat                                                        [OK]
   Build dz60:bcat                                                        [OK]
   Build eco/rev2:bcat                                                    [OK]
   Build kbdfans/kbd67/hotswap:bcat                                       [OK]
   Build keebio/bdn9/rev1:bcat                                            [OK]
   Build keebio/quefrency/rev1:bcat                                       [OK]
   Build lily58/rev1:bcat                                                 [OK]
   Build yanghu/unicorne/f411:bcat                                        [OK]
   

   Notice that there's only one dz60 firmware output, not two as expected. And none of the firmware filenames have a layout suffix:

   $ ls -l *.{bin,hex}
   -rw-r--r-- 1 bcat bcat 29697 Jan 11 15:02 9key_bcat.hex
   -rw-r--r-- 1 bcat bcat 49166 Jan 11 15:00 ai03_polaris_bcat.hex
   -rwxr-xr-x 1 bcat bcat 31520 Jan 11 15:00 cannonkeys_an_c_bcat.bin
   -rwxr-xr-x 1 bcat bcat 31532 Jan 11 15:00 cannonkeys_instant60_bcat.bin
   -rw-r--r-- 1 bcat bcat 77540 Jan 11 15:00 crkbd_rev1_bcat.hex
   -rw-r--r-- 1 bcat bcat 49088 Jan 11 15:00 dz60_bcat.hex
   -rw-r--r-- 1 bcat bcat 45525 Jan 11 15:00 eco_rev2_bcat.hex
   -rw-r--r-- 1 bcat bcat 31043 Jan 11 15:00 kbdfans_kbd67_hotswap_bcat.hex
   -rw-r--r-- 1 bcat bcat 49125 Jan 11 15:00 keebio_bdn9_rev1_bcat.hex
   -rw-r--r-- 1 bcat bcat 54472 Jan 11 15:00 keebio_quefrency_rev1_bcat.hex
   -rw-r--r-- 1 bcat bcat 54967 Jan 11 15:00 lily58_rev1_bcat.hex
   -rwxr-xr-x 1 bcat bcat 78360 Jan 11 15:01 yanghu_unicorne_f411_bcat.bin
   

3. Compare to the expected firmware file output with a separate output file per layout:

   $ ls -l *.{bin,hex}
   -rw-r--r-- 1 bcat bcat 29697 Jan 11 15:03 9key_bcat.hex
   -rw-r--r-- 1 bcat bcat 49166 Jan 11 15:03 ai03_polaris_bcat_60_tsangan_hhkb.hex
   -rwxr-xr-x 1 bcat bcat 31520 Jan 11 15:03 cannonkeys_an_c_bcat_60_tsangan_hhkb.bin
   -rwxr-xr-x 1 bcat bcat 31532 Jan 11 15:03 cannonkeys_instant60_bcat_60_tsangan_hhkb.bin
   -rw-r--r-- 1 bcat bcat 77540 Jan 11 15:03 crkbd_rev1_bcat_split_3x6_3.hex
   -rw-r--r-- 1 bcat bcat 49088 Jan 11 15:03 dz60_bcat_60_ansi_split_bs_rshift.hex
   -rw-r--r-- 1 bcat bcat 49088 Jan 11 15:03 dz60_bcat_60_tsangan_hhkb.hex
   -rw-r--r-- 1 bcat bcat 45525 Jan 11 15:03 eco_rev2_bcat.hex
   -rw-r--r-- 1 bcat bcat 31043 Jan 11 15:03 kbdfans_kbd67_hotswap_bcat_65_ansi_blocker_split_bs.hex
   -rw-r--r-- 1 bcat bcat 49125 Jan 11 15:03 keebio_bdn9_rev1_bcat.hex
   -rw-r--r-- 1 bcat bcat 54472 Jan 11 15:03 keebio_quefrency_rev1_bcat.hex
   -rw-r--r-- 1 bcat bcat 54967 Jan 11 15:03 lily58_rev1_bcat.hex
   -rwxr-xr-x 1 bcat bcat 78360 Jan 11 15:04 yanghu_unicorne_f411_bcat.bin
   

4. Doing a dry run, it seems the environment variables aren't being passed to qmk compile:

   $ qmk userspace-compile -n
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Preparing target list...
   Ψ Compilation targets:
   Ψ qmk compile -kb 9key -km bcat
   Ψ qmk compile -kb ai03/polaris -km bcat
   Ψ qmk compile -kb cannonkeys/an_c -km bcat
   Ψ qmk compile -kb cannonkeys/instant60 -km bcat
   Ψ qmk compile -kb crkbd/rev1 -km bcat
   Ψ qmk compile -kb dz60 -km bcat
   Ψ qmk compile -kb dz60 -km bcat
   Ψ qmk compile -kb eco/rev2 -km bcat
   Ψ qmk compile -kb kbdfans/kbd67/hotswap -km bcat
   Ψ qmk compile -kb keebio/bdn9/rev1 -km bcat
   Ψ qmk compile -kb keebio/quefrency/rev1 -km bcat
   Ψ qmk compile -kb lily58/rev1 -km bcat
   Ψ qmk compile -kb yanghu/unicorne/f411 -km bcat
   

Thanks again for the help so far! Let me know if there's more testing I can do. :)

[tzarc]

Thanks for that -- the testing I'd done was with CONVERT_TO, which seemed to be working fine when I ran it. Guess FORCE_LAYOUT is a special snowflake.

Can I get you to attach .build/parallel_kb_builds.mk from after a qmk userspace-compile, please? Dry run is "fake" and it doesn't actually run the commands internally -- takes too long and doesn't parallelise well. That said, I'll make sure to include the extra vars in its output in the final mergeable PR.

[bcat]

Sure, attached (and renamed to make GitHub happy): parallel_kb_builds.mk.txt

If I'm reading this correctly, FORCE_LAYOUT=foo is getting passed along at least part of the way. I wonder if now I'm just hitting bug #22815 ?

What's interesting is that compiling with FORCE_LAYOUT using make directly in the userspace (like FORCE_LAYOUT=60_tsangan_hhkb make dz60:bcat) does build the specified layout and use the right filename.

[zvecr]

I wonder if now I'm just hitting bug #22815 ?

That would be my assumption.

FORCE_LAYOUT issue is somewhat due to locate_keymap in keymap.py where it processes community_layouts without considering the value of FORCE_LAYOUT.

#22815 (comment)

Chaining the context through would be possible, but messy (as that function is also used in other places or can come directly from the environment).

[bcat]

Random side note: Without understanding the underlying implementation details, as a user I feel like it would be nice if layouts were more a "first class" concept in keymap selection. Being able to just say qmk compile -kb dz60 -km bcat -layout 60_tsangan_hhkb, or something like that. I understand this may be nontrivial to do, though.

(Though maybe that doesn't make sense, because someone might have two keyboards with the same layout but still want to configure them differently, and build the whole shebang via userspace infra. So actually, on further thought, maybe the environment variables are better after all, even if they're more verbose.)

[sigprof]

Is the usage of nested arrays in the JSON representation for the environment variables intentional? I would expect an object there instead:

        [
            "ai03/polaris",
            "bcat",
            {
                "FORCE_LAYOUT": "60_tsangan_hhkb"
            }
        ]

(Maybe even the outer level should be an object with keys like keyboard, keymap, environment; that would be more verbose than the existing format though.)

[tzarc]

Is the usage of nested arrays in the JSON representation for the environment variables intentional? I would expect an object there instead:

Tomato/tomato. Will swap it over, it's of no real consequence.

[tzarc]

@bcat any luck on confirmation?

[bcat]

@bcat any luck on confirmation?

Sorry for the long delay! Yes, I've flashed the firmware files built with this PR and they seem correct to me. Thanks again!

[tzarc]

Thanks mate. Needs some cleanup before it's good to go, will see if can get it sorted over the next week or so.

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[drashna]

__attribute__((weak)) ✅

Tested locally and didn't seem to break anything.