FPGAwars / apio

:seedling: Open source ecosystem for open FPGA boards
https://github.com/FPGAwars/apio/wiki
GNU General Public License v2.0
772 stars 131 forks source link

[Proposal] Moving the config settings from settings.json to apio.ini and deprecating the apio config command. #357

Open zapta opened 4 months ago

zapta commented 4 months ago

This is a proposal for discussion.

Moving the two config settings from profile.json to the project's apio.ini (with proper default values such that most users will not need them) and deprecate the apio config command.

    "config": {
        "exe": "default",
        "verbose": "0"
    },

Motivation: Simplification, consistency, and separation between user settings and the auto installed tools. Basically replicating the successful model of platformio where platformio.ini defines the entire configuration.

If it's OK, I can try making the change.

@Obijuan, any thoughts?

Obijuan commented 4 months ago

I fully agree with this proposal. All the configuration stuff should be moved to apio.ini

zapta commented 4 months ago

Thanks Juan. I will find time for it.

apio.ini has a lot of potential and I hope it will expand.

On Tue, Feb 27, 2024 at 9:05 PM Juan Gonzalez-Gomez < @.***> wrote:

I fully agree with this proposal. All the configuration stuff should be moved to apio.ini

— Reply to this email directly, view it on GitHub https://github.com/FPGAwars/apio/issues/357#issuecomment-1968237049, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQVMQNMTACIFVMIVM2JGBTYV23JJAVCNFSM6AAAAABD5JOAV2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRYGIZTOMBUHE . You are receiving this because you authored the thread.Message ID: @.***>

zapta commented 4 months ago

@Obijuan, I was looking in this context also at the init command.

  1. It has an option to have a user edited SConstruct file. Is it ok to remove that functionality (no apio init -s, and no processing of local SConstruct file)?

Rationale: It seems to bypass all of apio abstraction and expose to the user a very internal and unstable implementation detail. For example, I just made changes to the apio SConstruct, and I wouldn't expect users to be able to merge apio changes into their own SConstruct file.

  1. The apio init command seems to have two main use cases, i) create a new project and i) edit the apio.ini file. Is it ok to keep just the project creation functionality (e.g. with a create project command) and from that point having the users editing the apio.ini file manually?

Rationale:
i) The command doesn't preserve comments in apio.ini ii) since they users edit .v file, they have text editing skills iii) not having to change the command will simplify adding new apio.ini fields, iiii) same model works well with platformio.

zapta commented 4 months ago

@Obijuan, any thoughts?

TL;DR

  1. Keeping the api init project creation functionality.
  2. Removing the api init apio.ini mutation functionality (once a project is created, users will edit manually apio.ini).
  3. Removing the api init -s SConstruct functionality and removing the processing of user modified of SConstruct.

They are related to the apio config change. I will do them together.

Obijuan commented 4 months ago

I agree with the apio config change. But not for this release cycle. I am planing to freeze the current release, test it, and release as apio 0.9.0 (stable). Once it is release, you can continue with the apio config change I fully agree with points 1 and 2. The point 3 is not useful for the final user, but in my opinion is very useful for the developers, to test and fine tunning SConstruct

I will try to release it as soon as posible. In the meanwhile, you can implement it on another branch that can be merged into develop once the stable version is released

zapta commented 4 months ago

Sounds good. This will go in the next release.

BTW, regarding the development of SConstruct, I do all the editing in my git repository, and have a simlink that directs the pip apio package to my repository/apio. This works for all apio files, not just sconstruct.

On Fri, Mar 1, 2024 at 10:56 AM Juan Gonzalez-Gomez < @.***> wrote:

I agree with the apio config change. But not for this release cycle. I am planing to freeze the current release, test it, and release as apio 0.9.0 (stable). Once it is release, you can continue with the apio config change I fully agree with points 1 and 2. The point 3 is not useful for the final user, but in my opinion is very useful for the developers, to test and fine tunning SConstruct

I will try to release it as soon as posible. In the meanwhile, you can implement it on another branch that can be merged into develop once the stable version is released

— Reply to this email directly, view it on GitHub https://github.com/FPGAwars/apio/issues/357#issuecomment-1973752103, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQVMQI6VT6Z3OVP5PKYJHDYWDFOFAVCNFSM6AAAAABD5JOAV2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZTG42TEMJQGM . You are receiving this because you authored the thread.Message ID: @.***>

zapta commented 2 months ago

Hi @Obijuan, I am keep working on transition of apio.ini to be more centric to the project, similar to platformio.ini, and wanted to check with you before I make big changes.

The general approach is:

  1. apio.ini becomes required.
  2. apio.ini contains all the configuration of the project.
  3. Project variants are provided in a persisted and shareable way rather than in transient command line args. (platformio.ini for example, supports multiple envs in a single platformio.ini file, al local board files that allows users to override the stock board definitions).

