bundle: remove deprecated functions

Bug #1493031 reported by Daniel Kochmański
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
ASDF
Confirmed
Undecided
Unassigned

Bug Description

This patch removes deprecation comment (functionality provided by both functions isn't provided by other interfaces and it's used by various users. It was lately fixed).

Tags: cosmetic patch
Revision history for this message
Daniel Kochmański (jackdaniel) wrote :
Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

I don't feel that these functions can be "undprecated" until they at least have docstrings that indicate their purpose and the meaning of their arguments.

DELIVER-ASD-OP is particularly problematic. It writes an ASD file "...for delivering the system as a single fasl" What's the role of this ASD file? Is it a convenience? Is it part of the bundling process? I don't *believe* so, since I don't see any IN-ORDER-TO for this operation. What is the caller expected to do with this ASD file?

Until these functions are crisply defined, they don't deserve to be part of ASDF's API.

BUNDLE-SYSTEM seems to be particularly poorly named, since it looks like it simply produces a .ASD file, which doesn't seem to be what BUNDLING ought to be (compare with LOAD-SYSTEM, TEST-SYSTEM, and COMPILE-SYSTEM). Unless someone can explain what this does in a way that makes this a reasonable name, it will be deprecated and removed.

Note that there's no sense in which this function provides a feature that "isn't provided by other interfaces" since it's a very thin wrapper around OPERATE.

Revision history for this message
Daniel Kochmański (jackdaniel) wrote :

generally these things are documented in official ECL documentation:

https://common-lisp.net/project/ecl/manual/re55.html - asdf:make-build
https://common-lisp.net/project/ecl/manual/re56.html - asdf:load-bundle-op

I don't know well ASDF innards, but make-build is the only function in ASDF which allows adding linker flags and C code. load-bundle-op on the other hand creates set of object files (one fasl per module)

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

Thanks! I will look those over. But note that the function you tried to undeprecate was BUNDLE-SYSTEM, not LOAD-BUNDLE-OP, and BUNDLE-SYSTEM does deliver-asd-op

Revision history for this message
Faré (fahree) wrote :

These functions are leftovers from asdf-ecl.lisp indeed, and ECL documentation is probably the place to go looking for proper documentation.

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

I'm looking at MAKE-BUILD and BUNDLE-SYSTEM again. I'd like to kill BUNDLE-SYSTEM. It's name is, at best, misleading, and it provides essentially no value because it's a trivial wrapper around OPERATE.

MAKE-BUILD seems more useful, based on the documentation (thanks, Daniel!), but I wonder if it's generally useful, or primarily intended for ECL. For example, would LD-FLAGS make sense in another CL context?

I wonder if this should just live in an ECL extensions file for ASDF? I'd prefer that to trying to figure out how to make it work on other Lisp implementations. Or we could just #+ecl it....

Revision history for this message
Daniel Kochmański (jackdaniel) wrote :

I think LD-FLAGS are specific to Lisp->C compilers like ECL, GCL, MKCL, eventually XCL (not sure about the last one), so it might be conditionally defined. I have no problem with #+ecl 'ing it, but I'd ask MKCL and (if it's still supported) GCL maintainers if they use this function.

I removed two "deprecation" comments taking bundle-system as being load-bundle-op. If it's something else and load-bundle-op isn't deprecated nor it will affect it's working, then I have nothing against removing that interface (bundle-system, not load-bundle-op).

Revision history for this message
Faré (fahree) wrote :

1- I'm all for deprecating these functions, and having ECL users import them from some ECL-specific extension, if they need them. But can we make that with the deprecation interface we're working on for 3.2? I don't think that doing anything rash before the upcoming 3.1.6 is a good idea.

2- As for implementations, the GCL maintainer doesn't seem very interested in making ASDF work well with GCL or in providing ASDF with GCL, and hasn't replied to email on this topic for months or years. As for XCL, it's an experiment that isn't maintained anymore, and LD-FLAGS doesn't apply to it at this time. So really, the implementations that matter here are ECL, MKCL, and CLASP — though I admit I haven't tried CLASP, and don't know how it behaves in this respect.

Revision history for this message
Faré (fahree) wrote :

I updated the DEPRECATION notice in 6b678b3 (3.1.7.14).

BUNDLE-SYSTEM is in bad taste, has a confusing name inconsistent with our conventions, fails to provide an :output-directory argument, etc. It better die, and be replaced by a deliver-asd-system function in some contrib.

MAKE-BUILD is downright evil. It relies on initargs to an operation, which violates a fundamental invariant of operation.lisp, action.lisp and more, according to which an operation is uniquely identified by its class, when it comes to caching parts of the build. Any options such as epilogue-code, if defined, must be in the COMPONENT, not the OPERATION.

These functions are leftover from asdf-ecl from back in the dark ASDF1 days. Kill them with fire. But do it the right way, as per our obsolete-function-warnings branch.

summary: - bundle: remove deprecation comment
+ bundle: remove deprecated functions
Revision history for this message
Faré (fahree) wrote :

Re-titled the bug from "bundle: remove deprecation comment" to "bundle: remove deprecated functions".

Faré (fahree)
Changed in asdf:
status: New → Confirmed
Revision history for this message
Daniel Kochmański (jackdaniel) wrote :

This issue is not about removing deprecated functions, but deprecation comments, and the whole discussion revolves around it. Please create a separate issue instead of hijacking this one and providing misleading name.

Revision history for this message
Faré (fahree) wrote :

The deprecation comments are correct, and were updated. These functions are deprecated for a good reason.

Hopefully, the comments can be replaced or complemented by code that ensures these functions progressively raise style-warnings, warnings, errors, compile-time errors, when used in later versions of ASDF, until they are simply removed.

Revision history for this message
Daniel Kochmański (jackdaniel) wrote :

You've mentioned, that they are deprecated because they are broken and will fail under certain conditions (something with initargs etc etc). Please provide a reproducible test case before you deprecate these functions, so people relying on it doesn't have to take your word for that and eventually can think, if this brokeness may be fixed while preserving the API.

Otherwise deprecating them is an arbitrary choice without agreement with this particular API clients.

Revision history for this message
Faré (fahree) wrote :

A test case for what? The kind of things that will break wildly of you use the bad interface?

Well, you could have a system that indirectly depends on creating a binary with program-op, then using it (wouldn't currently work on sbcl, but that's another issue). If you put flags in the operation, the intermediate binary will be created with the wrong flags and fail in erratic ways.

Or, you could compile a system twice, with different flags in the operation. Because the cache cache and other data structures can't see the flags, ASDF will "succeed" immediately the second time over without having used the different flags at all.

Do you want to actually build such test cases just for the sake of checking that they fail currently?

Revision history for this message
Daniel Kochmański (jackdaniel) wrote :

I want to see such test cases to make sure it can't be fixed and is indeed broken. Is it surprising I want to know, why do you want to (or rather forcefully did) deprecate widely adopted and documented interface relevant for at least three maintained Common Lisp implementations? The only thing I've heard so far, that something will /break/ under conditions I don't know. You know ASDF, so it should be easy for you to convince me with an example.

That would convince me that something isn't right with the interface and they ought to be marked as deprecated (i.e in ECL documentation) if fix is impossible. Otherwise, as already stated, seems like an arbitrary decision.

Also, if ASDF has such cases in the regression tests (i.e marked as expected fail), then there will be no doubt that something ought to be done with it. Right now we have a very emotional comment, that the interface "must die" in the code, what isn't very convincing.

Revision history for this message
Daniel Kochmański (jackdaniel) wrote :

Also note, that I'm willing to learn why it's broken and can't be fixed.

Revision history for this message
Faré (fahree) wrote :

These functions have been deprecated since 2013, when ASDF3 introduced a better, supported, interface: program-op. Please use program-op and specify any flags in the system definition, NOT the operation.

These functions were NEVER supported by ASDF, and NEVER documented by ASDF. They were part of a brilliant but brittle hack, asdf-ecl, that was made obsolete by asdf-bundle, now an official part of ASDF since ASDF3 (in pre-release 2.27). I have told the ECL maintainers for years to move away from these unsupported interfaces. They will likely disappear from ASDF 3.2 (whenever THAT happens).

I gave plenty of explanations why operation initargs isn't supported. I will NOT write examples just for you, because you don't listen anyway. If Robert asks me, I will write test cases.

Everything "can" be fixed, but it's a lot of work, and that would only fix the second scenario I describe, not the first, which is intrinsic to the idiotic interface you fiercely defend. (No, the original authors of asdf-ecl weren't idiots, they just made a mistake; but you are stubborn in the worst way if you defend it after it's been obsoleted by better interfaces.)

Revision history for this message
Faré (fahree) wrote :

When YOU document some broken interface, it binds YOU, not US. You're the one making empty promises. Please back down, mark the interfaces as deprecated in the ECL documentation, and invite users to instead use the supported interface, PROGRAM-OP.

Revision history for this message
Daniel Kochmański (jackdaniel) wrote :

Very lazy explanation for deprecation (not that I expected anything else from your side), especially for change affecting others. Fact, that these operations were included in ASDF (by merging asdf-ecl) means, that they are supported by ASDF. Fact that ASDF is heavily underdocumented is an another issue.

It's a very sad state, when one of the project maintainers debunk question (with a good intent – I was willing to consider marking make-build as deprecated in ECL) with such rude answer – when you change your software, and it affects others, reasonable rationale is to be expected at least, without waving your hands and screaming that something is broken by design. Don't expect people will take your convictions at face value.

Revision history for this message
Faré (fahree) wrote :

If you want quixotic fights for deprecated or removed interfaces, may I suggest you also try to defend asdf:run-shell-command, :if-component-dep-fails, enable-asdf-binary-locations-compatibility or component-property? Also the old names fasl-op, binary-op, monolithic-load-fasl-op, etc.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.