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

zpp_bits: Add new wrap 4.4.22 #1574

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

QWERTIOX
Copy link
Contributor

@QWERTIOX QWERTIOX commented Jul 4, 2024

I'm not sure if meson_version: '>=1.0.1' should be so high but this is the one i'm using right now

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

of course sanity checks pass on local machine...

@benoit-pierre
Copy link
Contributor

You forgot to update releases.json.

As for the meson_version: '>=1.0.1' requirement, there's no reason for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file must be removed.

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

wait, i include whole directory and not just .h file

@benoit-pierre
Copy link
Contributor

Why?

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

because it was including junk like readme.md and I could't find another way to move this header file or force including just header file

@benoit-pierre
Copy link
Contributor

If by included, you mean add the directory to the include flags, yes… So? You're afraid someone is going to use #include <README.md>?

And you could have just used fs.copyfile to copy the include to an include sub-directory at setup.

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

i tried and it didn't see the include directory

@benoit-pierre
Copy link
Contributor

Please post the code you used.

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

project(
  'zpp_bits',
  'cpp',
  version: '4.4.22',
  license: 'MIT',
)
fs = import('fs')
copy = fs.copyfile('zpp_bits.h', 'include')

incdir = include_directories('include')

zpp_bits_dep = declare_dependency(include_directories: include_directories())

@benoit-pierre
Copy link
Contributor

benoit-pierre commented Jul 4, 2024

That's not how fs.copyfile work:

dest str: the name of the output file. If unset will be the basename of the src argument

Try something like this:

  • meson.build
project(
  'zpp_bits',
  'cpp',
  version: '4.4.22',
  license: 'MIT',
  meson_version: '>=0.64.0',
)

subdir('include')

zpp_bits_dep = declare_dependency(include_directories: incdir)
  • include/meson.build:
copy = import('fs').copyfile('../zpp_bits.h')

incdir = include_directories('.')

@benoit-pierre
Copy link
Contributor

(And go back to the original upstream source distribution).

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

i think meson.build will still be included this way

@benoit-pierre
Copy link
Contributor

What do you mean included? declare_dependency is not creating an archive.

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

i mean it's in the same directory as header

@benoit-pierre
Copy link
Contributor

IHMO, you could have kept the original code. The real use case for fs.copyfile with respect to headers is to avoid making private headers accessible when they are mixed with public ones, particularly things like headers with generic names, like config.h.

i mean it's in the same directory as header

So? You'll have multiple instances of that in the wraps provided (with makefiles, source files…). Unless someone is foolish enough to try to include<meson.build>, what is the harm exactly?

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

maybe i'm perfectionist but i don't like that my tools reports that meson.build is one of the headers ready to be included

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

but as you mentioned, other wraps do that so now i don't care

@QWERTIOX
Copy link
Contributor Author

QWERTIOX commented Jul 4, 2024

of course i'm stupid - i forget to set c++ at least to c++20

@@ -3686,4 +3694,4 @@
"1.3.3-1"
]
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhhh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why it did that, maybe because it's second to last so maybe it's some problem with trailing comma that makes diff freak out

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you removed the terminating newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was that bad?

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.

None yet

3 participants