With this approach, the apio build flags such as --board, --fpga, --size, --type, and --pack, can go away since all the configuration will be derived from apio.ini.

Does this make sense to you or is it required to keep the command line based configuration?

$ apio build -h
Usage: apio build [OPTIONS]

  Synthesize the bitstream.

Options:
  -b, --board str        Set the board.
  --fpga str             Set the FPGA.
  --size str             Set the FPGA type (1k/8k).
  --type str             Set the FPGA type (hx/lp).
  --pack str             Set the FPGA package.
  -p, --project-dir str  Set the target directory for the project.
  -v, --verbose          Show the entire output of the command.
  --verbose-yosys        Show the yosys output of the command.
  --verbose-pnr          Show the pnr output of the command.
  --top-module str       Set the top level module (w/o .v ending) for build.
  -h, --help             Show this message and exit.
zapta commented 2 months ago

Hi @Obijuan, any thoughts on the questions above? (making apio.ini required and persisted and shareable source of truth, and deprecating overriding args such as --board --fpga --size). I have a change in progress but would like to check first.

Obijuan commented 2 months ago

Hi @zapta . Sorry for the late response. Using apio by means of arguments cannot be removed. It is used in other tools that calls apio, such Icestudio. They invoke apio directly passing all the information through arguments (no apio.ini is created) I agree that the arguments does not make much sense for the user. It is better to have them in apio.ini. But for Icestudio and other applications that uses apio as a backend, i think it is better to keep the arguments At least in the sort term we can not remove them

zapta commented 2 months ago

Thanks Juan. I will leave it as is then.

If you have influence on the icestudio project, you may want discuss with them having their own apio.ini file, even if created on the fly on each invocation.

BTW, I started with icestudio and then switched to apio. It clicked better with me.

Zapta.

On Mon, Apr 22, 2024 at 12:21 AM Juan Gonzalez-Gomez < @.***> wrote:

Hi @zapta https://github.com/zapta . Sorry for the late response. Using apio by means of arguments cannot be removed. It is used in other tools that calls apio, such Icestudio. They invoke apio directly passing all the information through arguments (no apio.ini is created) I agree that the arguments does not make much sense for the user. It is better to have them in apio.ini. But for Icestudio and other applications that uses apio as a backend, i think it is better to keep the arguments At least in the sort term we can not remove them

— Reply to this email directly, view it on GitHub https://github.com/FPGAwars/apio/issues/357#issuecomment-2068664617, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQVMQNHYGKKAOR7FOOKBCDY6S27ZAVCNFSM6AAAAABD5JOAV2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYGY3DINRRG4 . You are receiving this because you were mentioned.Message ID: @.***>

zapta commented 3 weeks ago

Proposal not accepted. Closing.

Obijuan commented 2 weeks ago

Hi @zapta

Proposal not accepted. Closing.

I like this proposal, so I will keep this issue opened. I has not accepted it for this release cycle but I think it is ok for the nexts

If you have influence on the icestudio project, you may want discuss with them having their own apio.ini file, even if created on the fly on each invocation Yes, I am one of the Icestudio developers. This tool, contrary to apio, has been designed for educational purposes. It is targeted for new comers to the digital world: It allows them to easily learn how to design real digital circuits (not simulations), and undestrand the principles without having to learn first HDL languages So, in the next release cycles I will implement the feature you mentioned: Let's Icestudio create the apio.ini file on the fly (It will take some time, tough, as it is not a high priority feature)

zapta commented 2 weeks ago

Very good. Thanks.

If it makes sense I can do the following:

  1. Modifying the unit tests to use apio.ini instead of flags.
  2. Adding a warning when flags are used that this is going to be deprecated.

On Thu, Jun 13, 2024 at 12:04 AM Juan Gonzalez-Gomez < @.***> wrote:

Hi @zapta https://github.com/zapta

Proposal not accepted. Closing.

I like this proposal, so I will keep this issue opened. I has not accepted it for this release cycle but I think it is ok for the nexts

If you have influence on the icestudio project, you may want discuss with them having their own apio.ini file, even if created on the fly on each invocation Yes, I am one of the Icestudio developers. This tool, contrary to apio, has been designed for educational purposes. It is targeted for new comers to the digital world: It allows them to easily learn how to design real digital circuits (not simulations), and undestrand the principles without having to learn first HDL languages So, in the next release cycles I will implement the feature you mentioned: Let's Icestudio create the apio.ini file on the fly (It will take some time, tough, as it is not a high priority feature)

— Reply to this email directly, view it on GitHub https://github.com/FPGAwars/apio/issues/357#issuecomment-2164730477, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQVMQPAWHA52KKSY7YAV23ZHE77BAVCNFSM6AAAAABD5JOAV2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRUG4ZTANBXG4 . You are receiving this because you were mentioned.Message ID: @.***>