craftr-build / craftr-build-4.x

Frontend for the Craftr build framework.
https://craftr-build.github.io/craftr/
Other
60 stars 14 forks source link

Move 'loaders' from manifest into Craftrfiles #141

Closed NiklasRosenstein closed 7 years ago

NiklasRosenstein commented 7 years ago

Currently the "loaders" field in the manifest is used to define URLs from which a library sources can be downloaded. However, I feel like it would be much more flexible to be used directly from the Craftrfile as it gives you more control and you can use multiple loader simultaneously.

Instead of

  "loaders": [
    {
      "name": "source",
      "type": "url",
      "urls": [
        "file://$directory",
        "http://some/url"
      ]
    }
  ]

we could have a simple Python API.

loader = UrlLoader(
  name = "source",
  urls = [
    "file://" + options.directory,
    "http://some/url"
  ]
)
loader.load()

Or something similar. It should be simple to use, but also powerful and easy to customize.

NiklasRosenstein commented 7 years ago

I just found that CMake has something similar called "ExternalData"

https://crascit.com/2015/04/03/handling-binary-assets-and-test-data-with-cmake/

winksaville commented 7 years ago

What is the advantage of the manifest.json, did you happen to write up a design rationale document for craftr2?

On Wed, Nov 30, 2016 at 6:05 PM Niklas Rosenstein notifications@github.com wrote:

I just found that CMake has something similar called "ExternalData"

https://crascit.com/2015/04/03/handling-binary-assets-and-test-data-with-cmake/

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/craftr-build/craftr/issues/141#issuecomment-264057369, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-hHE70OpURv01mE72QneNErB1yNb86ks5rDitYgaJpZM4LA69H .

NiklasRosenstein commented 7 years ago

There's no document describing the design rationale, it's all in my head 🙈 Craftr is an immature piece of software and a one-man-show, there's no one else giving directions except for the input that you and a few other have given yet. :)

Anyway, regarding the advatanges over the manifest; Few. Everything that's in the manifest could be parsed and used by other frontends, although I don't think there will be much than 1) the Craftr command-line interface 2) Craftr.net as package manager and eventually a CMake-like GUI anytime soon. I was overdoing it a little bit with putting stuff into the Manifest ;)

Just implemented an alternative (backwards incompatible and removed the "loaders" from the manifest completely, I know that such changes might not be possible in the future should Craftr get more attention ;) But Craftr 2 doesn't even have a stable release yet).

Will add something that supports pkg-config soon (#131)

from craftr.loaders import external_archive
source_directory = external_archive(
  "https://github.com/craftr-build/craftr/archive/{}.zip".format(options.version or "master")
)
NiklasRosenstein commented 7 years ago

Added doc/loaders.md

NiklasRosenstein commented 7 years ago

With 32fee30 and 65b0815 transition is complete and no example or standard library package uses the "loaders" field anymore.

winksaville commented 7 years ago

I guess my big question is why require a separate file at all (manifest.json) and in a different language to boot?

Separately, why worry about a package manager, seems to me getting a build system "right" is going to be hard enough.

If you like to discuss generalities it should probably be in another place:)

On Thu, Dec 1, 2016, 6:44 AM Niklas Rosenstein notifications@github.com wrote:

With 32fee30 https://github.com/craftr-build/craftr/commit/32fee3086c1ce2844dd869e104e1694995f9c39f transition is complete and no example or standard library package uses the "loaders" field anymore.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/craftr-build/craftr/issues/141#issuecomment-264190701, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-hHII_P6hnOJcvsT-xTZoQ85tFf_qnks5rDt1WgaJpZM4LA69H .

NiklasRosenstein commented 7 years ago

The manifest.json was added in regards to the package manger that will be built into Craftr. The package manager should be able to find dependencies without first running the build script. Especially the same should be the case for the package manager frontend (i.e. the website) where you should be able to view a package's dependencies.

It also allowes you to change the project directory (where glob() and local() derive their paths from) in case you want to. And defining options in the manifest will allow GUI tool to provide the user with a list of the available options.

The manifest also gets around the ugly way buildscripts where named in Craftr 1 (either <name>.craftr.py or a line # craftr_module(<name>) in the first lines of the file) and it introduced a version field which will also be important for dependency resolving.

And finally there's some metadata like the package author, URL to the main project page. These will be extended a bit more for the package manager so you can add a short description and keywords to search for with the package manager.

Hope that gives you an idea why I deemed an additional file necessary :)

winksaville commented 7 years ago

Is there really a need to use a third language, there is already ninja and python. What about having the manifest be defined using python instead of json then it should be easy to include it in a Craftrfile if there no package manager is being used or it could be separate if there is one.

On Thu, Dec 1, 2016 at 11:06 AM Niklas Rosenstein notifications@github.com wrote:

The manifest.json was added in regards to the package manger that will be built into Craftr. The package manager should be able to find dependencies without first running the build script. Especially the same should be the case for the package manager frontend (i.e. the website) where you should be able to view a package's dependencies.

It also allowes you to change the project directory (where glob() and local() derive their paths from) in case you want to. And defining options in the manifest will allow GUI tool to provide the user with a list of the available options.

The manifest also gets around the ugly way buildscripts where named in Craftr 1 (either .craftr.py or a line # craftr_module() in the first lines of the file) and it introduced a version field which will also be important for dependency resolving.

And finally there's some metadata like the package author, URL to the main project page. These will be extended a bit more for the package manager so you can add a short description and keywords to search for with the package manager.

Hope that gives you an idea why I deemed an additional file necessary :)

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/craftr-build/craftr/issues/141#issuecomment-264263240, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-hHNqS79woMIVvKGtyKAGQA4MwMoN6ks5rDxq_gaJpZM4LA69H .

NiklasRosenstein commented 7 years ago

I don't feel like Python is the right way to express merely data in the form that is contained in the manifest. And it would still need to be a separate file.

Also when you're using Craftr, you should not need to fuzz with the Ninja manifest so it's only Python and JSON.

I understand that the manifest is an obstacle when using Craftr for very simple projects, thus I am thinking about making the manifest optional. However that would only work for the main project. That project could not be loaded by other projects and not be submitted into the online repository until it gets a manifest.

winksaville commented 7 years ago

Actually, what are the main features of the package manager?

What if craftr is being used for non-public code and a public repo isn't possible?

On Thu, Dec 1, 2016, 6:33 PM Niklas Rosenstein notifications@github.com wrote:

I don't feel like Python is the right way to express merely data in the form that is contained in the manifest. And it would still need to be a separate file.

Also when you're using Craftr, you should not need to fuzz with the Ninja manifest so it's only Python and JSON.

I understand that the manifest is an obstacle when using Craftr for very simple projects, thus I am thinking about making the manifest optional. However that would only work for the main project. That project could not be loaded by other projects and not be submitted into the online repository until it gets a manifest.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/craftr-build/craftr/issues/141#issuecomment-264357677, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-hHEleBizWA8IuxM1cgvB8Kh4Jy3_zks5rD4NdgaJpZM4LA69H .

NiklasRosenstein commented 6 years ago

What if craftr is being used for non-public code and a public repo isn't possible?

That will not be an issue as the package manager will be designed with that possibility in mind. Even for proprietary projects, dependencies from public sources will still be accessible. Dependencies from non-public sources will need to be configured so that the package manager can access them.

Any internal dependencies can also just be embedded into the project using Git submodules.

By the way, in Craftr 3, a manifest.json file is no longer required. However, for projects that are supposed to be used as dependencies in other projects (eg. build scripts for ZLib [which may just use pkg-config on Linux but compile from source on Windows]) should expose a nodepy.json manifest as they are treated as packages for the Node.py package manager.

There is no Node.py package registry, yet, but you can already install Craftr 3 with Node.py and theoretically and other packages which may just happen to be Craftr modules.