Skip to content

[c_compiler] package name #85

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

Closed
dcharkes opened this issue Jul 11, 2023 · 5 comments · Fixed by #87
Closed

[c_compiler] package name #85

dcharkes opened this issue Jul 11, 2023 · 5 comments · Fixed by #87
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_toolchain_c

Comments

@dcharkes
Copy link
Collaborator

"Package name c_compiler is too similar to another active package: ccompilers (https://pub.dev/packages/ccompilers)."

We can't use package:c_compiler as a package name.

ccompilers is an outdated package which hasn't been touched for 4 years.

The reason we went for c_compiler is that we could have a rust_compiler package etc.

Suggestion by @jonasfj: native_toolchain_c.

The upside of that name is it has 'native' which makes it fit with the other native packages in this repo.
The downside is that the package name is considerably longer.

Blocks:

@devoncarew @mit-mit @HosseinYousefi any opinions?

@dcharkes dcharkes added package:native_toolchain_c P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jul 11, 2023
@dcharkes
Copy link
Collaborator Author

@mezoni The package:ccompilers used to be on https://github.com/mezoni/ccompilers according to the last published version, but that repository has been deleted or made private. Do you have any intention of maintaining this package? If no, would you consider discontinuing it?

@dcharkes
Copy link
Collaborator Author

dcharkes commented Jul 11, 2023

For reference: Rust calls it's C compiler wrapper package cc https://docs.rs/cc/latest/cc/.

Some more notes dug up somewhere from @devoncarew and me:

Maybe this should be c_compiler, so that other packages can be named rust_compiler, cplusplus_compiler etc.

Or maybe c_builder, because it builds C code using a c compiler, but is not a c compiler itself. (And then rust_builder)

Or, maybe c_assets_builder, to connect the name with "native_assets".

Perhaps this could be the abstract impl. of a compiler - w/ utilities to parse the CLI standard, some tool locating utilities, and process management. We could ship a separate package which was the clang impl. of the above; so:

  • package:cc is the abstraction - the compiler agnostic package
  • package:cc_clang is one implementation of the above, based on clang
    Generally, I like having a unifying prefix for these packages; the base package, and the other compiler packages that will exists, either that we ourselves ship (clang) or that the community will ship (rust, ...). That could be 'cc_' or some other prefix.

package:native_toolchain_XXX does create a unifying prefix.

And notes from @jonasfj:

[...] what this package is supposed to do, is to:

  • Find a C compiler on the system
  • Detect compiler (gcc, msvc, clang, etc)
  • Detect compiler version
  • Detect operating system
  • Wrap the compiler in a Dart interface

then native_toolchain_c is not a bad name

@dcharkes
Copy link
Collaborator Author

@devoncarew I can't make new labels in this package or rename the existing label package:c_compiler label.

@devoncarew
Copy link
Member

devoncarew commented Jul 11, 2023

@devoncarew I can't make new labels in this package or rename the existing label package:c_compiler label.

You should have more than sufficient permissions to (a repo committer can modify labels).

@dcharkes
Copy link
Collaborator Author

@devoncarew I can't make new labels in this package or rename the existing label package:c_compiler label.

You should have more than sufficient permissions to (an repo committer can modify labels).

Ah, it's not in the "settings" part. Done.

HosseinYousefi pushed a commit that referenced this issue Nov 16, 2023
…246)

* Add pubspec.lock from JNI example to git. 
This file should be checked in.

* Remove ffi patch and instead depend on the internals of a pinned package:ffi version (closes #149)
* Add new tests on GlobalJniEnv
* Rename findJniClass -> findJClass
* Locking around method/field lookups. (closes #85)
* Rename asJString and asDartString to toJStringPtr and toDartString for uniform naming
* Rename JniClass -> JClass
* Compare bindings normally and fallback to ignoring spaces
HosseinYousefi pushed a commit that referenced this issue Nov 16, 2023
…246)

* Add pubspec.lock from JNI example to git. 
This file should be checked in.

* Remove ffi patch and instead depend on the internals of a pinned package:ffi version (closes #149)
* Add new tests on GlobalJniEnv
* Rename findJniClass -> findJClass
* Locking around method/field lookups. (closes #85)
* Rename asJString and asDartString to toJStringPtr and toDartString for uniform naming
* Rename JniClass -> JClass
* Compare bindings normally and fallback to ignoring spaces
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_toolchain_c
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants