Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ghdl: add split package #6688

Merged
merged 4 commits into from
Nov 20, 2020
Merged

ghdl: add split package #6688

merged 4 commits into from
Nov 20, 2020

Conversation

eine
Copy link
Contributor

@eine eine commented Jul 16, 2020

Co-authored-by: Tim Stahlhut [email protected]

This PR contributes an split package for GHDL: mcode on MINGW32, and LLVM on MINGW64. See #5757.

Close #6686

@elieux
Copy link
Member

elieux commented Jul 20, 2020

Eep. Why is it different between mingw32 and mingw64?

@elieux
Copy link
Member

elieux commented Jul 20, 2020

Is there a release archive for the source rather than a commit?

@eine
Copy link
Contributor Author

eine commented Jul 20, 2020

Eep. Why is it different between mingw32 and mingw64?

@elieux, see first message in #5757. The summary is that GHDL is a particular tool. It is a VHDL simulator and synthesizer, which is shaped after a "compiler". As such, it supports three different backends: mcode, LLVM and GCC. mcode is a built-in in-memory backend that works on x86 only (32/64bits on GNU/Linux, and 32 bits only on Windows). LLVM and GCC should work on any platform. So far, I was able to build:

  • MINGW32 mcode
  • MINGW64 llvm

Should work, but I could not build:

  • MINGW32 llvm
  • MINGW32 gcc
  • MINGW64 gcc

Might work in the future:

  • MINGW64 mcode

Hence, my first proposal was to contribute ghdl-mcode for MINGW32 only and ghdl-llvm for MINGW64 only. However, @mati865 told me that packages which cannot be built for both of them are normally not accepted: #5757 (comment). Then @stahta01 proposed and helped with writing an split package (#5757 (comment)), which I reworked to propose this PR.

Note that MINGW32 mcode and MINGW64 llvm are part of GHDL's CI workflow, and have been intensively tested for months: https://github.com/ghdl/ghdl/blob/master/.github/workflows/push.yml#L62-L65.

Is there a release archive for the source rather than a commit?

All GitHub repos can be retrieved as an archive, instead of using git. For example: https://github.com/ghdl/ghdl/archive/e2d751d6.zip or https://github.com/ghdl/ghdl/archive/e2d751d6.tar.gz. However, I think that those might be rate-limited. Alternatively, I think that https://codeload.github.com/ghdl/ghdl/tar.gz/e2d751d6 is not rate limited.

Nevertheless, if you mean some tagged/labeled release, instead of an "arbitrary" commit SHA, I'd argue in favour of not doing so. See ghdl/ghdl#1213 (comment). The summary is that GHDL is tagged once a year, but those tagged commits are not significantly different from any other. GHDL is mostly a single man show and it has a rolling development model: each issue/bug is solved with a matching test. For example, at the moment the latest stable release would need to be patched to work with LLVM 10. But that is already fixed in the selected commit SHA for this PR. Note that this same approach is used by other packagers at Fedora or Arch.

@elieux
Copy link
Member

elieux commented Jul 20, 2020

Thanks for the detailed response.

Should work, but I could not build:

  • MINGW32 llvm
  • MINGW32 gcc
  • MINGW64 gcc

Let's hope they work in the future.

However, @mati865 told me that packages which cannot be built for both of them are normally not accepted

Well, having a single recipe that essentially does a different package for each architecture isn't so much different IMO. But if you set a common provides (see link below) on both, it would be bearable.

Note that this same approach is used by other packagers at Fedora or Arch.

https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/ghdl says otherwise; am I missing something?

We used to have more *-git packages in the repo, but we decided to go the release route to lessen the workload. The issue is twofold, as you noticed:

  1. downloading using Git or HTTPS: It is somewhat bothersome to deal with makepkg+git. A GitHub archive would be a good workaround. Also we'd have to call the package mingw-w64-*-ghdl-git by convention.
  2. versioning: It's not great having to invent version numbers.

I see there are 841 patches since v0.37. Are there many important patches in there? If there are, can we make up a good versioning scheme or something?

@eine
Copy link
Contributor Author

eine commented Jul 20, 2020

Let's hope they work in the future.

MINGW32 llvm might be straightforward: #5757 (comment)

I wanted to try fixing that after having an initial package released.

Well, having a single recipe that essentially does a different package for each architecture isn't so much different IMO.

Agree. However, having a single recipe allows to pass CI tests, while separated packages do not.

But if you set a common provides (see link below) on both, it would be bearable.

I would like to allow installing multiple versions in the future. mcode and LLVM have different advantages, the former is faster for analysis and the latter for execution. Hence, it makes sense to use mcode for e.g. language server features or synthesis, and use LLVM for simulation.

Now there would be no problem with a common provides (and name). However, when MINGW32 LLVM is added, there would be two variants. I am thinking that the current split package does not account for it. Shall I add a common provides now and think about that later? Or is it better to add separate provides (ghdl-mcode and ghdl-llvm) already?

Note that this same approach is used by other packagers at Fedora or Arch.

git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/ghdl says otherwise; am I missing something?

I believe that is the latest package that was aded to "official" repos, and the packager was forced to use the latest stable release due to Arch's policies. That motivated several discussions similar to the one we are having here: https://github.com/ghdl/ghdl/issues?q=author%3AFFY00+

However, as seen in ghdl/ghdl#507, packages in AUR are different. The packages maintained by @marzoul are "arbitrary" commits: https://aur.archlinux.org/packages/?O=0&SeB=nd&K=ghdl&outdated=&SB=n&SO=a&PP=50&do_Search=Go

In Fedora: see ghdl/ghdl#888 and https://src.fedoraproject.org/rpms/ghdl/blob/master/f/ghdl.spec. Note that the version used in Fedora is 0.38~dev, although the "official" git describe (and the one used in AUR) is 1.0dev. An "arbitrary" commit is used too.

We used to have more *-git packages in the repo, but we decided to go the release route to lessen the workload.

I understand that, and I want to put it clear: I am asking for an exception. As such, I am doing my best to justify why such exception should be acceptable in the case of GHDL.

  1. downloading using Git or HTTPS: It is somewhat bothersome to deal with makepkg+git. A GitHub archive would be a good workaround. Also we'd have to call the package mingw-w64-*-ghdl-git by convention.

I think this is negligible. I will update it to use the archive instead, but I agree that the critical point is versioning.

Regarding mingw-w64-*-git, this is not exactly *-git, if that means getting master. I am proposing to get "unofficial release candidates" (see below). If *-git means "retrieved from a git repo", then it's ok.

  1. versioning: It's not great having to invent version numbers.

I see there are 841 patches since v0.37.

The point is that GHDL has been in development for 15-20 years. But for most of it, the target was simulation only. 1-2 years ago, the author decided to implement synthesis features and to plug it to yosys to allow open source synthesis of VHDL projects. Together with nextpnr and other projects (icestorm, prjtrellis, prjxray), it is possible for first time in 30+ years to configure FPGAs using open source tooling only. A few weeks ago, Google announced https://github.com/google/skywater-pdk, which allows all the way down to silicon.

The activity has been growing and Tristan (the author) has been doing an astonishing effort: https://github.com/ghdl/ghdl/graphs/contributors Hence, yes, potentially at least 50% of those 841 commits are patches. Specially since microwatt was synthetized in January, a lot of effort has gone into fixing memory inference. At the same time, many vendor tools are lacking VHDL 2008 support. GHDL can be effectively used as a pre-processor to convert modern VHDL code to an older version, so that vendor tools accept it. This allows to use modern language features with devices not yet supported by the fully open source toolchain. Again, each of the commits related to VHDL 2008 features is likely to be a bugfix or fix for a missing (corner) case.

Are there many important patches in there? If there are, can we make up a good versioning scheme or something?

Potentially, we could release a new version every few days or every week. Tristan (the author) is so responsive, he fixes issues in less than 24h: https://www.youtube.com/watch?v=LiCQzQsLFqA&feature=youtu.be&t=196 Hence, I agree with him that following semantic versioning strictly and tagging every new/breaking change can be a lot of unproductive work. The only viable solution I see is to somehow automate it by calendar/schedule. For example: every first monday of each month pick the commit SHA of the latest succesful CI run.

That's why I would like to coordinate with other packagers to decide some intermediate "unofficial release candidates", meaning that @marzoul, @sharkcz and me agree on using the same set of commit SHAs between releases of GHDL. However, I am not a maintainer of any package yet, so I don't see myself in a position to "demand" that to them. See other related efforts: ghdl/ghdl#901 and eine/ghdl-packaging.

@elieux
Copy link
Member

elieux commented Jul 21, 2020

You argue well. Unless anyone else voices a complaint, I'm convinced to merge once we pin down the technicalities:

Now there would be no problem with a common provides (and name). However, when MINGW32 LLVM is added, there would be two variants. I am thinking that the current split package does not account for it. Shall I add a common provides now and think about that later? Or is it better to add separate provides (ghdl-mcode and ghdl-llvm) already?

As you can see in official Arch Linux recipe and AUR recipes, the actual package names include the backend and on top of that they all provide ghdl. This allows the user to just pacman -S ghdl and have options presented to them, or choose the desired backend right away. They also set conflicts, so the packages currently are exclusive, but that's not directly related to provides.

I think we should build differently named packages (mingw-w64-i686-ghdl-mcode and mingw-w64-x86_64-ghdl-llvm) right away and have a common provides=("${MINGW_PACKAGE_PREFIX}-ghdl"). I'd postpone adding conflicts for when mingw-w64-i686-ghdl-llvm is available.

Regarding mingw-w64-*-git, this is not exactly *-git, if that means getting master. I am proposing to get "unofficial release candidates" (see below). If *-git means "retrieved from a git repo", then it's ok.

Good point. Let's not add -git.

I will update it to use the archive instead

And we'll have to remove pkgver() and specify the versions manually. If really needed, $pkgver can be computed from an expression, but I assume there's no data to compute from at build-time anyway.

I understand this might be a hassle, but the general spirit should IMHO be that this is a badly versioned package we have to work around rather than a Git package with special privileges.


The only viable solution I see is to somehow automate it by calendar/schedule. For example: every first monday of each month pick the commit SHA of the latest succesful CI run.

... and tag that commit. :)

@eine
Copy link
Contributor Author

eine commented Jul 22, 2020

As you can see in official Arch Linux recipe and AUR recipes, the actual package names include the backend and on top of that they all provide ghdl. This allows the user to just pacman -S ghdl and have options presented to them, or choose the desired backend right away. They also set conflicts, so the packages currently are exclusive, but that's not directly related to provides.

If I don't get it wrong, you mean that provides is only used when asking pacman which package installs some tool. Hence, all the packages should provide ${MINGW_PACKAGE_PREFIX}-ghdl only? Or should they provide ${MINGW_PACKAGE_PREFIX}-ghdl and ${MINGW_PACKAGE_PREFIX}-ghdl-backend?

I think we should build differently named packages (mingw-w64-i686-ghdl-mcode and mingw-w64-x86_64-ghdl-llvm) right away and have a common provides=("${MINGW_PACKAGE_PREFIX}-ghdl"). I'd postpone adding conflicts for when mingw-w64-i686-ghdl-llvm is available.

With "differently named packages" you mean two different PKGBUILD files? Or is it possible to generate mingw-w64-i686-ghdl-mcode and mingw-w64-x86_64-ghdl-llvm with a single PKGBUILD file? The issue with having two files is that the current CI infrastructure will fail. arch=('any') should checked, which I think is ignored now.

And we'll have to remove pkgver() and specify the versions manually. If really needed, $pkgver can be computed from an expression, but I assume there's no data to compute from at build-time anyway.

Indeed, if a tarball is used, there is no possibility to use git.

I understand this might be a hassle, but the general spirit should IMHO be that this is a badly versioned package we have to work around rather than a Git package with special privileges.

Agree.

The only viable solution I see is to somehow automate it by calendar/schedule. For example: every first monday of each month pick the commit SHA of the latest succesful CI run.

... and tag that commit. :)

I need to discuss that. For now, I was thinking about taking SHAs with some defined logic/criteria.

@elieux
Copy link
Member

elieux commented Jul 23, 2020

With "differently named packages" you mean two different PKGBUILD files? Or is it possible to generate mingw-w64-i686-ghdl-mcode and mingw-w64-x86_64-ghdl-llvm with a single PKGBUILD file? The issue with having two files is that the current CI infrastructure will fail. arch=('any') should checked, which I think is ignored now.

Right, I didn't think this through completely at first. I mean one file like this:

# Test recipe

_realname=ghdl
pkgbase="${MINGW_PACKAGE_PREFIX}-${_realname}"

pkgname=()
if [ "${CARCH}" = "x86_64" ]; then
	pkgname+="${pkgbase}-llvm"
fi
if [ "${CARCH}" = "i686" ]; then
	pkgname+="${pkgbase}-mcode"
fi

pkgver=1
pkgrel=1
arch=('any')
provides=("${pkgbase}")

package_mingw-w64-i686-ghdl-mcode() {
	touch "${pkgdir}/mcode"
}

package_mingw-w64-x86_64-ghdl-llvm() {
	touch "${pkgdir}/llvm"
}

@elieux
Copy link
Member

elieux commented Jul 23, 2020

If I don't get it wrong, you mean that provides is only used when asking pacman which package installs some tool. Hence, all the packages should provide ${MINGW_PACKAGE_PREFIX}-ghdl only? Or should they provide ${MINGW_PACKAGE_PREFIX}-ghdl and ${MINGW_PACKAGE_PREFIX}-ghdl-backend?

It's used in general to resolve additional names to a package. It could be more packages providing the same tool (e.g. any *-git package also provides the non-git package name so dependees are satisfied), or a package swallowing up another one etc. In our case, the actual package names will already be ${MINGW_PACKAGE_PREFIX}-ghdl-<backend>, so provides only needs to add ${MINGW_PACKAGE_PREFIX}-ghdl.

@eine eine marked this pull request as draft July 23, 2020 16:24
@eine
Copy link
Contributor Author

eine commented Jul 23, 2020

@elieux, I updated the PR according to your last comments. I was unsure about the following content:

package_mingw-w64-i686-ghdl-mcode() {
	touch "${pkgdir}/mcode"
}

package_mingw-w64-x86_64-ghdl-llvm() {
	touch "${pkgdir}/llvm"
}

I assume those touch commands were placeholders only.

Apart from that, I reverted this PR to a draft, because I used pkgver=1.0.0-rc.1. That is something I am talking with the author of GHDL, but which is not decided yet.

@elieux
Copy link
Member

elieux commented Jul 26, 2020

I assume those touch commands were placeholders only.

Correct.

@elieux
Copy link
Member

elieux commented Aug 2, 2020

If I change line 15 to pkgname=(__placeholder__), saneman won't complain:

/w/dev/mingw-packages/mingw-w64-ghdl$ saneman --verbose --no-terminal
success: mingw-w64-ghdl: ok

@eine
Copy link
Contributor Author

eine commented Nov 1, 2020

I wanted to have proper snapshot tags working upstream before having this merged, but it seems it's going to take longer than expected. Since this package is already working regardless, I'd like to have it merged. @elieux, should it be converted into a -git package and moved to https://github.com/msys2/MINGW-packages-dev?

@eine eine marked this pull request as ready for review November 1, 2020 13:08
eine and others added 4 commits November 20, 2020 17:26
Co-authored-by: Tim Stahlhut <[email protected]>
* generate a proper version
* restructure package() so makepkg --printsrcinfo can parse it
* use a shared pkgbase for both packages
@lazka
Copy link
Member

lazka commented Nov 20, 2020

@eine OK with my changes?

@eine
Copy link
Contributor Author

eine commented Nov 20, 2020

@lazka, LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants