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

add tests #16

Closed
wants to merge 5 commits into from
Closed

add tests #16

wants to merge 5 commits into from

Conversation

h-vetinari
Copy link
Member

Wanted to fix #15, found I had some uncommitted tests lying around. Let's try these first.

@h-vetinari h-vetinari requested a review from isuruf as a code owner April 1, 2022 02:18
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member Author

@isuruf
I managed to reproduce #15 with the tests here (c.f. 4d6ac64), but my first guess (making the paths explicit) didn't work out.

There's however some confusion (at least to me) with the various environments here - this feedstock is really only designed to be pulled in at build: time, but now I'm trying to shoehorn it into running at test: time. Perhaps you'd prefer a downstream: test-feedstock like for libcxx (sounds a bit overkill though for an internal tool)...?

@h-vetinari
Copy link
Member Author

@isuruf
Would you be able to have a look here? Would be good if we could unbreak this piece of pretty important infrastructure.

@isuruf
Copy link
Member

isuruf commented Apr 2, 2022

As the package name says, this is for building an autotools package and the test you are adding is not for an autotools package.

@h-vetinari
Copy link
Member Author

As the package name says, this is for building an autotools package and the test you are adding is not for an autotools package.

It's intended to be a minimal reproducer of #15, using the documented calls from the documentation in meta.yaml.

I don't really understand your point - I'm happy to adapt the tests however necessary (or indeed remove them) - but currently patch_libtool is not working

@isuruf
Copy link
Member

isuruf commented Apr 2, 2022

I don't think you understand what a autotools project is. See https://www.gnu.org/software/automake/manual/html_node/Autotools-Introduction.html

@h-vetinari
Copy link
Member Author

I do get the configure + make workflow, though - admittedly - not how it interacts with patch_libtool here. Are you saying that it's not an autotools projects without a call to configure before (note: the failure is the same regardless)?

The point is that the documentation of how to use this feedstock does not work even with the full monty of an autotools build, c.f.

# Not sure why this isn't working.
# [[ "$target_platform" == "win-64" ]] && patch_libtool

in conda-forge/ffmpeg-feedstock#126 (hmaarrfk/ffmpeg-feedstock@73461b1).

I don't know how important patch_libtool is in the grand scheme of things, but even though I don't understand all the intricacies, I'm trying to fix it.

@h-vetinari
Copy link
Member Author

but even though I don't understand all the intricacies, I'm trying to fix it.

It could be that the fix is documentation-only, but since it worked before, it seemed like there's more to it.

@isuruf
Copy link
Member

isuruf commented Apr 2, 2022

Are you saying that it's not an autotools projects without a call to configure before

Yes

The point is that the documentation of how to use this feedstock does not work even with the full monty of an autotools build, c.f.

No, ffmpeg is not an autotools project.

Please try a project using this package like mpfr and play around with it to get a feel of how it works.

@isuruf isuruf mentioned this pull request Apr 2, 2022
@h-vetinari
Copy link
Member Author

OK, closing this. I'm thinking the log message for calling patch_libtool only after configure could be made an error in the function itself, but otherwise things should be fine.

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.

can't read libtool
3 participants